[RFC,gdb/tui] Fix TUI with editing off
Commit Message
While running the test-suite with editing set to off by default (see PR30289)
I stumbled onto PR tui/30290, triggered as follows:
- start gdb
- type "set editing off<enter>"
- type "tui enable<enter>"
- type "quit<enter>"
After entering tui, nothing happens. The "quit" isn't echoed, the <enter>
after that seems to have no effect and gdb hangs.
GDB is stuck in gdb_readline_no_editing_callback, waiting for a '\n', which
never comes because the <enter> at the end of the quit somehow produces a
'\r' instead.
Fix this by:
- adding a tui version of gdb_readline_no_editing_callback called
tui_readline_no_editing_callback, and
- calling it from gdb_readline_no_editing_callback, if tui_active.
I've left header file changes aside for now, to make recompilation fast.
Function tui_readline_no_editing_callback is like
gdb_readline_no_editing_callback, but different in that:
- it uses tui_getc_1 instead fgetc,
- it handles a return value of tui_getc_1 == 0, which you get when
tui_getc_1 handled say a KEY_UP,
- it handles a '\r' as eol,
- it echos chars and eol, and
- it handles backspace.
This gives us a reasonably usable TUI, just without the readline key bindings, so
we can't do say c-x o to switch focus, but we can use the focus command to set
the focus to the source window, and then use up/down arrows to scroll through it.
And, without the line editing benefits that either readline or terminal cooked
mode provides, but with support for using backspace.
I've tested the tui test-cases (gdb.tui/*.exp and gdb.python/tui*.exp) with
the editing set to on by default and there were no regressions.
With editing set to off by default, this patch fixes a few FAILs, for instance
in gdb.tui/completion.exp, but I still see these FAILs:
...
FAIL: gdb.python/tui-window-factory.exp: msg_3: check for python output
FAIL: gdb.tui/tui-focus.exp: check test2 focus message
FAIL: gdb.tui/tui-focus.exp: check ambiguous focus message
FAIL: gdb.tui/basic.exp: scroll up
FAIL: gdb.tui/basic.exp: scroll right
FAIL: gdb.tui/basic.exp: scroll down
FAIL: gdb.tui/tui-layout-asm.exp: scroll to end of assembler (scroll failed)
...
I investigated the "gdb.tui/basic.exp: scroll up" failure because AFAIU this
should work. The check is done like so:
...
if {[Term::wait_for [string_to_regexp $line]] \
&& [Term::get_line 1] != $line\
&& [Term::get_line 2] == $line} {
pass "scroll up"
...
and fails in the Term::wait_for part, because it first waits for the line, and
then for the prompt, and the prompt is never redrawn and so the wait times
out.
AFAIU, the scroll up just needs to have an effect on the source window, and I
don't see a need for any updates on the command window, so my preliminary
conclusion is that the test is incorrect, in the sense that it specifically
tests for something readline would do, but is not necessary.
But perhaps I'm mistaken there and we do need to redraw the prompt from some
reason here.
I have not investigated the other FAILs.
I'd like comments on:
- whether it's a good idea to make this work. Alternative, we can also just
bail out instead, or always enable editing when entering tui.
- whether my analysis about the scroll up test is correct.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30290
---
gdb/event-top.c | 8 ++++++
gdb/tui/tui-io.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+)
base-commit: 0791d754e7ef06ccce88322315210d49c1d0540e
Comments
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
Tom> While running the test-suite with editing set to off by default (see PR30289)
Tom> I stumbled onto PR tui/30290, triggered as follows:
Tom> - start gdb
Tom> - type "set editing off<enter>"
Tom> - type "tui enable<enter>"
Tom> - type "quit<enter>"
Tom> I'd like comments on:
Tom> - whether it's a good idea to make this work. Alternative, we can also just
Tom> bail out instead, or always enable editing when entering tui.
Hard to know. I personally have never tried turning off editing, but I
don't know if users rely on this or not.
I wonder if there's a way to make the 'editing' feature be implemented
in readline itself instead of gdb weirdness. Like, disable all key
bindings, or install an empty keymap.
If it doesn't work at all right now then that seems like evidence that
nobody is using this combination, and if we just force-enabled editing
in the TUI, that might be ok.
Tom> - whether my analysis about the scroll up test is correct.
I think so.
Tom
@@ -859,6 +859,8 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
}
}
+extern void tui_readline_no_editing_callback (gdb_client_data client_data);
+
/* Does reading of input from terminal w/o the editing features
provided by the readline library. Calls the line input handler
once we have a whole input line. */
@@ -866,6 +868,12 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
void
gdb_readline_no_editing_callback (gdb_client_data client_data)
{
+ if (tui_active)
+ {
+ tui_readline_no_editing_callback (client_data);
+ return;
+ }
+
int c;
std::string line_buffer;
struct ui *ui = current_ui;
@@ -1291,3 +1291,76 @@ tui_getc (FILE *fp)
return 0;
}
}
+
+extern void tui_readline_no_editing_callback (gdb_client_data client_data);
+
+void
+tui_readline_no_editing_callback (gdb_client_data client_data)
+{
+ int c;
+ std::string line_buffer;
+ struct ui *ui = current_ui;
+
+ FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
+ gdb_assert (stream != nullptr);
+
+ /* We still need the while loop here, even though it would seem
+ obvious to invoke gdb_readline_no_editing_callback at every
+ character entered. If not using the readline library, the
+ terminal is in cooked mode, which sends the characters all at
+ once. Poll will notice that the input fd has changed state only
+ after enter is pressed. At this point we still need to fetch all
+ the chars entered. */
+
+ while (1)
+ {
+ /* Read from stdin if we are executing a user defined command.
+ This is the right thing for prompt_for_continue, at least. */
+ c = tui_getc_1 (stream);
+ if (c == 0)
+ continue;
+
+ if (c == EOF)
+ {
+ if (!line_buffer.empty ())
+ {
+ /* The last line does not end with a newline. Return it, and
+ if we are called again fgetc will still return EOF and
+ we'll return NULL then. */
+ break;
+ }
+ ui->input_handler (NULL);
+ return;
+ }
+
+ if (c == '\n' || c == '\r')
+ {
+ if (!line_buffer.empty () && line_buffer.back () == '\r')
+ line_buffer.pop_back ();
+ tui_putc ('\n');
+ break;
+ }
+
+ if (c == '\b')
+ {
+ if (line_buffer.empty ())
+ {
+ /* Ignore. */
+ }
+ else
+ {
+ line_buffer.pop_back ();
+ tui_putc (c);
+ tui_putc (' ');
+ tui_putc (c);
+ }
+ }
+ else
+ {
+ tui_putc (c);
+ line_buffer += c;
+ }
+ }
+
+ ui->input_handler (make_unique_xstrdup (line_buffer.c_str ()));
+}