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.
> 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?
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.
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.
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.
regards, tom lane