A few comments about v5.
> New patch.
Patch applies (with patch, I gave up on "git apply").
Make check ok.
Psql tap test ok.
> Highlights:
> - Interactive barking on branching state changes, commands typed while in
> inactive state
I noticed that the "barking" is conditional to "success". ISTM that it
should always "bark" in interactive mode, whether success or not.
While testing it and seeing the code, I agree that it is too
verbose/redundant. At least remove "active/inactive, ".
> - Help text. New block in help text called "Conditionals"
Maybe it could be moved to "Input/Output" renamed as "Input/Output
Control", or maybe the "Conditionals" section could be moved next to it,
it seems more logical than after large objects.
I think that the descriptions are too long. The interactive user can be
trusted to know what "if/elif/else/endif" mean, or to refer to the full
documentation otherwise. The point is just to provide a syntax and
function reminder, not a substitute for the doc. Thus I would suggest
shorter one-line messages like:
\if <expr> begin a new conditional block
\elif <expr> else if in the current conditional block
\else else in current conditional block
\endif end current conditional block
There should not be a \n at the end, I think, but just between sections.
> - SendQuery calls in mainloop.c are all encapsulated in send_query() to
> ensure the same if-active and if-interactive logic is used
Ok.
> - Exactly one perl TAP test, testing ON_ERROR_STOP. I predict more will be
> needed, but I'm not sure what coverage is desired
More that one:-)
> - I also predict that my TAP test style is pathetic
Hmmm. Perl is perl. Attached an attempt at improving that, which is
probably debatable, but at least it is easy to add further tests without
massive copy-pasting.
> - regression tests now have comments to explain purpose
Ok.
Small details about the code:
+ if (!pset.active_branch && !is_branching_command(cmd) )
Not sure why there is a space before the last closing parenthesis.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
