Re: Confusing behavior of psql's \e

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
Cc: Jacob Champion <pchampion(at)vmware(dot)com>, "chap(at)anastigmatix(dot)net" <chap(at)anastigmatix(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Confusing behavior of psql's \e
Date: 2021-03-12 18:12:46
Message-ID: 2539606.1615572766@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)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

Attachment Content-Type Size
0001-Improve-e-ef-and-ev-if-the-editor-is-quit.v5.patch text/x-diff 5.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bossart, Nathan 2021-03-12 18:16:15 Re: documentation fix for SET ROLE
Previous Message Robert Haas 2021-03-12 18:10:03 Re: pg_amcheck contrib application