gdb: don't print escape characters when a style is disabled

While working on another patch I noticed that if I disable a single
style with, for example:

  set style filename background none
  set style filename foreground none
  set style filename intensity normal

Then in some places escape characters are still injected into the
output stream.  This is a bit of an edge case, and I can't think when
this would actually cause problems, but it still felt like a bit of an
annoyance.

One place where this does impact is in testing, where it becomes
harder to write tight test patterns if it is not obvious when GDB will
decide to inject escape sequences.

It's especially annoying because depending on how something is printed
then GDB might, or might not, add escape characters.  So this would
not add escape characters if the filename style was disabled:

  fprintf_filtered (file, "%ps",
                    styled_string (file_name_style.style (),
                                   "This is a test"));

But this would add escape characters:

  fprintf_styled (file, file_name_style.style (),
                  "%s", "This is a test");

I tracked this down to some calls to set_output_style in utils.c.
Currently some calls to set_output_style (in utils.c) are guarded like
this:

  if (!STYLE.is_default ())
    set_output_style (stream, STYLE);

But some calls are not.  It is the calls that are NOT guarded that
cause the extra escape sequences to be emitted.

My initial proposal to resolve this issue was simply to ensure that
all calls to set_output_style were guarded.  The patch I posted for
this can be found here:

  https://sourceware.org/pipermail/gdb-patches/2021-January/175096.html

The feedback on this proposal was that it might be better to guard
against the escape sequences being emitted at a later lever, right
down at emit_style_escape.

So this is what this version does.  In emit_style_escape we already
track the currently applied style, so if the style we are being asked
to switch to is the same as the currently applied style then no escape
sequence needs to be emitted.

Making this change immediately exposed some issues in
fputs_maybe_filtered related to line wrapping.  The best place to start
to understand what's going on with the styling and wrapping is look at
the test:

  gdb.base/style.exp: all styles enabled: frame when width=20

If you run this test and then examine the output in an editor so the
escape sequences can be seen you'll see the duplicate escape sequences
that are emitted before this patch, the compare to after this patch
you'll see the set of escape sequences should be the minimum required.

In order to test these changes I have rewritten the gdb.base/style.exp
test script.  The core of the script is now run multiple times.  The
first time the test is run things are as they were before, all styles
are on.

After that the test is rerun multiple times.  Each time through a
single style is disabled using the 3 explicit set calls listed above.
I then repeat all the tests, however, I arrange so that the patterns
for the disabled style now require no escape sequences.

gdb/ChangeLog:

	* utils.c (emit_style_escape): Only emit an escape sequence if the
	requested style is different than the current applied style.
	(fputs_maybe_filtered): Adjust the juggling of the wrap_style, and
	current applied_style.
	(fputs_styled): Remove is_default check.
	(fputs_styled_unfiltered): Likewise.
	(vfprintf_styled_no_gdbfmt): Likewise.

gdb/testsuite/ChangeLog:

	* gdb.base/style.exp (limited_style): New proc.
	(clean_restart_and_disable): New proc.
	(run_style_tests): New proc.  Most of the old tests from this file
	are now in this proc.
	(test_startup_version_string): New proc.  Reamining test from the
	old file is in this proc.

1 files changed, 37 insertions, 36 deletions

diff --git a/gdb/utils.c b/gdb/utils.c
index 414e7b1..b9f8997 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1426,12 +1426,15 @@ static void
 emit_style_escape (const ui_file_style &style,
 		   struct ui_file *stream = nullptr)
 {
-  applied_style = style;
+  if (applied_style != style)
+    {
+      applied_style = style;
 
-  if (stream == nullptr)
-    wrap_buffer.append (style.to_ansi ());
-  else
-    stream->puts (style.to_ansi ().c_str ());
+      if (stream == nullptr)
+	wrap_buffer.append (style.to_ansi ());
+      else
+	stream->puts (style.to_ansi ().c_str ());
+    }
 }
 
 /* Set the current output style.  This will affect future uses of the
@@ -1800,14 +1803,20 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
 		 prompt is given; and to avoid emitting style
 		 sequences in the middle of a run of text, we track
 		 this as well.  */
-	      ui_file_style save_style;
+	      ui_file_style save_style = applied_style;
 	      bool did_paginate = false;
 
 	      chars_printed = 0;
 	      lines_printed++;
 	      if (wrap_column)
 		{
-		  save_style = wrap_style;
+		  /* We are about to insert a newline at an historic
+		     location in the WRAP_BUFFER.  Before we do we want to
+		     restore the default style.  To know if we actually
+		     need to insert an escape sequence we must restore the
+		     current applied style to how it was at the WRAP_COLUMN
+		     location.  */
+		  applied_style = wrap_style;
 		  if (stream->can_emit_style_escape ())
 		    emit_style_escape (ui_file_style (), stream);
 		  /* If we aren't actually wrapping, don't output
@@ -1822,10 +1831,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
 		  stream->puts ("\n");
 		}
 	      else
-		{
-		  save_style = applied_style;
-		  flush_wrap_buffer (stream);
-		}
+		flush_wrap_buffer (stream);
 
 	      /* Possible new page.  Note that
 		 PAGINATION_DISABLED_FOR_COMMAND might be set during
@@ -1841,8 +1847,19 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
 	      if (wrap_column)
 		{
 		  stream->puts (wrap_indent);
+
+		  /* Having finished inserting the wrapping we should
+		     restore the style as it was at the WRAP_COLUMN.  */
 		  if (stream->can_emit_style_escape ())
-		    emit_style_escape (save_style, stream);
+		    emit_style_escape (wrap_style, stream);
+
+		  /* The WRAP_BUFFER will still contain content, and that
+		     content might set some alternative style.  Restore
+		     APPLIED_STYLE as it was before we started wrapping,
+		     this reflects the current style for the last character
+		     in WRAP_BUFFER.  */
+		  applied_style = save_style;
+
 		  /* FIXME, this strlen is what prevents wrap_indent from
 		     containing tabs.  However, if we recurse to print it
 		     and count its chars, we risk trouble if wrap_indent is
@@ -1895,16 +1912,9 @@ void
 fputs_styled (const char *linebuffer, const ui_file_style &style,
 	      struct ui_file *stream)
 {
-  /* This just makes it so we emit somewhat fewer escape
-     sequences.  */
-  if (style.is_default ())
-    fputs_maybe_filtered (linebuffer, stream, 1);
-  else
-    {
-      set_output_style (stream, style);
-      fputs_maybe_filtered (linebuffer, stream, 1);
-      set_output_style (stream, ui_file_style ());
-    }
+  set_output_style (stream, style);
+  fputs_maybe_filtered (linebuffer, stream, 1);
+  set_output_style (stream, ui_file_style ());
 }
 
 /* See utils.h.  */
@@ -1913,16 +1923,9 @@ void
 fputs_styled_unfiltered (const char *linebuffer, const ui_file_style &style,
 			 struct ui_file *stream)
 {
-  /* This just makes it so we emit somewhat fewer escape
-     sequences.  */
-  if (style.is_default ())
-    fputs_maybe_filtered (linebuffer, stream, 0);
-  else
-    {
-      set_output_style (stream, style);
-      fputs_maybe_filtered (linebuffer, stream, 0);
-      set_output_style (stream, ui_file_style ());
-    }
+  set_output_style (stream, style);
+  fputs_maybe_filtered (linebuffer, stream, 0);
+  set_output_style (stream, ui_file_style ());
 }
 
 /* See utils.h.  */
@@ -2222,11 +2225,9 @@ vfprintf_styled_no_gdbfmt (struct ui_file *stream, const ui_file_style &style,
   std::string str = string_vprintf (format, args);
   if (!str.empty ())
     {
-      if (!style.is_default ())
-	set_output_style (stream, style);
+      set_output_style (stream, style);
       fputs_maybe_filtered (str.c_str (), stream, filter);
-      if (!style.is_default ())
-	set_output_style (stream, ui_file_style ());
+      set_output_style (stream, ui_file_style ());
     }
 }