From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16192 invoked by alias); 9 Jan 2015 15:27:15 -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 16077 invoked by uid 89); 9 Jan 2015 15:27:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 09 Jan 2015 15:27:12 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t09FR8RP029896 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 9 Jan 2015 10:27:09 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t09FR6i5017069; Fri, 9 Jan 2015 10:27:07 -0500 Message-ID: <54AFF34A.6070109@redhat.com> Date: Fri, 09 Jan 2015 15:27:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Patrick Palka , gdb-patches@sourceware.org Subject: Re: [PATCH] Consolidate the custom TUI query hook with the default query hook References: <1420808488-30044-1-git-send-email-patrick@parcs.ath.cx> In-Reply-To: <1420808488-30044-1-git-send-email-patrick@parcs.ath.cx> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-01/txt/msg00219.txt.bz2 On 01/09/2015 01:01 PM, 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... Thanks a lot for cleaning up the mess. :-) It's great that a lot of the little annoying issues that plague the TUI are getting fixed. This is OK. Another nice side effect is that now all the readline keys, like ctrl-w works in queries (even in the CLI). Thanks, Pedro Alves > > -- >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; > > wrefresh (w); > diff --git a/gdb/utils.c b/gdb/utils.c > index 72b1e2a..a9a3082 100644 > --- a/gdb/utils.c > +++ b/gdb/utils.c > @@ -1198,12 +1198,11 @@ compile_rx_or_error (regex_t *pattern, const char *rx, const char *message) > static int ATTRIBUTE_PRINTF (1, 0) > defaulted_query (const char *ctlstr, const char defchar, va_list args) > { > - int answer; > int ans2; > int retval; > int def_value; > char def_answer, not_def_answer; > - char *y_string, *n_string, *question; > + char *y_string, *n_string, *question, *prompt; > /* Used to add duration we waited for user to respond to > prompt_for_continue_wait_time. */ > struct timeval prompt_started, prompt_ended, prompt_delta; > @@ -1263,62 +1262,31 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args) > > /* Format the question outside of the loop, to avoid reusing args. */ > question = xstrvprintf (ctlstr, args); > + prompt = xstrprintf (_("%s%s(%s or %s) %s"), > + annotation_level > 1 ? "\n\032\032pre-query\n" : "", > + question, y_string, n_string, > + annotation_level > 1 ? "\n\032\032query\n" : ""); > + xfree (question); > > /* Used for calculating time spend waiting for user. */ > gettimeofday (&prompt_started, NULL); > > while (1) > { > - wrap_here (""); /* Flush any buffered output. */ > - gdb_flush (gdb_stdout); > - > - if (annotation_level > 1) > - printf_filtered (("\n\032\032pre-query\n")); > - > - fputs_filtered (question, gdb_stdout); > - printf_filtered (_("(%s or %s) "), y_string, n_string); > - > - if (annotation_level > 1) > - printf_filtered (("\n\032\032query\n")); > + char *response, answer; > > - wrap_here (""); > gdb_flush (gdb_stdout); > + response = gdb_readline_wrapper (prompt); > > - answer = fgetc (stdin); > - > - /* We expect fgetc to block until a character is read. But > - this may not be the case if the terminal was opened with > - the NONBLOCK flag. In that case, if there is nothing to > - read on stdin, fgetc returns EOF, but also sets the error > - condition flag on stdin and errno to EAGAIN. With a true > - EOF, stdin's error condition flag is not set. > - > - A situation where this behavior was observed is a pseudo > - terminal on AIX. */ > - while (answer == EOF && ferror (stdin) && errno == EAGAIN) > - { > - /* Not a real EOF. Wait a little while and try again until > - we read something. */ > - clearerr (stdin); > - gdb_usleep (10000); > - answer = fgetc (stdin); > - } > - > - clearerr (stdin); /* in case of C-d */ > - if (answer == EOF) /* C-d */ > + if (response == NULL) /* C-d */ > { > printf_filtered ("EOF [assumed %c]\n", def_answer); > retval = def_value; > break; > } > - /* Eat rest of input line, to EOF or newline. */ > - if (answer != '\n') > - do > - { > - ans2 = fgetc (stdin); > - clearerr (stdin); > - } > - while (ans2 != EOF && ans2 != '\n' && ans2 != '\r'); > + > + answer = response[0]; > + xfree (response); > > if (answer >= 'a') > answer -= 040; > @@ -1333,8 +1301,7 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args) > specify the required input or have it default by entering > nothing. */ > if (answer == def_answer > - || (defchar != '\0' && > - (answer == '\n' || answer == '\r' || answer == EOF))) > + || (defchar != '\0' && answer == '\0')) > { > retval = def_value; > break; > @@ -1350,7 +1317,7 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args) > timeval_add (&prompt_for_continue_wait_time, > &prompt_for_continue_wait_time, &prompt_delta); > > - xfree (question); > + xfree (prompt); > if (annotation_level > 1) > printf_filtered (("\n\032\032post-query\n")); > return retval; >