From patchwork Wed May 31 11:22:41 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: 70366 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 8E395385828E for ; Wed, 31 May 2023 11:23:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8E395385828E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1685532186; bh=9Zi9d/+nJDwDikosxPij0hnBT0d0jn7tUfXBF2yyYHc=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=b/tABx4MZ0++ulw6KUVGTimF/Ng9MiVhoRut6ZsbskXvm80U23v+xwdLZQMGF1Agz FNw1r4S8/3w6sRKLfCOtTo0OjH85fgfP+724iuqTXedEvTc+i7hyjOt1XcdVsW/bnl Op5QHDRNbbia5I9uoOTO8yNzDZHuAN7yTV5wY6jo= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 501803858D20 for ; Wed, 31 May 2023 11:22:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 501803858D20 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-out2.suse.de (Postfix) with ESMTPS id 716841F8C0; Wed, 31 May 2023 11:22:41 +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 5A025138E8; Wed, 31 May 2023 11:22:41 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id fyfpFAEud2RRQAAAMHmgww (envelope-from ); Wed, 31 May 2023 11:22:41 +0000 To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH] [gdb/tui] Fix spurious newline for long prompt Date: Wed, 31 May 2023 13:22:41 +0200 Message-Id: <20230531112241.19341-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_LOTSOFHASH, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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" I noticed that the test-suite doesn't excercise the case in tui_redisplay_readline that height (initially 1) is changed by this call: ... tui_puts_internal (w, prompt, &height); ... I decided to add a test-case, and ran into trouble. With a prompt of 40 chars, the same size as the terminal width, I expected to see: ... 16 (gdb) set prompt 123456789A123456789B123 17 456789C123456789> 18 123456789A123456789B123456789C123456789> 19 set prompt (gdb) 20 (gdb) ... but got instead: ... 16 (gdb) set prompt 123456789A123456789B123 17 456789C123456789> 18 123456789A123456789B123456789C123456789> 19 20 123456789A123456789B123456789C123456789> 21 set prompt (gdb) 22 (gdb) ... I traced this back to readline's readline_internal_setup, that does: ... /* If we're not echoing, we still want to at least print a prompt, because rl_redisplay will not do it for us. If the calling application has a custom redisplay function, though, let that function handle it. */ if (_rl_echoing_p == 0 && rl_redisplay_function == rl_redisplay) ... else { if (rl_prompt && rl_already_prompted) rl_on_new_line_with_prompt (); else rl_on_new_line (); (*rl_redisplay_function) (); ... and then we hit the case that calls rl_on_new_line_with_prompt, which does: ... /* If the prompt length is a multiple of real_screenwidth, we don't know whether the cursor is at the end of the last line, or already at the beginning of the next line. Output a newline just to be safe. */ if (l > 0 && (l % real_screenwidth) == 0) _rl_output_some_chars ("\n", 1); ... This doesn't look like a readline bug, because the behaviour matches the comment. I looked at ways to work around this, and managed by switching off rl_already_prompted, which we set to 1 in tui_rl_startup_hook: ... /* Readline hook to redisplay ourself the gdb prompt. In the SingleKey mode, the prompt is not printed so that the command window is cleaner. It will be displayed if we temporarily leave the SingleKey mode. */ static int tui_rl_startup_hook (void) { rl_already_prompted = 1; if (tui_current_key_mode != TUI_COMMAND_MODE && !gdb_in_secondary_prompt_p (current_ui)) tui_set_key_mode (TUI_SINGLE_KEY_MODE); tui_redisplay_readline (); return 0; } ... AFAIU, the intent of setting it to 1 has something to do with single-key mode. I don't think our testsuite excercises single-key mode. I've played around with single-key mode to see if I could notice a difference in behaviour due to the change, but didn't observe any. The call to tui_redisplay_readline looks related to setting rl_already_prompted to 1, so I've removed that one as well. Tested on x86_64-linux, no regressions. --- gdb/testsuite/gdb.tui/long-prompt.exp | 115 ++++++++++++++++++++++++++ gdb/tui/tui.c | 2 - 2 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.tui/long-prompt.exp base-commit: 768d1d879be2d134e049521f28d4d5e03b69bafc diff --git a/gdb/testsuite/gdb.tui/long-prompt.exp b/gdb/testsuite/gdb.tui/long-prompt.exp new file mode 100644 index 00000000000..3521d5c8b89 --- /dev/null +++ b/gdb/testsuite/gdb.tui/long-prompt.exp @@ -0,0 +1,115 @@ +# Copyright 2023 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test a prompt one less than, equal to, and one more than screen width in +# TUI. + +require allow_tui_tests + +tuiterm_env + +with_test_prefix "prompt size == width + 1" { + Term::clean_restart 24 40 + + if {![Term::enter_tui]} { + unsupported "TUI not supported" + return + } + + set prompt "123456789A123456789B123456789C123456789D>" + + # Set new prompt, and set old prompt back. + send_gdb "set prompt $prompt\n" + send_gdb "set prompt (gdb) \n" + + # Expected output: + # + # 16 (gdb) set prompt 123456789A123456789B123 + # 17 456789C123456789D> + # 18 123456789A123456789B123456789C123456789D + # 19 >set prompt (gdb) + # 20 (gdb) + + gdb_assert { [Term::wait_for "^>set prompt $gdb_prompt "] } \ + "got prompt back" + + gdb_assert { $Term::_cur_row == 20 } +} + +with_test_prefix "prompt size == width" { + Term::clean_restart 24 40 + + if {![Term::enter_tui]} { + unsupported "TUI not supported" + return + } + + set prompt "123456789A123456789B123456789C123456789>" + + # Set new prompt, and set old prompt back. + send_gdb "set prompt $prompt\n" + send_gdb "set prompt (gdb) \n" + + # Expected output: + # + # 16 (gdb) set prompt 123456789A123456789B123 + # 17 456789C123456789> + # 18 123456789A123456789B123456789C123456789> + # 19 set prompt (gdb) + # 20 (gdb) + # + # Note that we used to get: + # + # 16 (gdb) set prompt 123456789A123456789B123 + # 17 456789C123456789> + # 18 123456789A123456789B123456789C123456789> + # 19 + # 20 123456789A123456789B123456789C123456789> + # 21 set prompt (gdb) + # 22 (gdb) + + gdb_assert { [Term::wait_for "^set prompt $gdb_prompt "] } \ + "got prompt back" + + gdb_assert { $Term::_cur_row == 20 } +} + +with_test_prefix "prompt size == width - 1" { + Term::clean_restart 24 40 + + if {![Term::enter_tui]} { + unsupported "TUI not supported" + return + } + + set prompt "123456789A123456789B123456789C12345678>" + + # Set new prompt, and set old prompt back. + send_gdb "set prompt $prompt\n" + send_gdb "set prompt (gdb) \n" + + # Expected output: + # + # 16 (gdb) set prompt 123456789A123456789B123 + # 17 456789C12345678> + # 18 123456789A123456789B123456789C12345678>s + # 19 et prompt (gdb) + # 20 (gdb) + + gdb_assert { [Term::wait_for "^et prompt $gdb_prompt "] } \ + "got prompt back" + + gdb_assert { $Term::_cur_row == 20 } +} diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c index 10cf811a41e..eec9ee60cf9 100644 --- a/gdb/tui/tui.c +++ b/gdb/tui/tui.c @@ -264,11 +264,9 @@ tui_rl_next_keymap (int notused1, int notused2) static int tui_rl_startup_hook (void) { - rl_already_prompted = 1; if (tui_current_key_mode != TUI_COMMAND_MODE && !gdb_in_secondary_prompt_p (current_ui)) tui_set_key_mode (TUI_SINGLE_KEY_MODE); - tui_redisplay_readline (); return 0; }