At Mon, 20 Feb 2023 10:45:53 -0800, Andrey Borodin <amborodin86@gmail.com> wrote in
> > > > Also, if we do not provide a timespan, 2 seconds are selected. But if
> > > > we provide an incorrect argument - 1 second is selected.
> > > > PFA the patch that adds iteration count argument and makes timespan
> > > > argument more consistent.
> > >
> > > That should probably be fixed.
> >
> > And should probably be independent patches.
> >
>
> PFA 2 independent patches.
>
> Also, I've fixed a place to break after an iteration. Now if we have
> e.g. 2 iterations - there will be only 1 sleep time.
IMHO the current behavior for digit inputs looks fine to me. I feel
that the command should selently fix the input to the default in the
case of digits inputs like '-1'. But that may not be the case for
everyone. FWIW the patch still accepts an incorrect parameter '1abc'
by ignoring any trailing garbage.
In any case, I reckon the error message should be more specific. In
other words, it would be better if it suggests the expected input
format and range.
Regarding the second patch, if we want \watch to throw an error
message for the garbage trailing to sleep times, I think we should do
the same for iteration counts. Additionally, we need to update the
documentation.
By the way, when I looked this patch, I noticed that
exec_command_bind() doesn't free the malloc'ed return strings from
psql_scan_slash_option(). The same mistake is seen in some other
places. I'll take a closer look and get back in another thread.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
