Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands:\quit_if, \quit_unless)

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