[PATCH v3 13/21] Remove m_applied_style from ui_file

[Tom Tromey <tom@tromey.com>]

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>
---
 gdb/buffered-streams.h                         | 12 +++++
 gdb/cli-out.c                                  | 20 ++++++--
 gdb/cli-out.h                                  |  7 ++-
 gdb/mi/mi-out.c                                |  3 +-
 gdb/mi/mi-out.h                                |  5 +-
 gdb/pager.h                                    |  6 +++
 gdb/python/py-uiout.h                          |  5 +-
 gdb/testsuite/gdb.python/py-styled-execute.exp |  3 +-
 gdb/ui-file.c                                  |  9 ++--
 gdb/ui-file.h                                  |  7 +--
 gdb/ui-out.c                                   | 63 +++++++++++++++++---------
 gdb/ui-out.h                                   | 22 +++++++--
 gdb/utils.c                                    |  7 +++
 13 files changed, 118 insertions(+), 51 deletions(-)

diff --git a/gdb/buffered-streams.h b/gdb/buffered-streams.h
index 9da45b08b6b..62598e70ca0 100644
--- 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 @@ struct buffering_file : public ui_file
 
   /* 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 b1ea9560fcf..d8ac13ab827 100644
--- 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 b34601f0f46..74307074a7c 100644
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -35,6 +35,8 @@ class cli_ui_out : public ui_out
 
   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 @@ class cli_ui_out : public ui_out
     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 aac00ae7f76..41b0ee8cdbd 100644
--- 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 ee70659abd3..d1d26718ac4 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -78,9 +78,10 @@ class mi_ui_out : public ui_out
     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 697134be805..114024d5e5f 100644
--- a/gdb/pager.h
+++ b/gdb/pager.h
@@ -51,6 +51,9 @@ class pager_file : public wrapped_file<ui_file_up>
     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 @@ class pager_file : public wrapped_file<ui_file_up>
      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 159f1b22e46..21c068e9077 100644
--- a/gdb/python/py-uiout.h
+++ b/gdb/python/py-uiout.h
@@ -109,9 +109,10 @@ class py_ui_out : public ui_out
   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 afaaf928573..efc65d3be50 100644
--- 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 c5931deaf20..d655e7edd9e 100644
--- 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 4aaf4d0e54e..84529de3618 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -55,7 +55,7 @@ class ui_file
 
   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 @@ class ui_file
     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 8a41d9897fa..ac792b43905 100644
--- 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 69d9910443e..28af8d521e7 100644
--- 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 @@ class ui_out
     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 @@ class ui_out
   { 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 03a1d9146f2..bec749afc40 100644
--- 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)
 {

-- 
2.49.0