[PATCH] Consolidate the custom TUI query hook with the default query hook
Patrick Palka
patrick@parcs.ath.cx
Fri Jan 9 13:11:00 GMT 2015
On Fri, Jan 9, 2015 at 8:01 AM, Patrick Palka <patrick@parcs.ath.cx> 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.
More information about the Gdb-patches
mailing list