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

Hello Corey,

> v24 highlights:
>
> - finally using git format-patch
> - all conditional slash commands broken out into their own functions
> (exec_command_$NAME) , each one tests if it's in an active branch, and if
> it's not it consumes the same number of parameters, but discards them.
> comments for each slash-command family were left as-is, may need to be
> bulked up.
> - TAP tests discarded in favor of one ON_EROR_STOP test for invalid \elif
> placement
> - documentation recommending ON_ERROR_STOP removed, because invalid
> expressions no longer throw off if-endif balance
> - documentation updated to reflex that contextually-correct-but-invalid
> boolean expressions are treated as false
> - psql_get_variable has a passthrough void pointer now, but I ended up not
> needing it. Instead, all slash commands in false blocks either fetch
> OT_NO_EVAL or OT_WHOLE_LINE options. If I'm missing something, let me know.

A few comments about the patch.

Patch applies. "make check" ok.

As already pointed out, there is one useless file in the patch.

Although currently there is only one expected argument for boolean 
expressions, the n² concatenation algorithm in gather_boolean_expression 
is not very elegant. Is there some string buffer data structure which 
could be used instead?

ISTM that ignore_boolean_expression may call free on a NULL pointer if the 
expression is empty?

Generally I find the per-command functions rather an improvement.

However there is an impact on testing because of all these changes. ISTM 
that test cases should reflect this situation and test that \cd, \edit, 
... are indeed ignored properly and taking account there expected args...

In "exec_command_connect" an argument is changed from "-reuse-previous" to 
"-reuse-previous=", not sure why.

There seems to be pattern repetition for _ev _ef and _sf _sv. Would it 
make sense to create a function instead of keeping the initial copy-paste?

I think that some functions could be used for some repeated cases such as 
"discard one arg", "discard one or two arg", "discard whole line", for the 
various inactive branches, so as to factor out code.

I would suggest to put together all if-related backslash command, 
so that the stack management is all in one function instead of 4.

For pset the inactive branch does OT_NORMAL instead of OT_NOT_EVAL, not 
sure why.

-- 
Fabien.