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

On Sun, Feb 26, 2017 at 2:47 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:

>
> Hello Corey,
>
> About v18: Patch applies, make check ok, psql tap tests ok.
>
>
> ISTM that contrary to the documentation "\elif something" is not evaluated
> in all cases, and the resulting code is harder to understand with a nested
> switch and condition structure:
>
> switch
> default
> read
> if
> switch
>
> I wish (this is a personal taste) it could avoid switch-nesting on the
> very same value. It should also conform to the documentation.
>

I wasn't too happy with it, but I figured it would spark discussion. I
succeeded.

>
> If there is no compelling reason for the switch-nesting, I would suggest
> to move the read_boolean_expression before the swich, to deal with error
> immediately there, and then to have just one switch.
>

I thought about doing it that way. However, in the case of:

\set x invalid

\if true

\else
\elif :x

\endif

The error has already "happened" at line 4, char 5, and it doesn't matter
what expression follows, you will get an error. But because
read_boolean_expression() can emit errors, you would see the error saying
"invalid" isn't a valid boolean expression, and then see another error
saying that the \elif was out of place. If we suppress
read_boolean_expression()'s error reporting, then we have to re-create that
error message ourselves, or be fine with the error message on invalid
\elifs being inconsistent with invalid \ifs.

Similar to your suggestion below, we could encapsulate the first switch
into a function valid_elif_context(ConditionalStack), which might make the
code cleaner, but would make it harder to see that all swtich-cases are
covered between the two. That might be a tradeoff we want to make.

>
> Alternatively if the structure must really be kept, then deal with errors
> in a first switch, read value *after* switch and deal with other errors
> there, then start a second switch, and adjust the documentation accordingly?
>
> switch
> errors
> read
> if
> errors
> // no error
> switch
>

How would the documentation have to change?

Also, the %R documentation has single line marker '^' before not executed
> '@', but it is the reverse in the code.

Noted and fixed in the next patch, good catch.