Re: [PATCH 2/2] gdb: handle bracketed-paste-mode and ctrl-d correctly

[Eli Zaretskii <eliz@gnu.org>]

> Date: Mon,  7 Mar 2022 15:13:15 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
> Currently, when readline is using bracketed-paste-mode, and the user
> quits gdb with EOF (typically bound to ctrl-d), then we end up with
> some corruption of the output.
> 
> If the user is sat at a prompt, like this:
> 
>   (gdb)
> 
> And then the user sends EOF, we currently see this:
> 
>   quit)
>   ... gdb terminates ...
> 
> Notice, that the 'quit' is printed over the (gdb) prompt.  This is not
> what we wanted, what we want to see is:
> 
>   (gdb) quit
>   ... gdb terminates ...
> 
> The problem is that, when readline detects that the user has entered a
> complete line, either by entering a newline, or by spotting EOF,
> readline will restore the terminal to a pre-readline state.  When
> bracketed-paste-mode is on, this includes switching
> bracketed-paste-mode back off.
> 
> In the case where a real line of input is entered, and then a newline
> sent, the cursor will have moved onto the next line before the
> terminal settings are restored.  However, when an EOF is detected, the
> cursor remains on the current line when the terminal settings are
> restored.
> 
> This difference, of whether the cursor is on a new line or not, is
> important.  In command_line_handler (event-top.c), this is where we
> print the final "quit" message, or more accurately, we print "quit\n",
> when the user sends EOF.  The final "\n" on that string moves the
> cursor onto the next line so that we're in a similar state as if the
> user had manually entered "quit" and then sent a newline.
> 
> And this is where bracketed-paste-mode causes a problem.  For
> background reading, see:
> 
>   https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html
> 
> The outcome of that discussion, is that, when readline disables
> bracketed-paste-mode, it also sends a "\r" to the terminal.
> 
> Now, in the case where the user sent a real command and a newline,
> this isn't a problem.  The cursor has moved to the next line
> anyway (which would be empty), and the "\r" simply returns us to the
> start of the (empty) line.  No problem there.
> 
> However, in the EOF case, we don't move to a newline.  So the "\r"
> after disabling bracketed-paste-mode returns the cursor to the start
> of the prompt line.  Now in command_line_handler, when we print
> "quit\n", this will overwrite the prompt.
> 
> It turns out that there is actually some problems with readline in
> this area.  Disabling of bracketed paste mode is done in the readline
> function rl_deprep_terminal.  As part of this readline should print a
> "\n" after disabling bracketed paste mode if an EOF was received.
> This is to work around specifically the issue we see above.
> Unfortunately, how readline detects that the EOF was seen doesn't work
> correctly when using the callback API, as GDB does.  As a result the
> "\n" is not printed from rl_deprep_terminal.
> 
> If we fix readline's detection of EOF then rl_deprep_terminal will now
> print the "\n" as expected, however, this has a knock on problem for
> GDB.  rl_deprep_terminal is called before the command line is
> dispatched back to GDB, this means that the "\n" will be printed from
> rl_deprep_terminal before GDB prints "quit\n", the result is that when
> the user sends EOF GDB's output will be:
> 
>   (gdb)
>   quit
> 
> Which is now what we wanted; we wanted the quit on the same line as
> the prompt.
> 
> To fix this problem I propose that within GDB we override the
> rl_deprep_terminal function.  This is a valid thing to do, the
> readline API exposes rl_deprep_term_function, which can be used to
> replace the deprep function.
> 
> Now, we can move the printing of "quit" into GDB's new deprep
> function, before calling on to readlines default rl_deprep_terminal
> function, which will then print the trailing "\n" for us.
> 
> The only problem with this is that readline doesn't expose whether it
> is handling EOF as part of its API.  However, in this thread:
> 
>   https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html
> 
> changes to readline were proposed, and accepted, that extended the
> readline API such that information about whether we're handling EOF is
> now available.
> 
> With this information available we can now use rl_deprep_term_function
> to print the "quit" message.
> 
> There's just a couple of final cases we need to cover.
> 
> First, GDB can link against the system readline.  The system readline
> might not have all of the fixes to expose the EOF status.  If this is
> the case then we can't print "quit" from our rl_deprep_term_function
> hook, as we can't know if we are handling EOF or not.
> 
> In this case, I have left GDB printing "quit" from the old location in
> command_line_handler.  To avoid prompt corruption, then, if bracketed
> paste mode is on, then we now deliberately print an extra "\n",
> forcing the quit onto the next line, like this:
> 
>   (gdb)
>   quit
> 
> This isn't ideal, but is, I think, the best we can do in this case.
> 
> The other edge case is from gdb_rl_deprep_term_function, our new
> deprep hook.  We have to consider two cases, bracketed paste mode on,
> and bracketed paste mode off.  As readline emits a "\n" after
> leaving bracketed paste mode, then if we print "quit\n" from
> gdb_rl_deprep_term_function we will end up with an extra blank line,
> like this:
> 
>   (gdb) quit
>   		# <--- This line left blank
>   [Shell-Prompt]>
> 
> However, if we only print "quit" from gdb_rl_deprep_term_function, but
> the user has bracketed paste mode disabled (say from .inputrc), then
> we end up with this:
> 
>   (gdb) quit[Shell-Prompt]>
> 
> Which obviously is not right.
> 
> My solution here is to temporarily set rl_eof_found back to 0 before
> calling the builtin rl_deprep_terminal function.  Right now, the only
> impact this has is to prevent readline printing the extra "\n", which
> solves the above problems.

Let me play the devil's advocate role and ask: isn't this too much of
a hassle for something that is basically an aesthetic issue, and in a
corner use case at that?  We are making non-trivial surgery in
Readline, something that was never tried before, we are importing a
partial change to upstream Readline, and we do all that a minute or so
before branching for GDB 12.  Are we sure this won't destabilize GDB
12 in ways we will never be able to discover and fix before the
release?