> On 17 Mar 2025, at 13:37, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
> 0 is an accepted value for interval, even though it might look insensible.
>
> The behaviour should be same in both cases \watch i=<some value> and
> \set WATCH_INTERVAL <some value> \watch. In this case it's not.
Having a watch interval of zero is IMHO somewhat nonsensical, but since it was
done intentionally in 6f9ee74d45 (which I had missed) I agree that the default
should support it as well. Fixed.
> In fact, we should use the same validation code in both the cases. Why
> don't we perform the same additional validations in
> ParseVariableDouble() in exec_watch_command() as well?
Well, they don't use the same code since they are two different things
(variables and command input, while using the same syscalls they have different
errorhandling requirements). If executing \watch would use ParseVariableDouble
it would make errorhandling quite awkward at best. I added a comment in
exec_command_watch that any changes in internval parsing should take default
intervals into consideration.
> The test only validate default variable value. We need a test where we
> see variable value being honored lie tests between 369 to 376 in the
> same file.
I'm not sure it's worth spending test cycles on as this code doesn't affect the
execution of \watch at all. That being said, I added a testcase which sets a
default and then executes \watch. It doesn't test that the interval was
correct (we don't do that for any), but at least it will catch if setting a
default totally breaks \watch.
--
Daniel Gustafsson
