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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
