From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11816 invoked by alias); 9 Jan 2015 13:11:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 11804 invoked by uid 89); 9 Jan 2015 13:11:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mail-pa0-f52.google.com Received: from mail-pa0-f52.google.com (HELO mail-pa0-f52.google.com) (209.85.220.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 09 Jan 2015 13:11:14 +0000 Received: by mail-pa0-f52.google.com with SMTP id eu11so18420949pac.11 for ; Fri, 09 Jan 2015 05:11:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=TuUQS3Q95Dv/7EVGc3zu3wBDSiz8o/6qG2EfBkQVRko=; b=dTt9IXhKyA8u3bZM8RCJ0u0ggismPmYXIWTyWkGODUr0/cWC/GIYePGzr4XWNdJoqu Jpb63XgRMuyuCsdkOsc3aYsjc3HRy6BxSV/MTxdklkP5hxNhmTO1qpsach5COtpAQ0gX yN5DYCaMm7v97vbJfmrN5mIYXIQwGrpEjKNLiWbMAQkjTe4x+mqyIm7/3grDLw6nQBIJ EgyfPkXX7nHIJWn69vjve766z/vUgWttDoMqwHR++RJd9DyJ9M32Hw+AvSUx7weyiQ1/ UFFLcZ4X66uIBGmSsWw3ZPDQdITbnbH3/dxTO2DXIVjyzPSqVWAw8+39txq0skCc8R4g 0DOw== X-Gm-Message-State: ALoCoQky2m3qrEEO62U9aM46MZXNvXrkLHbeOP/JfGX2iQbm9TfFvDiJTUCDiaisF1Bh8GzLpQ4r X-Received: by 10.70.43.105 with SMTP id v9mr23818657pdl.158.1420809072156; Fri, 09 Jan 2015 05:11:12 -0800 (PST) MIME-Version: 1.0 Received: by 10.70.22.145 with HTTP; Fri, 9 Jan 2015 05:10:51 -0800 (PST) In-Reply-To: <1420808488-30044-1-git-send-email-patrick@parcs.ath.cx> References: <1420808488-30044-1-git-send-email-patrick@parcs.ath.cx> From: Patrick Palka Date: Fri, 09 Jan 2015 13:11:00 -0000 Message-ID: Subject: Re: [PATCH] Consolidate the custom TUI query hook with the default query hook To: gdb-patches@sourceware.org Cc: Pedro Alves , Patrick Palka Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-01/txt/msg00217.txt.bz2 On Fri, Jan 9, 2015 at 8:01 AM, Patrick Palka wrote: > Hi Pedro, > > I noticed some latent "rendering" issues when invoking GDB's quit prompt > within TUI, e.g. the quit prompt would repeat itself every time you > pressed a key. Here's an updated version of the patch that fixes the > two redisplay issues I found. TUI is a mess... > > -- >8 -- > > This patch primarily rewrites defaulted_query() to use > gdb_readline_wrapper() to prompt the user for input, like > prompt_for_continue() does. The motivation for this rewrite is to be > able to reuse the default query hook in TUI, obviating the need for a > custom TUI query hook. > > However, having TUI use the default query mechanism exposed a couple of > latent bugs in tui_redisplay_readline() related to the handling of > multi-line prompts, in particular GDB's multi-line quit prompt. > > The first issue is an off-by-one error in the height calculation of the > prompt. The check in question should be col <= prev_col, not > c < prev_col, to properly account for the case when a prompt contains > multiple consecutive newlines. Failing to do so makes TUI have the > wrong idea of the vertical height of the prompt. This patch fixes the > column check. > > The second issue is that cur_line does not get updated to reflect the > cursor position if the user's prompt cursor is at the end of the prompt > (i.e. if rl_point == rl_end). cur_line only gets updated if rl_point > lies between 0..rl_end-1 because that is the bounds of the for loop > responsible for updating cur_line. This patch changes the loop's bounds > to 0..rl_end so that cur_line always gets updated. > > With these two bug fixes out of the way, the default query mechanism > works well in TUI even with multi-line prompts like GDB's quit prompt. > > gdb/ChangeLog: > > * utils.c (defaulted_query): Rewrite to use gdb_readline_wrapper > to prompt for input. > * tui/tui-hooks.c (tui_query_hook): Remove. > (tui_install_hooks): Don't set deprecated_query_hook. > * tui/tui-io.c (tui_redisplay_readline): Fix off-by-one error in > height calculation. Always update the command window's cur_line. > --- > gdb/tui/tui-hooks.c | 64 ----------------------------------------------------- > gdb/tui/tui-io.c | 19 ++++++++-------- > gdb/utils.c | 61 ++++++++++++-------------------------------------- > 3 files changed, 24 insertions(+), 120 deletions(-) > > diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c > index 6ba6285..e53f526 100644 > --- a/gdb/tui/tui-hooks.c > +++ b/gdb/tui/tui-hooks.c > @@ -63,68 +63,6 @@ tui_new_objfile_hook (struct objfile* objfile) > tui_display_main (); > } > > -static int ATTRIBUTE_PRINTF (1, 0) > -tui_query_hook (const char *msg, va_list argp) > -{ > - int retval; > - int ans2; > - int answer; > - char *question; > - struct cleanup *old_chain; > - > - /* Format the question outside of the loop, to avoid reusing > - ARGP. */ > - question = xstrvprintf (msg, argp); > - old_chain = make_cleanup (xfree, question); > - > - echo (); > - while (1) > - { > - wrap_here (""); /* Flush any buffered output. */ > - gdb_flush (gdb_stdout); > - > - fputs_filtered (question, gdb_stdout); > - printf_filtered (_("(y or n) ")); > - > - wrap_here (""); > - gdb_flush (gdb_stdout); > - > - answer = tui_getc (stdin); > - clearerr (stdin); /* in case of C-d */ > - if (answer == EOF) /* C-d */ > - { > - retval = 1; > - break; > - } > - /* Eat rest of input line, to EOF or newline. */ > - if (answer != '\n') > - do > - { > - ans2 = tui_getc (stdin); > - clearerr (stdin); > - } > - while (ans2 != EOF && ans2 != '\n' && ans2 != '\r'); > - > - if (answer >= 'a') > - answer -= 040; > - if (answer == 'Y') > - { > - retval = 1; > - break; > - } > - if (answer == 'N') > - { > - retval = 0; > - break; > - } > - printf_filtered (_("Please answer y or n.\n")); > - } > - noecho (); > - > - do_cleanups (old_chain); > - return retval; > -} > - > /* Prevent recursion of deprecated_register_changed_hook(). */ > static int tui_refreshing_registers = 0; > > @@ -263,8 +201,6 @@ tui_install_hooks (void) > deprecated_print_frame_info_listing_hook > = tui_print_frame_info_listing_hook; > > - deprecated_query_hook = tui_query_hook; > - > /* Install the event hooks. */ > tui_bp_created_observer > = observer_attach_breakpoint_created (tui_event_create_breakpoint); > diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c > index 233e7a6..7b1ec43 100644 > --- a/gdb/tui/tui-io.c > +++ b/gdb/tui/tui-io.c > @@ -234,20 +234,23 @@ tui_redisplay_readline (void) > { > waddch (w, prompt[in]); > getyx (w, line, col); > - if (col < prev_col) > + if (col <= prev_col) > height++; > prev_col = col; > } > - for (in = 0; in < rl_end; in++) > + for (in = 0; in <= rl_end; in++) > { > unsigned char c; > > - c = (unsigned char) rl_line_buffer[in]; > if (in == rl_point) > { > getyx (w, c_line, c_pos); > } > > + if (in == rl_end) > + break; > + > + c = (unsigned char) rl_line_buffer[in]; > if (CTRL_CHAR (c) || c == RUBOUT) > { > waddch (w, '^'); > @@ -270,12 +273,10 @@ tui_redisplay_readline (void) > wclrtobot (w); > getyx (w, TUI_CMD_WIN->detail.command_info.start_line, > TUI_CMD_WIN->detail.command_info.curch); > - if (c_line >= 0) > - { > - wmove (w, c_line, c_pos); > - TUI_CMD_WIN->detail.command_info.cur_line = c_line; > - TUI_CMD_WIN->detail.command_info.curch = c_pos; > - } > + > + wmove (w, c_line, c_pos); > + TUI_CMD_WIN->detail.command_info.cur_line = c_line; > + TUI_CMD_WIN->detail.command_info.curch = c_pos; > TUI_CMD_WIN->detail.command_info.start_line -= height - 1; > Oops, this last hunk in tui-io.c is redundant. The c_line >= 0 predicate can stay (and probably should, to be safe). I intended to leave this hunk out.