From patchwork Thu Mar 30 16:19:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 67125 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A2DBE385842B for ; Thu, 30 Mar 2023 16:19:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A2DBE385842B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1680193188; bh=4Omk0uQ7avbj3uCCSFZQDG41WI1yFb4BeLIFJfM/n00=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=A17Sht15NH8RorpR2NOqx5LGdbVirRhHjpfVA1AWD9tkVEbbKITnaRG4AdXtTQRSH n91T81VP+39ysepUgxkosoXxyoYpTTljJ9HADNfjmVubrSnnhsFN+D8wKVilUIiwkI pJyOdVAtEDhsDgyX7ivLK6Ke9TsCa1dF0Q+0It0Q= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id EAA2B3858CDA for ; Thu, 30 Mar 2023 16:19:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EAA2B3858CDA Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 27641219F8; Thu, 30 Mar 2023 16:19:21 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 0BE3D133E0; Thu, 30 Mar 2023 16:19:21 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id WseZAYm2JWSvZgAAMHmgww (envelope-from ); Thu, 30 Mar 2023 16:19:21 +0000 To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFC] [gdb/tui] Fix TUI with editing off Date: Thu, 30 Mar 2023 18:19:22 +0200 Message-Id: <20230330161922.5191-1-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Tom de Vries via Gdb-patches From: Tom de Vries Reply-To: Tom de Vries Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" 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" - type "tui enable" - type "quit" After entering tui, nothing happens. The "quit" isn't echoed, the 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 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 diff --git a/gdb/event-top.c b/gdb/event-top.c index 53ddd515be7..c1f9f7d948b 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -859,6 +859,8 @@ command_line_handler (gdb::unique_xmalloc_ptr &&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 &&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; diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c index 7752701378e..e6e72d91e65 100644 --- a/gdb/tui/tui-io.c +++ b/gdb/tui/tui-io.c @@ -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 ())); +}