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

Attached is the latest work. Not everything is done yet. I post it because
the next step is likely to be "tedious" as Tom put it, and if there's a way
out of it, I want to avoid it.

What is done:
- all changes here built off the v22 patch
- any function which had scan_state and cond_stack passed in now only has
scan_state, and cond_stack is extracted from the cb_passthrough pointer.
- ConditonalStack is now only explictly passed to get_prompt ... which
doesn't have scan state
- Conditional commands no longer reset scan state, nor do they clear the
query buffer
- boolean expressions consume all options, but only evaluate variables and
backticks in situations where those would be active
- invalid boolean arguments are treated as false
- contextually wrong \else, \endif, \elif are still errors

What is not done:
- TAP tests are not converted to regular regression test(s)
- skipped slash commands still consume the rest of the line

That last part is big, to quote Tom:

* More generally, I do not think that the approach of having exec_command
simply fall out immediately when in a false branch is going to work,
because it ignores the fact that different backslash commands have
different argument parsing rules. Some will eat the rest of the line and
some won't. I'm afraid that it might be necessary to remove that code
block and add a test to every single backslash command that decides
whether to actually perform its action after it's consumed its arguments.
That would be tedious :-(. But as it stands, backslash commands will get
parsed differently (ie with potentially-different ending points) depending
on whether they're in a live branch or not, and that seems just way too
error-prone to be allowed to stand.

If that's what needs to be done, does it make sense to first commit a
pre-patch that encapsulates each command family ( \c and \connect are a
family, all \d* commands are one family) into its own static function? It
would make the follow-up patch to if-endif cleaner and easier to review.

On Thu, Mar 16, 2017 at 5:14 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Corey Huinker <corey(dot)huinker(at)gmail(dot)com> writes:
> > Ok, I've got some time now and I'm starting to dig into this. I'd like to
> > restate what I *think* my feedback is, in case I missed or misunderstood
> > something.
> > ...
> > 3. Change command scans to scan the whole boolean expression, not just
> > OT_NORMAL.
> > There's a couple ways to go about this. My gut reaction is to create a
> new
> > scan type OT_BOOL_EXPR, which for the time being is the same as
> > OT_WHOLE_LINE, but could one day be something different.
>
> OT_WHOLE_LINE is not what you want because that results in verbatim
> copying, without variable expansion or anything. My vote would be to
> repeatedly do OT_NORMAL until you get a NULL, thereby consuming as
> many regular arguments as the backslash command has. (After which,
> if it wasn't exactly one argument, complain, for the moment. But this
> leaves the door open for something like "\if :foo = :bar".) Note that
> this implies that "\if some-expression \someothercommand" will be allowed,
> but I think that's fine, as I see no reason to allow backslashes in
> whatever if-expression syntax we invent later. OT_WHOLE_LINE is a bit of
> a bastard child and I'd just as soon not define it as being the lexing
> behavior of any new commands.
>
> > 5. Allow contextually-correct invalid boolean expressions to map to
> false.
>
> > Out-of-context \endif, \else, and \elif commands remain as errors to be
> > ignored, invalid expressions in an \if or legallyl-placed \elif are just
> > treated as false.
>
> WFM.
>
> regards, tom lane
>