PING Re: [RFA] Have 'thread|frame apply' style their output.

[Philippe Waroquiers <philippe dot waroquiers at skynet dot be>]

Ping ?
Thanks
Philippe

On Sat, 2019-03-09 at 23:59 +0100, Philippe Waroquiers wrote:
> 'thread|frame apply CMD' launches CMD so that CMD output goes to a string_file.
> This patch ensures that string_file for such CMD output contains
> style escape sequences that 'thread|frame apply' will later on
> output on the real terminal, so as to have CMD output properly styled.
> 
> The idea is to have the class ui_file having overridable methods
> to indicate that the output to this ui_file should be done using
> 'terminal' behaviour such as styling.
> Then these methods are overriden in string_file so that a specially
> constructed string_file will get output with style escape sequences.
> 
> After this patch, the output of CMD by thread|frame apply CMD is styled
> similarly as when CMD is launched directly.
> Note that string_file (term_out true) could also support wrapping,
> but this is not done (yet?).
> 
> Tested on debian/amd64.
> 
> gdb/ChangeLog
> 2019-XX-YY  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	Support style in 'frame|thread apply'
> 
> 	* gdbcmd.h (execute_command_to_string): New term_out parameter.
> 	* record.c (record_start, record_stop): Update callers of
> 	execute_command_to_string with false.
> 	* ui-file.h (class ui_file): New term_out and can_emit_style_escape
> 	methods.
> 	(class string_file): New constructor with term_out parameter.
> 	Override methods term_out and can_emit_style_escape.  New member
> 	term_out.
> 	(class stdio_file): Override can_emit_style_escape.
> 	(class tee_file): Override term_out and can_emit_style_escape.
> 	* utils.h (can_emit_style_escape): Remove.
> 	* utils.c (can_emit_style_escape): Likewise.
> 	Update all callers of can_emit_style_escape (SOMESTREAM) to
> 	SOMESTREAM->can_emit_style_escape.
> 	* source-cache.c (source_cache::get_source_lines): Likewise.
> 	* stack.c (frame_apply_command_count): Call execute_command_to_string
> 	passing the term_out characteristic of the current gdb_stdout.
> 	* thread.c (thr_try_catch_cmd): Likewise.
> 	* top.c (execute_command_to_string): pass term_out parameter
> 	to construct the string_file for the command output.
> 	* ui-file.c (term_cli_styling): New function (most code moved
> 	from utils.c can_emit_style_escape.
> 	(string_file::string_file, string_file::can_emit_style_escape,
> 	stdio_file::can_emit_style_escape, tee_file::term_out,
> 	tee_file::can_emit_style_escape): New functions.
> ---
>  gdb/gdbcmd.h       |  7 ++++-
>  gdb/record.c       | 12 ++++----
>  gdb/source-cache.c |  2 +-
>  gdb/stack.c        |  3 +-
>  gdb/thread.c       |  3 +-
>  gdb/top.c          |  5 ++--
>  gdb/ui-file.c      | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/ui-file.h      | 25 ++++++++++++++++-
>  gdb/utils.c        | 37 ++++--------------------
>  gdb/utils.h        |  4 ---
>  10 files changed, 120 insertions(+), 48 deletions(-)
> 
> diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
> index 4614ec748c..5d0e697d83 100644
> --- a/gdb/gdbcmd.h
> +++ b/gdb/gdbcmd.h
> @@ -133,7 +133,12 @@ extern struct cmd_list_element *showchecklist;
>  extern struct cmd_list_element *save_cmdlist;
>  
>  extern void execute_command (const char *, int);
> -extern std::string execute_command_to_string (const char *p, int from_tty);
> +
> +/* Execute command P and returns its output.  If TERM_OUT,
> +   the output is built using terminal output behaviour such
> +   as cli_styling.  */
> +extern std::string execute_command_to_string (const char *p, int from_tty,
> +					      bool term_out);
>  
>  extern void print_command_line (struct command_line *, unsigned int,
>  				struct ui_file *);
> diff --git a/gdb/record.c b/gdb/record.c
> index 9c703646a7..828c19968a 100644
> --- a/gdb/record.c
> +++ b/gdb/record.c
> @@ -99,25 +99,25 @@ record_start (const char *method, const char *format, int from_tty)
>    if (method == NULL)
>      {
>        if (format == NULL)
> -	execute_command_to_string ("record", from_tty);
> +	execute_command_to_string ("record", from_tty, false);
>        else
>  	error (_("Invalid format."));
>      }
>    else if (strcmp (method, "full") == 0)
>      {
>        if (format == NULL)
> -	execute_command_to_string ("record full", from_tty);
> +	execute_command_to_string ("record full", from_tty, false);
>        else
>  	error (_("Invalid format."));
>      }
>    else if (strcmp (method, "btrace") == 0)
>      {
>        if (format == NULL)
> -	execute_command_to_string ("record btrace", from_tty);
> +	execute_command_to_string ("record btrace", from_tty, false);
>        else if (strcmp (format, "bts") == 0)
> -	execute_command_to_string ("record btrace bts", from_tty);
> +	execute_command_to_string ("record btrace bts", from_tty, false);
>        else if (strcmp (format, "pt") == 0)
> -	execute_command_to_string ("record btrace pt", from_tty);
> +	execute_command_to_string ("record btrace pt", from_tty, false);
>        else
>  	error (_("Invalid format."));
>      }
> @@ -130,7 +130,7 @@ record_start (const char *method, const char *format, int from_tty)
>  void
>  record_stop (int from_tty)
>  {
> -  execute_command_to_string ("record stop", from_tty);
> +  execute_command_to_string ("record stop", from_tty, false);
>  }
>  
>  /* See record.h.  */
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 097c8a3ae1..0643ef4343 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -174,7 +174,7 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
>      return false;
>  
>  #ifdef HAVE_SOURCE_HIGHLIGHT
> -  if (can_emit_style_escape (gdb_stdout))
> +  if (gdb_stdout->can_emit_style_escape ())
>      {
>        const char *fullname = symtab_to_fullname (s);
>  
> diff --git a/gdb/stack.c b/gdb/stack.c
> index bce8d58f54..63cff3f171 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -2703,7 +2703,8 @@ frame_apply_command_count (const char *which_command,
>  	       set to the selected frame.  */
>  	    scoped_restore_current_thread restore_fi_current_frame;
>  
> -	    cmd_result = execute_command_to_string (cmd, from_tty);
> +	    cmd_result = execute_command_to_string
> +	      (cmd, from_tty, gdb_stdout->term_out ());
>  	  }
>  	  fi = get_selected_frame (_("frame apply "
>  				     "unable to get selected frame."));
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 6c23252964..99ed6efdee 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1460,7 +1460,8 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
>    switch_to_thread (thr);
>    TRY
>      {
> -      std::string cmd_result = execute_command_to_string (cmd, from_tty);
> +      std::string cmd_result = execute_command_to_string
> +	(cmd, from_tty, gdb_stdout->term_out ());
>        if (!flags.silent || cmd_result.length () > 0)
>  	{
>  	  if (!flags.quiet)
> diff --git a/gdb/top.c b/gdb/top.c
> index 4065df7081..f21d9b5e43 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -657,7 +657,8 @@ execute_command (const char *p, int from_tty)
>     temporarily set to true.  */
>  
>  std::string
> -execute_command_to_string (const char *p, int from_tty)
> +execute_command_to_string (const char *p, int from_tty,
> +			   bool term_out)
>  {
>    /* GDB_STDOUT should be better already restored during these
>       restoration callbacks.  */
> @@ -665,7 +666,7 @@ execute_command_to_string (const char *p, int from_tty)
>  
>    scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
>  
> -  string_file str_file;
> +  string_file str_file (term_out);
>  
>    {
>      current_uiout->redirect (&str_file);
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 77f6b31ce4..ad7a6df51c 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -101,6 +101,32 @@ ui_file_isatty (struct ui_file *file)
>    return file->isatty ();
>  }
>  
> +/* true if the gdb terminal supports styling, and styling is enabled.  */
> +static bool
> +term_cli_styling (void)
> +{
> +  extern int cli_styling;
> +
> +  if (!cli_styling)
> +    return false;
> +
> +  const char *term = getenv ("TERM");
> +  /* Windows doesn't by default define $TERM, but can support styles
> +     regardless.  */
> +#ifndef _WIN32
> +  if (term == nullptr || !strcmp (term, "dumb"))
> +    return false;
> +#else
> +  /* But if they do define $TERM, let us behave the same as on Posix
> +     platforms, for the benefit of programs which invoke GDB as their
> +     back-end.  */
> +  if (term && !strcmp (term, "dumb"))
> +    return false;
> +#endif
> +  return true;
> +}
> +
> +
>  void
>  ui_file_write (struct ui_file *file,
>  		const char *buf,
> @@ -131,6 +157,16 @@ fputs_unfiltered (const char *buf, struct ui_file *file)
>  
>  
>  
> +string_file::string_file ()
> +{
> +  m_term_out = false;
> +}
> +
> +string_file::string_file (bool term_out)
> +{
> +  m_term_out = term_out;
> +}
> +
>  string_file::~string_file ()
>  {}
>  
> @@ -140,6 +176,18 @@ string_file::write (const char *buf, long length_buf)
>    m_string.append (buf, length_buf);
>  }
>  
> +bool
> +string_file::term_out ()
> +{
> +  return m_term_out;
> +}
> +
> +bool
> +string_file::can_emit_style_escape ()
> +{
> +  return m_term_out && term_cli_styling ();
> +}
> +
>  
>  
>  stdio_file::stdio_file (FILE *file, bool close_p)
> @@ -255,6 +303,14 @@ stdio_file::isatty ()
>    return ::isatty (m_fd);
>  }
>  
> +bool
> +stdio_file::can_emit_style_escape ()
> +{
> +  return this == gdb_stdout
> +    && this->isatty ()
> +    && term_cli_styling ();
> +}
> +
>  
>  
>  /* This is the implementation of ui_file method 'write' for stderr.
> @@ -332,3 +388,17 @@ tee_file::isatty ()
>  {
>    return m_one->isatty ();
>  }
> +
> +bool
> +tee_file::term_out ()
> +{
> +  return m_one->term_out ();
> +}
> +
> +bool
> +tee_file::can_emit_style_escape ()
> +{
> +  return this == gdb_stdout
> +    && m_one->term_out ()
> +    && term_cli_styling ();
> +}
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 6e6ca1c9cd..1a688dc341 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -70,6 +70,14 @@ public:
>    virtual bool isatty ()
>    { return false; }
>  
> +  /* true indicates terminal output behaviour such as cli_styling.  */
> +  virtual bool term_out ()
> +  { return isatty (); }
> +
> +  /* true if ANSI escapes can be used on STREAM.  */
> +  virtual bool can_emit_style_escape ()
> +  { return false; }
> +
>    virtual void flush ()
>    {}
>  };
> @@ -109,7 +117,13 @@ extern int gdb_console_fputs (const char *, FILE *);
>  class string_file : public ui_file
>  {
>  public:
> -  string_file () {}
> +  /* Construct a string_file to collect 'raw' output, i.e. without
> +     'terminal' behaviour such as cli_styling.  */
> +  string_file ();
> +  /* If TERM_OUT, construct a string_file with terminal output behaviour
> +     such as cli_styling)
> +     else collect 'raw' output like the previous constructor.  */
> +  string_file (bool term_out);
>    ~string_file () override;
>  
>    /* Override ui_file methods.  */
> @@ -119,6 +133,9 @@ public:
>    long read (char *buf, long length_buf) override
>    { gdb_assert_not_reached ("a string_file is not readable"); }
>  
> +  bool term_out () override;
> +  bool can_emit_style_escape () override;
> +
>    /* string_file-specific public API.  */
>  
>    /* Accesses the std::string containing the entire output collected
> @@ -145,6 +162,8 @@ public:
>  private:
>    /* The internal buffer.  */
>    std::string m_string;
> +
> +  bool m_term_out;
>  };
>  
>  /* A ui_file implementation that maps directly onto <stdio.h>'s FILE.
> @@ -183,6 +202,8 @@ public:
>  
>    bool isatty () override;
>  
> +  bool can_emit_style_escape () override;
> +
>  private:
>    /* Sets the internal stream to FILE, and saves the FILE's file
>       descriptor in M_FD.  */
> @@ -255,6 +276,8 @@ public:
>    void puts (const char *) override;
>  
>    bool isatty () override;
> +  bool term_out () override;
> +  bool can_emit_style_escape () override;
>    void flush () override;
>  
>  private:
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 840779a630..e35fa5798c 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1435,38 +1435,13 @@ emit_style_escape (const ui_file_style &style,
>      fputs_unfiltered (style.to_ansi ().c_str (), stream);
>  }
>  
> -/* See utils.h.  */
> -
> -bool
> -can_emit_style_escape (struct ui_file *stream)
> -{
> -  if (stream != gdb_stdout
> -      || !cli_styling
> -      || !ui_file_isatty (stream))
> -    return false;
> -  const char *term = getenv ("TERM");
> -  /* Windows doesn't by default define $TERM, but can support styles
> -     regardless.  */
> -#ifndef _WIN32
> -  if (term == nullptr || !strcmp (term, "dumb"))
> -    return false;
> -#else
> -  /* But if they do define $TERM, let us behave the same as on Posix
> -     platforms, for the benefit of programs which invoke GDB as their
> -     back-end.  */
> -  if (term && !strcmp (term, "dumb"))
> -    return false;
> -#endif
> -  return true;
> -}
> -
>  /* Set the current output style.  This will affect future uses of the
>     _filtered output functions.  */
>  
>  static void
>  set_output_style (struct ui_file *stream, const ui_file_style &style)
>  {
> -  if (!can_emit_style_escape (stream))
> +  if (!stream->can_emit_style_escape ())
>      return;
>  
>    /* Note that we don't pass STREAM here, because we want to emit to
> @@ -1479,7 +1454,7 @@ set_output_style (struct ui_file *stream, const ui_file_style &style)
>  void
>  reset_terminal_style (struct ui_file *stream)
>  {
> -  if (can_emit_style_escape (stream))
> +  if (stream->can_emit_style_escape ())
>      {
>        /* Force the setting, regardless of what we think the setting
>  	 might already be.  */
> @@ -1504,7 +1479,7 @@ prompt_for_continue (void)
>    bool disable_pagination = pagination_disabled_for_command;
>  
>    /* Clear the current styling.  */
> -  if (can_emit_style_escape (gdb_stdout))
> +  if (gdb_stdout->can_emit_style_escape ())
>      emit_style_escape (ui_file_style (), gdb_stdout);
>  
>    if (annotation_level > 1)
> @@ -1552,7 +1527,7 @@ prompt_for_continue (void)
>    pagination_disabled_for_command = disable_pagination;
>  
>    /* Restore the current styling.  */
> -  if (can_emit_style_escape (gdb_stdout))
> +  if (gdb_stdout->can_emit_style_escape ())
>      emit_style_escape (applied_style);
>  
>    dont_repeat ();		/* Forget prev cmd -- CR won't repeat it.  */
> @@ -1805,7 +1780,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
>  	      lines_printed++;
>  	      if (wrap_column)
>  		{
> -		  if (can_emit_style_escape (stream))
> +		  if (stream->can_emit_style_escape ())
>  		    emit_style_escape (ui_file_style (), stream);
>  		  /* If we aren't actually wrapping, don't output
>  		     newline -- if chars_per_line is right, we
> @@ -1827,7 +1802,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
>  	      if (wrap_column)
>  		{
>  		  fputs_unfiltered (wrap_indent, stream);
> -		  if (can_emit_style_escape (stream))
> +		  if (stream->can_emit_style_escape ())
>  		    emit_style_escape (wrap_style, stream);
>  		  /* FIXME, this strlen is what prevents wrap_indent from
>  		     containing tabs.  However, if we recurse to print it
> diff --git a/gdb/utils.h b/gdb/utils.h
> index f0cb48e7a5..76c10049a7 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -440,10 +440,6 @@ extern void fputs_styled (const char *linebuffer,
>  
>  extern void reset_terminal_style (struct ui_file *stream);
>  
> -/* Return true if ANSI escapes can be used on STREAM.  */
> -
> -extern bool can_emit_style_escape (struct ui_file *stream);
> -
>  /* Display the host ADDR on STREAM formatted as ``0x%x''.  */
>  extern void gdb_print_host_address_1 (const void *addr, struct ui_file *stream);
>