Improve psql's behavior when the editor is exited without saving.

When editing the previous query buffer, if the editor is exited
without modifying the temp file then clear the query buffer,
rather than re-loading (and probably re-executing) the previous
query buffer.  This reduces the probability of accidentally
re-executing something you didn't intend to.

Similarly, in "\e file", if the file isn't actually modified
then don't load it into the query buffer.  And in "\ef" and
"\ev", if no changes are made then clear the query buffer
instead of loading the function or view definition into it.

Cases where we fail to invoke the editor at all, or it returns
a nonzero status, are treated like the no-file-modification case.

Laurenz Albe, reviewed by Jacob Champion

Discussion: https://postgr.es/m/0ba3f2a658bac6546d9934ab6ba63a805d46a49b[email protected]

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 01ec9b8b0a65e873d3c20b599efb7231c424b5cd..0ba1e2d44d47fe49be6a03db62fcf07cc2565c7d 100644 (file)
--- 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 e42be914d22cd16d78d4c8ae4294b3bb0c67aa8d..e04ccc5b628f52aa450103187577788d8ef3b869 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -148,11 +148,11 @@ static void save_query_text_state(PsqlScanState scan_state, ConditionalStack cst
                                  PQExpBuffer query_buf);
 static void discard_query_text(PsqlScanState scan_state, ConditionalStack cstack,
                               PQExpBuffer query_buf);
-static void copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf);
+static bool 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 discard_on_quit, bool *edited);
 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,
@@ -418,7 +418,7 @@ exec_command(const char *cmd,
     * the individual command subroutines.
     */
    if (status == PSQL_CMD_SEND)
-       copy_previous_query(query_buf, previous_buf);
+       (void) copy_previous_query(query_buf, previous_buf);
 
    return status;
 }
@@ -1004,14 +1004,27 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
            }
            if (status != PSQL_CMD_ERROR)
            {
+               bool        discard_on_quit;
+
                expand_tilde(&fname);
                if (fname)
+               {
                    canonicalize_path(fname);
+                   /* Always clear buffer if the file isn't modified */
+                   discard_on_quit = true;
+               }
+               else
+               {
+                   /*
+                    * If query_buf is empty, recall previous query for
+                    * editing.  But in that case, the query buffer should be
+                    * emptied if editing doesn't modify the file.
+                    */
+                   discard_on_quit = copy_previous_query(query_buf,
+                                                         previous_buf);
+               }
 
-               /* 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, discard_on_quit, NULL))
                    status = PSQL_CMD_NEWEDIT;
                else
                    status = PSQL_CMD_ERROR;
@@ -1134,7 +1147,7 @@ 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, true, &edited))
                status = PSQL_CMD_ERROR;
            else if (!edited)
                puts(_("No changes"));
@@ -2637,7 +2650,7 @@ exec_command_watch(PsqlScanState scan_state, bool active_branch,
        }
 
        /* If query_buf is empty, recall and execute previous query */
-       copy_previous_query(query_buf, previous_buf);
+       (void) copy_previous_query(query_buf, previous_buf);
 
        success = do_watch(query_buf, sleep);
 
@@ -2961,12 +2974,19 @@ discard_query_text(PsqlScanState scan_state, ConditionalStack cstack,
  * This is used by various slash commands for which re-execution of a
  * previous query is a common usage.  For convenience, we allow the
  * case of query_buf == NULL (and do nothing).
+ *
+ * Returns "true" if the previous query was copied into the query
+ * buffer, else "false".
  */
-static void
+static bool
 copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf)
 {
    if (query_buf && query_buf->len == 0)
+   {
        appendPQExpBufferStr(query_buf, previous_buf->data);
+       return true;
+   }
+   return false;
 }
 
 /*
@@ -3647,10 +3667,11 @@ UnsyncVariables(void)
 
 
 /*
- * do_edit -- handler for \e
+ * helper for do_edit(): actually invoke the editor
  *
- * If you do not specify a filename, the current query buffer will be copied
- * into a temporary one.
+ * Returns true on success, false if we failed to invoke the editor or
+ * it returned nonzero status.  (An error message is printed for failed-
+ * to-invoke cases, but not if the editor returns nonzero status.)
  */
 static bool
 editFile(const char *fname, int lineno)
@@ -3719,10 +3740,23 @@ 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 file.
+ *
+ * After this function is done, the resulting file will be copied back into the
+ * query buffer.  As an exception to this, the query buffer will be emptied
+ * if the file was not modified (or the editor failed) and the caller passes
+ * "discard_on_quit" = true.
+ *
+ * If "edited" isn't NULL, *edited will be set to true if the query buffer
+ * is successfully replaced.
+ */
 static bool
 do_edit(const char *filename_arg, PQExpBuffer query_buf,
-       int lineno, bool *edited)
+       int lineno, bool discard_on_quit, bool *edited)
 {
    char        fnametmp[MAXPGPATH];
    FILE       *stream = NULL;
@@ -3870,6 +3904,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
            {
                pg_log_error("%s: %m", fname);
                error = true;
+               resetPQExpBuffer(query_buf);
            }
            else if (edited)
            {
@@ -3879,6 +3914,15 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
            fclose(stream);
        }
    }
+   else
+   {
+       /*
+        * If the file was not modified, and the caller requested it, discard
+        * the query buffer.
+        */
+       if (discard_on_quit)
+           resetPQExpBuffer(query_buf);
+   }
 
    /* remove temp file */
    if (!filename_arg)