On Tue, 2020-12-01 at 11:34 -0500, Chapman Flack wrote: > On 12/01/20 11:21, Laurenz Albe wrote: > > On Tue, 2020-12-01 at 11:03 -0500, Chapman Flack wrote: > > > > I propose to clear the query buffer if the > > > > editor exits without modifying the file. > > > > > > Or how about keeping the query buffer content unchanged, but not > > > executing it? Then it's still there if you have another idea about > > > editing it. It's easy enough to \r if you really want it gone. > > > > What if the buffer was empty? Would you want to get the previous > > query in the query buffer? I'd assume not... > > I took your proposal to be specifically about what happens if the editor > is exited with no change to the buffer, and in that case, I would suggest > making no change to the buffer, but not re-executing it. > > If the editor is exited after deliberately emptying the editor buffer, > I would expect that to be treated as emptying the query buffer. > > I don't foresee any case that would entail bringing a /previous/ query > back into the query buffer.
I see I'll have to try harder. The attached patch changes the behavior as follows: - If the current query buffer is edited, and you quit the editor, the query buffer is retained. This is as it used to be. - If the query buffer is empty and you run \e, the previous query is edited (as it used to be), but quitting the editor will empty the query buffer and execute nothing. - Similarly, if you "\e file" and quit the editor, nothing will be executed and the query buffer is emptied. - The same behavior change applies to \ef and \ev. There is no need to retain the definition in the query buffer, you can always run the \ev or \ev again. I consider this a bug fix, but one that shouldn't be backpatched. Re-executing the previous query if the editor is quit is annoying at least and dangerous at worst. I'll add this patch to the next commitfest. Yours, Laurenz Albe
From ecbf6cb81282ea5646b087290ca60a50daabe22c Mon Sep 17 00:00:00 2001 From: Laurenz Albe <laurenz.a...@cybertec.at> Date: Wed, 16 Dec 2020 10:34:14 +0100 Subject: [PATCH] Improve \e, \ef and \ev if the editor is quit Before, quitting the editor without saving would cause the *previous query* to be executed, which is certainly not what anyone would expect. It is better to execute nothing and clear the query buffer in this case. The exception is if the current query buffer is editied: in this case, the behavior is unclanged, and the query buffer is retained. Reviewed-by: Chapman Flack Discussion: https://postgr.es/m/0ba3f2a658bac6546d9934ab6ba63a805d46a49b.camel%40cybertec.at --- doc/src/sgml/ref/psql-ref.sgml | 10 +++++--- src/bin/psql/command.c | 43 +++++++++++++++++++++++----------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 221a967bfe..44eebbf141 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1949,7 +1949,9 @@ testdb=> </para> <para> - The new contents of the query buffer are then re-parsed according to + If you edit a file or the previous query, and you quit the editor without + modifying the file, the query buffer is cleared. + Otherwise, the new contents of the query buffer are re-parsed according to the normal rules of <application>psql</application>, treating the whole buffer as a single line. Any complete queries are immediately executed; that is, if the query buffer contains or ends with a @@ -2018,7 +2020,8 @@ Tue Oct 26 21:40:57 CEST 1999 in the form of a <command>CREATE OR REPLACE FUNCTION</command> or <command>CREATE OR REPLACE PROCEDURE</command> command. Editing is done in the same way as for <literal>\edit</literal>. - After the editor exits, the updated command is executed immediately + If you quit the editor without saving, the statement is discarded. + If you save and exit the editor, the updated command is executed immediately if you added a semicolon to it. Otherwise it is redisplayed; type semicolon or <literal>\g</literal> to send it, or <literal>\r</literal> to cancel. @@ -2094,7 +2097,8 @@ Tue Oct 26 21:40:57 CEST 1999 This command fetches and edits the definition of the named view, in the form of a <command>CREATE OR REPLACE VIEW</command> command. Editing is done in the same way as for <literal>\edit</literal>. - After the editor exits, the updated command is executed immediately + If you quit the editor without saving, the statement is discarded. + If you save and exit the editor, the updated command is executed immediately if you added a semicolon to it. Otherwise it is redisplayed; type semicolon or <literal>\g</literal> to send it, or <literal>\r</literal> to cancel. diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 38b588882d..15d7c343e9 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -151,7 +151,7 @@ static void copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf) static bool do_connect(enum trivalue reuse_previous_specification, char *dbname, char *user, char *host, char *port); static bool do_edit(const char *filename_arg, PQExpBuffer query_buf, - int lineno, bool *edited); + int lineno, bool *edited, bool discard_on_quit); static bool do_shell(const char *command); static bool do_watch(PQExpBuffer query_buf, double sleep); static bool lookup_object_oid(EditableObjectType obj_type, const char *desc, @@ -1000,6 +1000,13 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch, } if (status != PSQL_CMD_ERROR) { + /* + * If the query buffer is empty, we'll edit the previous query. + * But in that case, we don't want to keep that if the editor + * is quit. + */ + bool discard_on_quit = (query_buf->len == 0); + expand_tilde(&fname); if (fname) canonicalize_path(fname); @@ -1007,7 +1014,7 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch, /* If query_buf is empty, recall previous query for editing */ copy_previous_query(query_buf, previous_buf); - if (do_edit(fname, query_buf, lineno, NULL)) + if (do_edit(fname, query_buf, lineno, NULL, discard_on_quit)) status = PSQL_CMD_NEWEDIT; else status = PSQL_CMD_ERROR; @@ -1130,11 +1137,9 @@ exec_command_ef_ev(PsqlScanState scan_state, bool active_branch, { bool edited = false; - if (!do_edit(NULL, query_buf, lineno, &edited)) + if (!do_edit(NULL, query_buf, lineno, &edited, true)) status = PSQL_CMD_ERROR; - else if (!edited) - puts(_("No changes")); - else + else if (edited) status = PSQL_CMD_NEWEDIT; } @@ -3632,12 +3637,7 @@ UnsyncVariables(void) } -/* - * do_edit -- handler for \e - * - * If you do not specify a filename, the current query buffer will be copied - * into a temporary one. - */ +/* helper for do_edit() */ static bool editFile(const char *fname, int lineno) { @@ -3705,10 +3705,18 @@ editFile(const char *fname, int lineno) } -/* call this one */ +/* + * do_edit -- handler for \e + * + * If you do not specify a filename, the current query buffer will be copied + * into a temporary one. + * "edited" will be set to true if the file is modified. + * If "discard_on_quit" is true, discard the query buffer if the file was + * not modified. + */ static bool do_edit(const char *filename_arg, PQExpBuffer query_buf, - int lineno, bool *edited) + int lineno, bool *edited, bool discard_on_quit) { char fnametmp[MAXPGPATH]; FILE *stream = NULL; @@ -3845,6 +3853,13 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, fclose(stream); } } + else + { + if (discard_on_quit) + resetPQExpBuffer(query_buf); + else + puts(_("No changes")); + } /* remove temp file */ if (!filename_arg) -- 2.26.2