On Wed, 2021-03-03 at 17:12 +0000, Jacob Champion wrote:
> On Wed, 2021-03-03 at 17:08 +0100, Laurenz Albe wrote:
> > This is no new behavior, and I think this is rare enough that we don't
> > have to bother.
>
> I agree that it's not new behavior, but this patch exposes that
> behavior for the temporary file case, because you've fixed the bug that
> caused the timestamp check to not matter. As far as I can tell, you
> can't run into that race on the master branch for temporary files, and
> you won't run into it in practice for explicit filenames.
Actually, the timestamp check *did* matter before.
The code in "do_edit" has:
[after the editor has been called]
if (!error && before.st_mtime != after.st_mtime)
{
[read file back into query_buf]
}
This is pre-existing code. I just added an else branch:
else
{
[discard query_buf if we were editing a script, function or view]
}
So if you do your "modify and save the file in less than a second"
trick with the old code, you would end up with the old, unmodified
data in the query buffer.
I would say that the old behavior is worse in that case, and
discarding the query buffer is better.
I am not against fixing or improving the behavior, but given the
fact that we can't portably get less than second precision, it
seems impossible. For good measure, I have added a check if the
file size has changed.
I don't think we can or have to do better than that.
Few people are skilled enough to modify and save a file in less
than a second, and I don't think there have been complaints
about that behavior so far.
Attached is version 2 of the patch, with the added file size
check and a pgindent run.
Yours,
Laurenz Albe
From 8dc37cc9e775032f8f95cb837760dfe5463fc343 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Thu, 4 Mar 2021 17:33:59 +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.
Author: Laurenz Albe <laurenz.a...@cybertec.at>
Reviewed-by: Chapman Flack, Jacob Champion
Discussion: https://postgr.es/m/0ba3f2a658bac6546d9934ab6ba63a805d46a49b.camel%40cybertec.at
---
doc/src/sgml/ref/psql-ref.sgml | 10 ++++---
src/bin/psql/command.c | 49 +++++++++++++++++++++++-----------
2 files changed, 41 insertions(+), 18 deletions(-)
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 c98e3d31d0..2264ed5bab 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,
@@ -1003,6 +1003,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);
@@ -1010,7 +1017,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;
@@ -1133,11 +1140,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;
}
@@ -3626,12 +3631,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)
{
@@ -3699,10 +3699,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;
@@ -3809,7 +3817,10 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
error = true;
}
- if (!error && before.st_mtime != after.st_mtime)
+ /* consider the file edited if the size or modification time differ */
+ if (!error &&
+ (before.st_size != after.st_size ||
+ before.st_mtime != after.st_mtime))
{
stream = fopen(fname, PG_BINARY_R);
if (!stream)
@@ -3839,6 +3850,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)
--
2.26.2
