From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 278603858409 for ; Fri, 21 Jul 2023 07:37:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 278603858409 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de 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 F13651FE9B; Fri, 21 Jul 2023 07:37:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1689925049; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ba13gJ5nFvvX98RsfKPznDMpR2zzAst3FF250wkRMy8=; b=BHIC97fuzwjpNbHryh1xLUiXEi/TQrJ0+qkOsiK2cYKV9WOBhqpdKfT45DHL2BccYbgIl6 NFgSSfAxAzqeVqVXO98THL/00Qtqdi9G8lBUXgsC5aITOOJrsnNIvcj+FWW2UY8hesZTU+ YuXWe/ktVlQ14TDRhPpvr5/VwFky7qA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1689925049; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ba13gJ5nFvvX98RsfKPznDMpR2zzAst3FF250wkRMy8=; b=A3bkO/Vjl5JsXUTonjmeiAMcCeMnAitqxRuVQG7d9CXjmogMsucvFtcP/sMKe63zyQrra6 zzWcq5KMcf2dWUBg== 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 CF507134BA; Fri, 21 Jul 2023 07:37:29 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id kMxQMbk1umR2OAAAMHmgww (envelope-from ); Fri, 21 Jul 2023 07:37:29 +0000 Message-ID: <3c2b0584-591d-8f2d-dd3c-cc478d319bea@suse.de> Date: Fri, 21 Jul 2023 09:37:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v2] [gdb/tui] Fix superfluous newline for long prompt Content-Language: en-US From: Tom de Vries To: gdb-patches@sourceware.org Cc: Tom Tromey References: <20230622191423.4197-1-tdevries@suse.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_LOTSOFHASH,KAM_SHORT,NICE_REPLY_A,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 List-Id: On 7/14/23 14:18, Tom de Vries wrote: > On 6/22/23 21:14, Tom de Vries via Gdb-patches wrote: >> In test-case gdb.tui/long-prompt.exp, with a prompt of 40 chars, the >> same size >> as the terminal width, we get a superfluous newline at line 19: >> ... >> 16 (gdb) set prompt 123456789A123456789B123 >> 17 456789C123456789> >> 18 123456789A123456789B123456789C123456789> >> 19 >> 20 123456789A123456789B123456789C123456789> >> 21 set prompt (gdb) >> 22 (gdb) >> ... >> as well as a superfluous repetition of the prompt at line 20 once we >> type the >> 's' starting "set prompt". >> >> I traced the superfluous newline 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. >> >> [ And the fact that the output of the newline doesn't happen in the >> scope of >> tui_redisplay_readline means it doesn't get the prompt wrap detection >> treatment, causing start_line to be incorrect, which causes the >> superfluous >> repetition of the prompt. ] >> >> 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; >> } >> ... >> >> Then I started looking at why rl_already_prompted is set to 1. >> >> The use case for rl_already_prompted seems to be: >> - app (application, the readline user) outputs prompt, >> - app sets rl_already_prompted to 1, and >> - app calls readline, which calls rl_on_new_line_with_prompt, which >> figures >>    out how long the prompt is, and sets a few readline variables >> accordingly, >>    which can be used in the following call to rl_redisplay_function. >> >> AFAICT, TUI does not fit this pattern.  It does not output an initial >> prompt, >> rather it writes the prompt in every rl_redisplay_function.  It >> doesn't use >> the variables set by rl_on_new_line_with_prompt, instead it figures >> stuff out >> by itself. >> >> Fix this by removing the rl_already_prompted setting. >> >> Also remove the call to tui_redisplay_readline, it's not necessary, the >> function is called anyway. >> > > I'll commit this end of next week, unless there are comments. > Pushed. Thanks, - Tom >> Tested on x86_64-linux, no regressions. >> --- >>   gdb/testsuite/gdb.tui/long-prompt.exp | 18 +----------------- >>   gdb/tui/tui.c                         |  2 -- >>   2 files changed, 1 insertion(+), 19 deletions(-) >> >> diff --git a/gdb/testsuite/gdb.tui/long-prompt.exp >> b/gdb/testsuite/gdb.tui/long-prompt.exp >> index e4e8b75ede0..a08c08a884a 100644 >> --- a/gdb/testsuite/gdb.tui/long-prompt.exp >> +++ b/gdb/testsuite/gdb.tui/long-prompt.exp >> @@ -101,29 +101,13 @@ with_test_prefix "prompt size == width" { >>       #   16 (gdb) set prompt 123456789A123456789B123 >>       #   17 456789C123456789> >>       #   18 123456789A123456789B123456789C123456789> >> -    #   19 >> -    #   20 123456789A123456789B123456789C123456789> >> -    #   21 set prompt (gdb) >> -    #   22 (gdb) >> -    # >> -    # Note that it would be nice to get instead: >> -    # >> -    #   16 (gdb) set prompt 123456789A123456789B123 >> -    #   17 456789C123456789> >> -    #   18 123456789A123456789B123456789C123456789> >>       #   19 set prompt (gdb) >>       #   20 (gdb) >> -    # >> -    # The extra newline is added by rl_on_new_line_with_prompt, which >> decides >> -    # this as follows: >> -    #  /* 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. */ >>       gdb_assert { [Term::wait_for "^set prompt $gdb_prompt "] } \ >>       "got prompt back" >> -    gdb_assert { $Term::_cur_row == 22 } >> +    gdb_assert { $Term::_cur_row == 20 } >>   } >>   with_test_prefix "prompt size == width - 1" { >> diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c >> index 43be8161e4c..941c65c970f 100644 >> --- a/gdb/tui/tui.c >> +++ b/gdb/tui/tui.c >> @@ -271,11 +271,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; >>   } >> >> base-commit: d4d5b571954a2bde2a14772d9b18027a9048eb2d >