This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Consolidate the custom TUI query hook with the default query hook


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;
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]