Re: [PATCH] gdb: make timestamped_file implement can_emit_style_escape

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2022-04-05 10:25, Tom Tromey wrote:
> Simon> I'm thinking that isatty and flush should be implemented, since their
> Simon> default behavior from ui_file is probably not adequate?
>
> How about the appended?
>
> Tom
>
> commit b490690480b50918b1b4f5533e6391e9fb1e8072
> Author: Tom Tromey <tom@tromey.com>
> Date:   Tue Apr 5 07:44:59 2022 -0600
>
>     Introduce wrapped_file
>
>     Simon pointed out that timestamped_file probably needed to implement a
>     few more methods.  This patch introduces a new file-wrapping file that
>     forwards most of its calls, making it simpler to implement new such
>     files.  It also converts timestamped_file and pager_file to use it.

Nice, I thought of doing something like this but didn't have time to.

One formatting nit below.

>
>     Regression tested on x86-64 Fedora 34.
>
> diff --git a/gdb/pager.h b/gdb/pager.h
> index 0151a283629..d2a3a2b8ec8 100644
> --- a/gdb/pager.h
> +++ b/gdb/pager.h
> @@ -23,16 +23,21 @@
>
>  /* A ui_file that implements output paging and unfiltered output.  */
>
> -class pager_file : public ui_file
> +class pager_file : public wrapped_file
>  {
>  public:
>    /* Create a new pager_file.  The new object takes ownership of
>       STREAM.  */
>    explicit pager_file (ui_file *stream)
> -    : m_stream (stream)
> +    : wrapped_file (stream)
>    {
>    }
>
> +  ~pager_file ()
> +  {
> +    delete m_stream;
> +  }
> +
>    DISABLE_COPY_AND_ASSIGN (pager_file);
>
>    void write (const char *buf, long length_buf) override;
> @@ -44,31 +49,11 @@ class pager_file : public ui_file
>      m_stream->write_async_safe (buf, length_buf);
>    }
>
> -  bool term_out () override
> -  {
> -    return m_stream->term_out ();
> -  }
> -
> -  bool isatty () override
> -  {
> -    return m_stream->isatty ();
> -  }
> -
> -  bool can_emit_style_escape () override
> -  {
> -    return m_stream->can_emit_style_escape ();
> -  }
> -
>    void emit_style_escape (const ui_file_style &style) override;
>    void reset_style () override;
>
>    void flush () override;
>
> -  int fd () const override
> -  {
> -    return m_stream->fd ();
> -  }
> -
>    void wrap_here (int indent) override;
>
>    void puts_unfiltered (const char *str) override
> @@ -98,9 +83,6 @@ class pager_file : public ui_file
>    /* The style applied at the time that wrap_here was called.  */
>    ui_file_style m_wrap_style;
>
> -  /* The unfiltered output stream.  */
> -  ui_file_up m_stream;
> -
>    /* This is temporarily set when paging.  This will cause some
>       methods to change their behavior to ignore the wrap buffer.  */
>    bool m_paging = false;
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index bffdeaa28c4..3af769e7ae6 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -391,32 +391,92 @@ class no_terminal_escape_file : public stdio_file
>    }
>  };
>
> -/* A ui_file that optionally puts a timestamp at the start of each
> -   line of output.  */
> +/* A base class for ui_file types that that wrap another ui_file.  */
>
> -class timestamped_file : public ui_file
> +class wrapped_file :  public ui_file
>  {
>  public:
> -  explicit timestamped_file (ui_file *stream)
> -    : m_stream (stream)
> +
> +  bool isatty () override
> +  {
> +    return m_stream->isatty ();
> +  }
> +
> +  bool term_out () override
>    {
> +    return m_stream->term_out ();
>    }
>
>    bool can_emit_style_escape () override
> -  { return m_stream->can_emit_style_escape (); }
> +  {
> +    return m_stream->can_emit_style_escape ();
> +  }
> +
> +  void flush () override
> +  {
> +    m_stream->flush ();
> +  }
> +
> +  void wrap_here (int indent) override
> +  {
> +    m_stream->wrap_here (indent);
> +  }
> +
> +  void emit_style_escape (const ui_file_style &style) override
> +  {
> +    m_stream->emit_style_escape (style);
> +  }
> +
> +  /* Rest the current output style to the empty style.  */
> +  void reset_style () override
> +  {
> +    m_stream->reset_style ();
> +  }
> +
> +  int fd () const override
> +  {
> +    return m_stream->fd ();
> +  }
> +
> +  void puts_unfiltered (const char *str) override
> +  {
> +    m_stream->puts_unfiltered (str);
> +  }
>
>    void write_async_safe (const char *buf, long length_buf) override
>    { return m_stream->write_async_safe (buf, length_buf); }

The formatting (braces placement) of write_async_safe clashes with the
rest.  Can you make it uniform?  I do like the short version with the
braces on the same line, for simple one-liners, but I don't really mind,
as long as it's consistent.

Simon