Fix pager bugs with style output

I believe this fixes all the pager output problems with styling that
Philippe pointed out, plus at least one more.  The patch is somewhat
hard to reason about, so you may wish to give it a try.  Even writing
the tests was hard.

This removes the style caching, because it was difficult to keep the
style cache correct in all cases.  Since this would cause more style
escapes to be emitted, instead it changes fputs_styled to try to avoid
unnecessary changes.

Another bug was that the wrap buffer was not flushed in the case where
wrap_column==0.  In the old (pre-patch series) code, characters were
directly emitted in this case; so flushing the wrap buffer here
restores this behavior.

On error the wrap buffer must be emptied.  Otherwise, interrupting
output can leave characters in the buffer that will be emitted later.

As discussed on gdb-patches, this fixes the ada-lang.c problem where
filtered and unfiltered printing were mixed.  Now user_select_syms
uses filtered printing, which is what its callees were already doing.

Finally, it was possible for source line highlighting to be garbled
(and invalid escape sequences emitted) if the pager was invoked at the
wrong spot.  To fix this, the patch arranges for source line escapes
to always be emitted as a unit.

gdb/ChangeLog
2019-02-17  Tom Tromey  <[email protected]>

	* ada-lang.c (user_select_syms): Use filtered printing.
	* utils.c (wrap_style): New global.
	(desired_style): Remove.
	(emit_style_escape): Add stream parameter.
	(set_output_style, reset_terminal_style, prompt_for_continue):
	Update.
	(flush_wrap_buffer): Only flush gdb_stdout.
	(wrap_here): Set wrap_style.
	(fputs_maybe_filtered): Clear the wrap buffer on exception.  Don't
	treat escape sequences as a character.  Change when wrap buffer is
	flushed.
	(fputs_styled): Do not set the output style when the default is
	requested.
	* ui-style.h (struct ui_file_style) <is_default>: New method.
	* source.c (print_source_lines_base): Emit escape sequences in one
	piece.

gdb/testsuite/ChangeLog
2019-02-17  Tom Tromey  <[email protected]>

	* gdb.base/style.exp: Add line-wrapping tests.
	* gdb.base/page.exp: Add test for quitting during pagination.

Author: tromey

gdb/ChangeLog

@@ -1,3 +1,22 @@
+2019-02-17  Tom Tromey  <[email protected]>
+
+	* ada-lang.c (user_select_syms): Use filtered printing.
+	* utils.c (wrap_style): New global.
+	(desired_style): Remove.
+	(emit_style_escape): Add stream parameter.
+	(set_output_style, reset_terminal_style, prompt_for_continue):
+	Update.
+	(flush_wrap_buffer): Only flush gdb_stdout.
+	(wrap_here): Set wrap_style.
+	(fputs_maybe_filtered): Clear the wrap buffer on exception.  Don't
+	treat escape sequences as a character.  Change when wrap buffer is
+	flushed.
+	(fputs_styled): Do not set the output style when the default is
+	requested.
+	* ui-style.h (struct ui_file_style) <is_default>: New method.
+	* source.c (print_source_lines_base): Emit escape sequences in one
+	piece.
+
 2019-02-17  Joel Brobecker  <[email protected]>
 
 	* gdbtypes.c (type_align): Handle TYPE_CODE_RANGE the same as

gdb/ada-lang.c

@@ -3903,16 +3903,16 @@ user_select_syms (struct block_symbol *syms, int nsyms, int max_results)
     error (_("\
 canceled because the command is ambiguous\n\
 See set/show multiple-symbol."));
-  
+
   /* If select_mode is "all", then return all possible symbols.
      Only do that if more than one symbol can be selected, of course.
      Otherwise, display the menu as usual.  */
   if (select_mode == multiple_symbols_all && max_results > 1)
     return nsyms;
 
-  printf_unfiltered (_("[0] cancel\n"));
+  printf_filtered (_("[0] cancel\n"));
   if (max_results > 1)
-    printf_unfiltered (_("[1] all\n"));
+    printf_filtered (_("[1] all\n"));
 
   sort_choices (syms, nsyms);
 
@@ -3926,16 +3926,16 @@ See set/show multiple-symbol."));
           struct symtab_and_line sal =
             find_function_start_sal (syms[i].symbol, 1);
 
-	  printf_unfiltered ("[%d] ", i + first_choice);
+	  printf_filtered ("[%d] ", i + first_choice);
 	  ada_print_symbol_signature (gdb_stdout, syms[i].symbol,
 				      &type_print_raw_options);
 	  if (sal.symtab == NULL)
-	    printf_unfiltered (_(" at <no source file available>:%d\n"),
-			       sal.line);
+	    printf_filtered (_(" at <no source file available>:%d\n"),
+			     sal.line);
 	  else
-	    printf_unfiltered (_(" at %s:%d\n"),
-			       symtab_to_filename_for_display (sal.symtab),
-			       sal.line);
+	    printf_filtered (_(" at %s:%d\n"),
+			     symtab_to_filename_for_display (sal.symtab),
+			     sal.line);
           continue;
         }
       else
@@ -3951,37 +3951,37 @@ See set/show multiple-symbol."));
 
           if (SYMBOL_LINE (syms[i].symbol) != 0 && symtab != NULL)
 	    {
-	      printf_unfiltered ("[%d] ", i + first_choice);
+	      printf_filtered ("[%d] ", i + first_choice);
 	      ada_print_symbol_signature (gdb_stdout, syms[i].symbol,
 					  &type_print_raw_options);
-	      printf_unfiltered (_(" at %s:%d\n"),
-				 symtab_to_filename_for_display (symtab),
-				 SYMBOL_LINE (syms[i].symbol));
+	      printf_filtered (_(" at %s:%d\n"),
+			       symtab_to_filename_for_display (symtab),
+			       SYMBOL_LINE (syms[i].symbol));
 	    }
           else if (is_enumeral
                    && TYPE_NAME (SYMBOL_TYPE (syms[i].symbol)) != NULL)
             {
-              printf_unfiltered (("[%d] "), i + first_choice);
+              printf_filtered (("[%d] "), i + first_choice);
               ada_print_type (SYMBOL_TYPE (syms[i].symbol), NULL,
                               gdb_stdout, -1, 0, &type_print_raw_options);
-              printf_unfiltered (_("'(%s) (enumeral)\n"),
-                                 SYMBOL_PRINT_NAME (syms[i].symbol));
+              printf_filtered (_("'(%s) (enumeral)\n"),
+			       SYMBOL_PRINT_NAME (syms[i].symbol));
             }
 	  else
 	    {
-	      printf_unfiltered ("[%d] ", i + first_choice);
+	      printf_filtered ("[%d] ", i + first_choice);
 	      ada_print_symbol_signature (gdb_stdout, syms[i].symbol,
 					  &type_print_raw_options);
 
 	      if (symtab != NULL)
-		printf_unfiltered (is_enumeral
-				   ? _(" in %s (enumeral)\n")
-				   : _(" at %s:?\n"),
-				   symtab_to_filename_for_display (symtab));
+		printf_filtered (is_enumeral
+				 ? _(" in %s (enumeral)\n")
+				 : _(" at %s:?\n"),
+				 symtab_to_filename_for_display (symtab));
 	      else
-		printf_unfiltered (is_enumeral
-				   ? _(" (enumeral)\n")
-				   : _(" at ?\n"));
+		printf_filtered (is_enumeral
+				 ? _(" (enumeral)\n")
+				 : _(" at ?\n"));
 	    }
         }
     }

gdb/source.c

@@ -1249,7 +1249,6 @@ static void
 print_source_lines_base (struct symtab *s, int line, int stopline,
 			 print_source_lines_flags flags)
 {
-  int c;
   scoped_fd desc;
   int noprint = 0;
   int nlines = stopline - line;
@@ -1343,13 +1342,10 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 	   line, symtab_to_filename_for_display (s), s->nlines);
 
   const char *iter = lines.c_str ();
-  while (nlines-- > 0)
+  while (nlines-- > 0 && *iter != '\0')
     {
       char buf[20];
 
-      c = *iter++;
-      if (c == '\0')
-	break;
       last_line_listed = current_source_line;
       if (flags & PRINT_SOURCE_LINES_FILENAME)
         {
@@ -1358,33 +1354,55 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
         }
       xsnprintf (buf, sizeof (buf), "%d\t", current_source_line++);
       uiout->text (buf);
-      do
+
+      while (*iter != '\0')
 	{
-	  if (c < 040 && c != '\t' && c != '\n' && c != '\r' && c != '\033')
+	  /* Find a run of characters that can be emitted at once.
+	     This is done so that escape sequences are kept
+	     together.  */
+	  const char *start = iter;
+	  while (true)
 	    {
-	      xsnprintf (buf, sizeof (buf), "^%c", c + 0100);
-	      uiout->text (buf);
+	      int skip_bytes;
+
+	      char c = *iter;
+	      if (c == '\033' && skip_ansi_escape (iter, &skip_bytes))
+		iter += skip_bytes;
+	      else if (c < 040 && c != '\t')
+		break;
+	      else if (c == 0177)
+		break;
+	      else
+		++iter;
 	    }
-	  else if (c == 0177)
-	    uiout->text ("^?");
-	  else if (c == '\r')
+	  if (iter > start)
 	    {
-	      /* Skip a \r character, but only before a \n.  */
-	      if (*iter != '\n')
-		printf_filtered ("^%c", c + 0100);
+	      std::string text (start, iter);
+	      uiout->text (text.c_str ());
 	    }
-	  else
+	  if (*iter == '\r')
 	    {
-	      xsnprintf (buf, sizeof (buf), "%c", c);
+	      /* Treat either \r or \r\n as a single newline.  */
+	      ++iter;
+	      if (*iter == '\n')
+		++iter;
+	      break;
+	    }
+	  else if (*iter == '\n')
+	    {
+	      ++iter;
+	      break;
+	    }
+	  else if (*iter > 0 && *iter < 040)
+	    {
+	      xsnprintf (buf, sizeof (buf), "^%c", *iter + 0100);
 	      uiout->text (buf);
 	    }
+	  else if (*iter == 0177)
+	    uiout->text ("^?");
 	}
-      while (c != '\n' && (c = *iter++) != '\0');
-      if (c == '\0')
-	break;
+      uiout->text ("\n");
     }
-  if (!lines.empty() && lines.back () != '\n')
-    uiout->text ("\n");
 }
 
 

gdb/testsuite/ChangeLog

@@ -1,3 +1,8 @@
+2019-02-17  Tom Tromey  <[email protected]>
+
+	* gdb.base/style.exp: Add line-wrapping tests.
+	* gdb.base/page.exp: Add test for quitting during pagination.
+
 2019-02-17  Joel Brobecker  <[email protected]>
 
 	* gdb.ada/big_packed_array: New testcase.

gdb/testsuite/gdb.base/page.exp

@@ -80,6 +80,20 @@ gdb_expect_list "paged count remainder" "${gdb_prompt} " {
     11
 }
 
+set fours [string repeat 4 40]
+set str "1\\n2\\n3\\n$fours\\n5\\n"
+
+# Avoid some confusing output from readline.
+gdb_test_no_output "set editing off"
+
+gdb_test_no_output "set width 30"
+send_gdb "printf \"$str\"\n"
+gdb_expect_list "paged count for interrupt" \
+    ".*$pagination_prompt" \
+    [list 1\r\n 2\r\n 3\r\n 444444444444444444444444444444]
+
+gdb_test "q" "Quit" "quit while paging"
+
 gdb_exit
 return 0
 

gdb/testsuite/gdb.base/style.exp

@@ -45,6 +45,24 @@ save_vars { env(TERM) } {
 
     gdb_test "print &main" " = .* \033\\\[34m$hex\033\\\[m <$main_expr>"
 
+    # Regression test for a bug where line-wrapping would occur at the
+    # wrong spot with styling.  There were different bugs at different
+    # widths, so try two.
+    foreach width {20 30} {
+	gdb_test_no_output "set width $width"
+	# There was also a bug where the styling could be wrong in the
+	# line listing; this is why the words from the source code are
+	# spelled out in the final result line of the test.
+	gdb_test "frame" \
+	    [multi_line \
+		 "#0 *$main_expr.*$arg_expr.*" \
+		 ".*$arg_expr.*" \
+		 ".* at .*$file_expr.*" \
+		 "\[0-9\]+.*return.* break here .*"
+	    ] \
+	    "frame when width=$width"
+    }
+
     gdb_exit
     gdb_spawn
 

gdb/ui-style.h

@@ -163,6 +163,16 @@ struct ui_file_style
   /* Return the ANSI escape sequence for this style.  */
   std::string to_ansi () const;
 
+  /* Return true if this style is the default style; false
+     otherwise.  */
+  bool is_default () const
+  {
+    return (m_foreground == NONE
+	    && m_background == NONE
+	    && m_intensity == NORMAL
+	    && !m_reverse);
+  }
+
   /* Return true if this style specified reverse display; false
      otherwise.  */
   bool is_reverse () const