Remove m_applied_style from ui_file

While working on this series, I found a number of odd styling issues
recurred.  For instance, the issue where the pager would lose track
and style subsequent output incorrectly reappeared.

It turned out that different ui_file objects in the output pipeline
would get confused about their current style.  And, looking deeper at
this, I realized that mainly it is the pager that really needs to
track the current style at all.  All the other file implementations
can be purely reactive (except the buffered stream code, as Andrew
pointed out).

This patch moves m_applied_style from ui_file and into pager_file.
This necessitated making ui_file::vprintf virtual, so that the base
class could pass in the "plain" style as the starting point, whereas
the pager could use the applied style.  (I did not investigate whether
this was truly necessary, and I somewhat suspect it might not be.)

This straightforward approach caused some regressions, mostly
involving extra ANSI escapes being emitted.  I fixed most of these by
arranging for ui_out::call_do_message to track styles a little more
thoroughly.

Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>

diff --git a/gdb/buffered-streams.h b/gdb/buffered-streams.h
index 9da45b08b6b4eaa1a1c1cf14d0a210f7e7573676..62598e70ca0095ff7edb8b61d808a3dc4cb40fbe 100644 (file)
--- a/gdb/buffered-streams.h
+++ b/gdb/buffered-streams.h
@@ -154,6 +154,15 @@ struct buffering_file : public ui_file
     return m_stream->can_emit_style_escape ();
   }
 
+  void emit_style_escape (const ui_file_style &style) override
+  {
+    if (can_emit_style_escape () && style != m_applied_style)
+      {
+       m_applied_style = style;
+       ui_file::emit_style_escape (style);
+      }
+  }
+
   /* Flush the underlying output stream.  */
   void flush () override
   {
@@ -167,6 +176,9 @@ private:
 
   /* The underlying output stream.  */
   ui_file *m_stream;
+
+  /* The currently applied style.  */
+  ui_file_style m_applied_style;
 };
 
 /* Attaches and detaches buffers for each of the gdb_std* streams.  */

diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index b1ea9560fcf6f45b7513d7bfb6f39cbdc64bda15..d8ac13ab82774a6319cc741287ce75611f697bec 100644 (file)
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -221,20 +221,24 @@ cli_ui_out::do_text (const char *string)
 }
 
 void
-cli_ui_out::do_message (const ui_file_style &style,
+cli_ui_out::do_message (ui_file_style &current_style,
+                       const ui_file_style &style,
                        const char *format, va_list args)
 {
   if (m_suppress_output)
     return;
 
   std::string str = string_vprintf (format, args);
-  if (!str.empty ())
+  if (str.empty ())
+    return;
+
+  ui_file *stream = m_streams.back ();
+  if (current_style != style)
     {
-      ui_file *stream = m_streams.back ();
       stream->emit_style_escape (style);
-      stream->puts (str.c_str ());
-      stream->emit_style_escape (ui_file_style ());
+      current_style = style;
     }
+  stream->puts (str.c_str ());
 }
 
 void
@@ -489,6 +493,12 @@ cli_ui_out::can_emit_style_escape () const
   return m_streams.back ()->can_emit_style_escape ();
 }
 
+void
+cli_ui_out::emit_style_escape (const ui_file_style &style)
+{
+  m_streams.back ()->emit_style_escape (style);
+}
+
 /* CLI interface to display tab-completion matches.  */
 
 /* CLI version of displayer.crlf.  */

diff --git a/gdb/cli-out.h b/gdb/cli-out.h
index b34601f0f4664a6bae33506519ab23eadf049dad..74307074a7c96527db2f3a87d36c200e8165eb13 100644 (file)
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -35,6 +35,8 @@ public:
 
   bool can_emit_style_escape () const override;
 
+  void emit_style_escape (const ui_file_style &style) override;
+
   ui_file *current_stream () const override
   { return m_streams.back (); }
 
@@ -69,9 +71,10 @@ protected:
     override ATTRIBUTE_PRINTF (7, 0);
   virtual void do_spaces (int numspaces) override;
   virtual void do_text (const char *string) override;
-  virtual void do_message (const ui_file_style &style,
+  virtual void do_message (ui_file_style &current_style,
+                          const ui_file_style &style,
                           const char *format, va_list args) override
-    ATTRIBUTE_PRINTF (3,0);
+    ATTRIBUTE_PRINTF (4, 0);
   virtual void do_wrap_hint (int indent) override;
   virtual void do_flush () override;
   virtual void do_redirect (struct ui_file *outstream) override;

diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index aac00ae7f761cee3292e7d2763976ef07a963929..41b0ee8cdbd1c3bb57bc034d1e912d517ab7bef4 100644 (file)
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -167,7 +167,8 @@ mi_ui_out::do_text (const char *string)
 }
 
 void
-mi_ui_out::do_message (const ui_file_style &style,
+mi_ui_out::do_message (ui_file_style &current_style,
+                      const ui_file_style &style,
                       const char *format, va_list args)
 {
 }

diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index ee70659abd3b7847a7bc7dc2b1fdbd0bd7fc953c..d1d26718ac48146c3c61e09b07cea14b15a3d187 100644 (file)
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -78,9 +78,10 @@ protected:
     override ATTRIBUTE_PRINTF (7,0);
   virtual void do_spaces (int numspaces) override;
   virtual void do_text (const char *string) override;
-  virtual void do_message (const ui_file_style &style,
+  virtual void do_message (ui_file_style &current_style,
+                          const ui_file_style &style,
                           const char *format, va_list args) override
-    ATTRIBUTE_PRINTF (3,0);
+    ATTRIBUTE_PRINTF (4, 0);
   virtual void do_wrap_hint (int indent) override;
   virtual void do_flush () override;
   virtual void do_redirect (struct ui_file *outstream) override;

diff --git a/gdb/pager.h b/gdb/pager.h
index 697134be80512250100c93b40bfec4b502ae3bed..114024d5e5f3db6ce69d52b78263d01c475f982e 100644 (file)
--- a/gdb/pager.h
+++ b/gdb/pager.h
@@ -51,6 +51,9 @@ public:
     m_stream->puts_unfiltered (str);
   }
 
+  void vprintf (const char *fmt, va_list args) override
+    ATTRIBUTE_PRINTF (2, 0);
+
 private:
 
   void prompt_for_continue ();
@@ -92,6 +95,9 @@ private:
      wrapping is not in effect.  */
   int m_wrap_column = 0;
 
+  /* The currently applied style.  */
+  ui_file_style m_applied_style;
+
   /* The style applied at the time that wrap_here was called.  */
   ui_file_style m_wrap_style;
 

diff --git a/gdb/python/py-uiout.h b/gdb/python/py-uiout.h
index 159f1b22e467a1a2fcc3024e9cc1e91f9bb79f5b..21c068e90778f4c2568ebcde3c660f88d2e6f489 100644 (file)
--- a/gdb/python/py-uiout.h
+++ b/gdb/python/py-uiout.h
@@ -109,9 +109,10 @@ protected:
   void do_text (const char *string) override
   { }
 
-  void do_message (const ui_file_style &style,
+  void do_message (ui_file_style &current_style,
+                  const ui_file_style &style,
                   const char *format, va_list args)
-    override ATTRIBUTE_PRINTF (3,0)
+    override ATTRIBUTE_PRINTF (4, 0)
   { }
 
   void do_wrap_hint (int indent) override

diff --git a/gdb/testsuite/gdb.python/py-styled-execute.exp b/gdb/testsuite/gdb.python/py-styled-execute.exp
index afaaf928573c819790d247ee795d823c0af64474..efc65d3be507c8ef2d0a27277f154b1ba87c5b0c 100644 (file)
--- a/gdb/testsuite/gdb.python/py-styled-execute.exp
+++ b/gdb/testsuite/gdb.python/py-styled-execute.exp
@@ -60,8 +60,7 @@ proc test_gdb_execute_styling {} {
     # Two possible outputs, BASIC_RE, the unstyled output text, or
     # STYLED_RE, the same things, but with styling applied.
     set text "\"version\" style"
-    set styled_text \
-       [style "\"" version][style "version" version][style "\" style" version]
+    set styled_text [style $text version]
     set basic_re "The $text foreground color is: \[^\r\n\]+"
     set styled_re "The $styled_text foreground color is: \[^\r\n\]+"
 

diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index c5931deaf2093ab217d35587e75c9dd047ec1325..d655e7edd9e00ea37f9b035799e2521a88c56b94 100644 (file)
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -70,7 +70,7 @@ void
 ui_file::vprintf (const char *format, va_list args)
 {
   ui_out_flags flags = disallow_ui_out_field;
-  cli_ui_out (this, flags).vmessage (m_applied_style, format, args);
+  cli_ui_out (this, flags).vmessage ({}, format, args);
 }
 
 /* See ui-file.h.  */
@@ -78,11 +78,8 @@ ui_file::vprintf (const char *format, va_list args)
 void
 ui_file::emit_style_escape (const ui_file_style &style)
 {
-  if (can_emit_style_escape () && style != m_applied_style)
-    {
-      m_applied_style = style;
-      this->puts (style.to_ansi ().c_str ());
-    }
+  if (can_emit_style_escape ())
+    this->puts (style.to_ansi ().c_str ());
 }
 
 /* See ui-file.h.  */

diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 4aaf4d0e54e3d34ef4759f5883d1676d2f6d6aa0..84529de36186bf7d729c3dc98ed566d5cbf61e0f 100644 (file)
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -55,7 +55,7 @@ public:
 
   void putc (int c);
 
-  void vprintf (const char *, va_list) ATTRIBUTE_PRINTF (2, 0);
+  virtual void vprintf (const char *, va_list) ATTRIBUTE_PRINTF (2, 0);
 
   /* Methods below are both public, and overridable by ui_file
      subclasses.  */
@@ -139,11 +139,6 @@ public:
     this->puts (str);
   }
 
-protected:
-
-  /* The currently applied style.  */
-  ui_file_style m_applied_style;
-
 private:
 
   /* Helper function for putstr and putstrn.  Print the character C on

diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 8a41d9897fa890598186d24475a4f8a5f8642efe..ac792b439050b1b51fb248d29ff18760844e33a3 100644 (file)
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -559,7 +559,8 @@ ui_out::field_fmt (const char *fldname, const ui_file_style &style,
 }
 
 void
-ui_out::call_do_message (const ui_file_style &style, const char *format,
+ui_out::call_do_message (ui_file_style &current_style,
+                        const ui_file_style &style, const char *format,
                         ...)
 {
   va_list args;
@@ -571,7 +572,7 @@ ui_out::call_do_message (const ui_file_style &style, const char *format,
      to put a "format" attribute on call_do_message.  */
   DIAGNOSTIC_PUSH
   DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
-  do_message (style, format, args);
+  do_message (current_style, style, format, args);
   DIAGNOSTIC_POP
 
   va_end (args);
@@ -583,6 +584,7 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
 {
   format_pieces fpieces (&format, true);
 
+  ui_file_style current_style = in_style;
   ui_file_style style = in_style;
 
   for (auto &&piece : fpieces)
@@ -608,13 +610,15 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
            switch (piece.n_int_args)
              {
              case 0:
-               call_do_message (style, current_substring, str);
+               call_do_message (current_style, style, current_substring,
+                                str);
                break;
              case 1:
-               call_do_message (style, current_substring, intvals[0], str);
+               call_do_message (current_style, style, current_substring,
+                                intvals[0], str);
                break;
              case 2:
-               call_do_message (style, current_substring,
+               call_do_message (current_style, style, current_substring,
                                 intvals[0], intvals[1], str);
                break;
              }
@@ -627,7 +631,8 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
          gdb_assert_not_reached ("wide_char_arg not supported in vmessage");
          break;
        case long_long_arg:
-         call_do_message (style, current_substring, va_arg (args, long long));
+         call_do_message (current_style, style, current_substring,
+                          va_arg (args, long long));
          break;
        case int_arg:
          {
@@ -635,13 +640,15 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
            switch (piece.n_int_args)
              {
              case 0:
-               call_do_message (style, current_substring, val);
+               call_do_message (current_style, style, current_substring,
+                                val);
                break;
              case 1:
-               call_do_message (style, current_substring, intvals[0], val);
+               call_do_message (current_style, style, current_substring,
+                                intvals[0], val);
                break;
              case 2:
-               call_do_message (style, current_substring,
+               call_do_message (current_style, style, current_substring,
                                 intvals[0], intvals[1], val);
                break;
              }
@@ -653,13 +660,15 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
            switch (piece.n_int_args)
              {
              case 0:
-               call_do_message (style, current_substring, val);
+               call_do_message (current_style, style, current_substring,
+                                val);
                break;
              case 1:
-               call_do_message (style, current_substring, intvals[0], val);
+               call_do_message (current_style, style, current_substring,
+                                intvals[0], val);
                break;
              case 2:
-               call_do_message (style, current_substring,
+               call_do_message (current_style, style, current_substring,
                                 intvals[0], intvals[1], val);
                break;
              }
@@ -671,13 +680,15 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
            switch (piece.n_int_args)
              {
              case 0:
-               call_do_message (style, current_substring, val);
+               call_do_message (current_style, style, current_substring,
+                                val);
                break;
              case 1:
-               call_do_message (style, current_substring, intvals[0], val);
+               call_do_message (current_style, style, current_substring,
+                                intvals[0], val);
                break;
              case 2:
-               call_do_message (style, current_substring,
+               call_do_message (current_style, style, current_substring,
                                 intvals[0], intvals[1], val);
                break;
              }
@@ -689,20 +700,23 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
            switch (piece.n_int_args)
              {
              case 0:
-               call_do_message (style, current_substring, val);
+               call_do_message (current_style, style, current_substring,
+                                val);
                break;
              case 1:
-               call_do_message (style, current_substring, intvals[0], val);
+               call_do_message (current_style, style, current_substring,
+                                intvals[0], val);
                break;
              case 2:
-               call_do_message (style, current_substring,
+               call_do_message (current_style, style, current_substring,
                                 intvals[0], intvals[1], val);
                break;
              }
          }
          break;
        case double_arg:
-         call_do_message (style, current_substring, va_arg (args, double));
+         call_do_message (current_style, style, current_substring,
+                          va_arg (args, double));
          break;
        case long_double_arg:
          gdb_assert_not_reached ("long_double_arg not supported in vmessage");
@@ -743,7 +757,7 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
            case 's':
              {
                styled_string_s *ss = va_arg (args, styled_string_s *);
-               call_do_message (ss->style, "%s", ss->str);
+               call_do_message (current_style, ss->style, "%s", ss->str);
              }
              break;
            case '[':
@@ -758,7 +772,8 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
              }
              break;
            default:
-             call_do_message (style, current_substring, va_arg (args, void *));
+             call_do_message (current_style, style, current_substring,
+                              va_arg (args, void *));
              break;
            }
          break;
@@ -770,12 +785,16 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format,
             because some platforms have modified GCC to include
             -Wformat-security by default, which will warn here if
             there is no argument.  */
-         call_do_message (style, current_substring, 0);
+         call_do_message (current_style, style, current_substring, 0);
          break;
        default:
          internal_error (_("failed internal consistency check"));
        }
     }
+
+  ui_file_style plain;
+  if (can_emit_style_escape () && current_style != plain)
+    emit_style_escape (plain);
 }
 
 void

diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 69d9910443ecdc9ea8273dffc2ce98873fa5e5c5..28af8d521e77c52e015c2fe94fd4e7542d2b6ed8 100644 (file)
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -282,6 +282,11 @@ class ui_out
      escapes.  */
   virtual bool can_emit_style_escape () const = 0;
 
+  /* Emit a style escape, if possible.  */
+  virtual void emit_style_escape (const ui_file_style &style)
+  {
+  }
+
   /* Return the ui_file currently used for output.  */
   virtual ui_file *current_stream () const = 0;
 
@@ -361,9 +366,17 @@ protected:
     ATTRIBUTE_PRINTF (7, 0) = 0;
   virtual void do_spaces (int numspaces) = 0;
   virtual void do_text (const char *string) = 0;
-  virtual void do_message (const ui_file_style &style,
+
+  /* A helper for vprintf and call_do_message.  Formats a string and
+     then prints it using STYLE.  This should take care to only change
+     the style when necessary (i.e., don't bother if the formatted
+     string is empty, or if the desired style is the same as
+     CURRENT_STYLE).  Updates current_style if the style was
+     changed.  */
+  virtual void do_message (ui_file_style &current_style,
+                          const ui_file_style &style,
                           const char *format, va_list args)
-    ATTRIBUTE_PRINTF (3,0) = 0;
+    ATTRIBUTE_PRINTF (4, 0) = 0;
   virtual void do_wrap_hint (int indent) = 0;
   virtual void do_flush () = 0;
   virtual void do_redirect (struct ui_file *outstream) = 0;
@@ -380,7 +393,10 @@ protected:
   { return false; }
 
  private:
-  void call_do_message (const ui_file_style &style, const char *format,
+  /* A helper for vmessage that wraps a call to do_message.  This will
+     update CURRENT_STYLE when needed.  */
+  void call_do_message (ui_file_style &current_style,
+                       const ui_file_style &style, const char *format,
                        ...);
 
   ui_out_flags m_flags;

diff --git a/gdb/utils.c b/gdb/utils.c
index 2f8bf5551c5b948442930c29106f8eaff78e476e..061161b1f3a35e56da311c1bfabc3e25a87602c2 100644 (file)
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1673,6 +1673,13 @@ pager_file::check_for_overfull_line (const unsigned int lines_allowed)
     }
 }
 
+void
+pager_file::vprintf (const char *format, va_list args)
+{
+  ui_out_flags flags = disallow_ui_out_field;
+  cli_ui_out (this, flags).vmessage (m_applied_style, format, args);
+}
+
 void
 pager_file::puts (const char *linebuffer)
 {