Hello Tom,
>> ISTM that I've tried to suggest to work around that complexity by:
>> - document that \if-related commands should only occur at line start
>> (and extend to eol).
>> - detect and complain when this is not the case.
>
> I think this is a lousy definition, and would never be considered if we
> were working in a green field.
Yes, sure. As you pointed out, the field is not green: there is no clean
lexical convention, too bad. I'm trying to deal with that without too much
fuss in the code.
> Moreover, preventing such cases would be pretty darn ugly/messy as well.
>
> I also fear that there are corner cases where the behavior would still
> be inconsistent. Consider
>
> \if ...
> \set foo `echo \endif should not appear here`
In this instance, ISTM that there is no problem. On "\if true", set is
executed, all is well. On "\if false", the whole line would be skipped
because the if-related commands are only expected on their own line, all
is well again. No problem.
Another more interesting one would be:
\if ... \unset foo \endif
On true, unset get its argument, then endif is detected as a backslash
command, but it would see that it is not on its own line, so it would
error out *and* be ignored. On false, the whole line would be ignored, it
would just not complain, but it would be the same, i.e. it is *not* an
\endif again. The drawback is only that the wrong \endif is not detected
when under a false branch. That is why I added a third bullet "call border
cases a feature".
ISTM that the proposed simple rules allow to deal with the situation
without having to dive into each command lexing rules, and changing the
existing code significantly. The drawback is that misplaced \endif are not
detected in false branch, but they are ignored anyway, which is fine.
> I'm imagining that instead of
>
> [...] char *envvar = psql_scan_slash_option(scan_state,
>
> we'd write
>
> [...] char *envvar = args[0];
>
> where the args array had been filled at the top of the function.
> The top-of-function code would have to know all the cases where
> commands didn't use basic OT_NORMAL processing, but there aren't
> that many of those, I think.
Yep, I understood the idea. There are a few of those, about 49 OT_* in
"command.c", including 34 OT_NORMAL, 1 OT_NO_EVAL, 3 OT_FILEPIPE, 9
OT_WHOLELINE, some OT_SQLHACKID & OT_SQLID. I'm not sure of the
combinations.
It still means splitting command lexing knowledge in several places. I'm
not convinced by the impact on the resulting code with regard to
readability and maintainability, so if there could be a way to get
something without taking that path that would be nice, hence my
suggestions.
--
Fabien.
