Laurenz Albe <laurenz.a...@cybertec.at> writes: > Done like that in the attached patch version 4.
I pushed the race-condition-fixing part of this, since that's an unarguable bug fix and hence seems OK to back-patch. (I added a check on change of file size, because why not.) Attached is the rest, just to keep the cfbot happy. I don't think this is quite committable yet. The division of labor between do_edit() and its callers seems to need more thought: in particular, I see that \ef now fails to print "No changes" when I would expect it to. But the real question is whether there is any non-error condition in which do_edit should not set the query_buffer, either to the edit result or empty. If we're going to improve the header comment for do_edit, I would expect it to specify what happens to the query_buf, and the fact that I can't write a concise spec leads me to think that a bit more design effort is needed. Also, somewhat cosmetic, but: I feel like the hunk that is setting discard_on_quit in exec_command_edit is assuming more than it ought to about what copy_previous_query will do. Perhaps it's worth making copy_previous_query return a bool indicating whether it copied previous_buf, and then exec_command_edit becomes discard_on_quit = copy_previous_query(query_buf, previous_buf); regards, tom lane
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 13c1edfa4d..0bf69174ff 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1970,7 +1970,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 @@ -2039,7 +2041,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. @@ -2115,7 +2118,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 838f74bbbb..3b97cd51dc 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -152,7 +152,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, @@ -1004,6 +1004,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); @@ -1011,7 +1018,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; @@ -1134,11 +1141,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; } @@ -3627,12 +3632,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) { @@ -3700,10 +3700,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; @@ -3860,6 +3868,14 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, fclose(stream); } } + else + { + /* if we were editing anything else than the query buffer, discard */ + if (discard_on_quit) + resetPQExpBuffer(query_buf); + else + puts(_("No changes")); + } /* remove temp file */ if (!filename_arg)