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

On Wed, Feb 22, 2017 at 4:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Corey Huinker <corey.huinker@gmail.com> writes:
> >> but if you think that it should be somewhere else, please advise Corey
> >> about where to put it.
>
> > Just a reminder that I'm standing by for advice.
>
> Sorry, I'd lost track of this thread.
>

Judging by the volume of the 50-or-so active threads on this list, I
figured you were busy.


> > The issue at hand is whether the if-state should be a part of the
> > PsqlScanState, or if it should be a separate state variable owned by
> > MainLoop() and passed to HandleSlashCommands(), ... or some other
> solution.
>
> My reaction to putting it in PsqlScanState is pretty much "over my dead
> body".  That's just trashing any pretense of an arms-length separation
> between psql and the lexer proper.  We only recently got done sweating
> blood to create that separation, why would we toss it away already?
>

Good to know that history.


>
> If you're concerned about the notational overhead of passing two arguments
> rather than one, my druthers would be to invent a new struct type, perhaps
> named along the lines of PsqlFileState or PsqlInputState, and pass that
> around.  One of its fields would be a PsqlScanState pointer, the rest
> would be for if-state and whatever else we think we need in per-input-file
> state.
>

I wasn't, my reviewer was. I thought about the super-state structure like
you described, and decided I was happy with two state params.


> However, that way is doubling down on the assumption that the if-state is
> exactly one-to-one with input file levels, isn't it?  We might be better
> off to just live with the separate arguments to preserve some flexibility
> in that regard.  The v12 patch doesn't look that awful in terms of what
> it's adding to argument lists.
>

The rationale for tying if-state to file levels is not so much of anything
if-then related, but rather of the mess we'd create for whatever poor soul
decided to undertake \while loops down the road, and the difficulties
they'd have trying to unwind/rewind jump points in file(s)...keeping it
inside one file makes things simpler for the coding and the coder.


> One thing I'm wondering is why the "active_branch" bool is in "pset"
> and not in the conditional stack.  That seems, at best, pretty grotty.
> _psqlSettings is meant for reasonably persistent state.
>

With the if-stack moved to MainLoop(), nearly all the active_branch checks
could be against a variable that lives in MainLoop(), with two big
exceptions: GetVariable() needs to know when NOT to expand a variable
because it's in a false-block, and get_prompt will need to know when it's
in a false block for printing the '@' prompt hint or equivalent, and pset
is the only global around I know of to do that. I can move nearly all the
is-this-branch-active checks to structures inside of MainLoop(), and that
does strike me as cleaner, but there will still have to be that gross bit
where we update pset.active_branch so that the prompt and GetVariable() are
clued in.