Hello Corey,
>> The check I was suggesting on whether Ctrl+C has been pressed
>>> on an empty line seems harder to implement, because get_interactive()
>>> just calls readline() or fgets(), which block to return when a whole
>>> line is ready. AFAICS psql can't know what was the edit-in-progress
>>> when these functions are interrupted by a signal instead of
>>> returning normally.
>>> But I don't think this check is essential, it could be left to another
>>> patch.
>>
>> Glad I wasn't missing something obvious.
>> I suppose we could base the behavior on whether there's at least one full
>> line already buffered.
>> However, I agree that it can be left to another patch.
Hmmm. ISTM that control-c must at least reset the stack, otherwise their
is not easy way to get out. What can be left to another patch is doing a
control-C for contents and then another one for the stack when there is no
content.
Comments about v6:
> - error messages are now a bit more terse, following suggestions
Ok.
> - help text is more terse and Conditionals section was moved below Input
> Output
Ok.
> - leverage IFSTATE_NONE a bit to fold some not-in-a-branch cases into
> existing switch statements, giving flatter, slightly cleaner code and that
> addresses expected cases before exceptional ones
Code looks ok.
> - comments highlight which messages are printed in both interactive and
> script mode.
Yep.
> - put Fabien's tap test in place verbatim
Hmmm. That was really just a POC... I would suggest some more tests, eg:
# elif error "\\if false\n\\elif error\n\\endif\n"
# ignore commands on error (stdout must be empty) "\\if error\n\\echo NO\n\\else\n\\echo NO\n\\endif\n"
Also there is an empty line before the closing } of the while loop.
> - No mention of Ctrl-C or PROMPT. Those can be addressed in separate
> patches.
I think that Ctrl-C resetting the stack must be addressed in this patch.
Trying to be more intelligent/incremental on Ctrl-C can wait for another
time.
> There's probably some more consensus building to do over the interactive
> messages and comments,
Barking is now quite more verbose (?), but at least it is clear about the
status and what is expected. I noticed that it is now always on, whether
an error occured or not, which is ok with me.
> and if interactive-ish tests are possible with TAP, we should add those
> too.
Good point. It seems that it is decided based on "source == stdin" plus
checking whether both stdin/stdout are on terminal. Allowing to work
around the later requires some more infrastructure to force "notty" (yuk,
a negative variable tested negatively...) to false whatever, which does
not seem to exist. So this is for another time.
--
Fabien.
