> On 21 Mar 2025, at 11:34, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
> There are following differences between command input parsing and
> variable value parsing
> 1. empty string is considered as 0 value while parsing command input
> whereas it wiil cause error when setting to a variable.
> #\watch c=1 i=
> Fri 21 Mar 2025 08:27:25 AM IST (every 0s)
>
> ?column?
> ----------
> 1
> (1 row)
> #\set WATCH_INTERVAL
> invalid input syntax for "WATCH_INTERVAL"
>
> That can be considered as an existing bug and maybe fixed later.
An empty interval in command parsing implies "use the default", I don't think
there is a sensical counterpart in parsing actually setting the default value.
I think trying to define the default value without providing a value is an
error condition.
> With this patch, we are doing something unprecedented (at least
> AFAIK); allowing command arguments defaults to be configurable through
> a psql variable (as against an environment variable). I admit that
> configurable through a psql variable is better since it doesn't meddle
> with environment. Glancing through psql documentation, I didn't find a
> lot of command which may need default argument to be configurable.
> Nonetheless we should mention why this is special and set some
> guidance for such future additions - preferrably in code or at least
> in the commit message.
Sure, I'll mention it in the commit message.
> - of rows. Wait the specified number of seconds (default 2) between executions.
> - For backwards compatibility,
> + of rows. Wait the specified number of seconds (defaults to 2 if
> omitted, which can be
> + changed with the variable <xref linkend="app-psql-variables-watch-interval"/>)
> + between executions. For backwards compatibility,
>
> The text in parenthesis is quite long and it's hard to read ...
> seconds between execution. I suggest
> "Wait the specified number of seconds (default 2) between executions.
> The default wait can be changed by setting the variable <xref
> linkend="app-psql-variables-watch-interval"/>."
Fixed.
> + " number of seconds \\watch waits between executing the query buffer\n");
>
> I am feeling that this should mention "default" somewhere - maybe just
> add it before "number of ".
Fixed.
--
Daniel Gustafsson
