diff mbox

ui_out format strings for fields and styles (Re: [PATCH] Style "pwd" output)

Message ID 87imtjbrmx.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey June 5, 2019, 10:21 p.m. UTC 
  >> We could just remove the enum, move to passing pointers everywhere, and
>> convert the *_style objects to be pointers.  This would streamline
>> things a bit; 

Pedro> ...

>> and we could use nullptr to mean "keep the default".

Pedro> Yeah, that would %pN / %p] redundant/unnecessary...

I didn't go quite as far as making everything take pointers, partly
because it made it harder to preserve the distinction between the CLI
layer and the ui-file styling layer.  So instead I just removed the
enum.

My patch also removes (really just comments out) %pS and %pN in favor of
requiring %ps.  Those could be restored with a bit of effort I guess.

Pedro> If you'd like to give this a try, please do feel free to push to
Pedro> the branch.

I didn't push but my patch is appended.  Let me know what you think.  I
can push tomorrow if you want it.  I'm not sure now if what remains of
the patch is a good idea or not; or if moving fully to pointers would be
better.

Tom

Comments

Pedro Alves June 6, 2019, 3:49 p.m. UTC | #1 
On 6/5/19 11:21 PM, Tom Tromey wrote:
>>> We could just remove the enum, move to passing pointers everywhere, and
>>> convert the *_style objects to be pointers.  This would streamline
>>> things a bit; 
> 
> Pedro> ...
> 
>>> and we could use nullptr to mean "keep the default".
> 
> Pedro> Yeah, that would %pN / %p] redundant/unnecessary...
> 
> I didn't go quite as far as making everything take pointers, partly
> because it made it harder to preserve the distinction between the CLI
> layer and the ui-file styling layer.  So instead I just removed the
> enum.
> 
> My patch also removes (really just comments out) %pS and %pN in favor of
> requiring %ps.  Those could be restored with a bit of effort I guess.

Yes, I think so.  I think the main issue is that cli_style_option::style()
returns a new temporary ui_file_style, so we can't take its address, like in:

       current_uiout->message (_("Reading symbols from %pS%s%pS...\n"),
                               &file_name_style.style (), name, nullptr);

But that could be fixed by adding a m_style field to cli_style_option,
and making cli_style_option::style() return a const reference to that.
We'd either need to make cli_style_option register set command hooks
to update that m_size variable whenever the users changes the corresponding
foreground/background setting, or always update m_style when
cli_style_option::style() is called.

> 
> Pedro> If you'd like to give this a try, please do feel free to push to
> Pedro> the branch.
> 
> I didn't push but my patch is appended.  Let me know what you think.  I
> can push tomorrow if you want it.  I'm not sure now if what remains of
> the patch is a good idea or not; or if moving fully to pointers would be
> better.
> 

Please push.  I don't know either, but undoing a patch on a branch is
simple enough if we find that we want to do that.  :-)

Thanks,
Pedro Alves
Tom Tromey June 6, 2019, 11:55 p.m. UTC | #2 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Yes, I think so.  I think the main issue is that cli_style_option::style()
Pedro> returns a new temporary ui_file_style, so we can't take its address, like in:

If it had a 'ptr ()' method, wouldn't the temporary live long enough in
the callee for it to work?

Pedro> Please push.  I don't know either, but undoing a patch on a branch is
Pedro> simple enough if we find that we want to do that.  :-)

I did it.

Tom
Tom Tromey June 7, 2019, 6:27 p.m. UTC | #3 
Tom> My patch also removes (really just comments out) %pS and %pN in favor of
Tom> requiring %ps.  Those could be restored with a bit of effort I guess.

I think we do need these.

I have been trying a patch to style gdb's meta-data output, stuff like
<unavailable>.  There are cases where the <...> contains some other
value, like:

	  fprintf_filtered (stream, " <repeats %u times>", reps);

Here we want to rewrite as:

	  fprintf_filtered (stream, " %pS<repeats %u times>%pN",
                            metadata_style.style ().ptr (), reps, nullptr);

Tom
Tom Tromey June 7, 2019, 7:20 p.m. UTC | #4 
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> My patch also removes (really just comments out) %pS and %pN in favor of
Tom> requiring %ps.  Those could be restored with a bit of effort I guess.

Tom> I think we do need these.

I pushed a couple more patches to the branch, including one intended to
fix this.

What's left to do on that branch in order to merge it?

Tom
Pedro Alves July 1, 2019, 12:01 p.m. UTC | #5 
On 6/7/19 7:27 PM, Tom Tromey wrote:
> Tom> My patch also removes (really just comments out) %pS and %pN in favor of
> Tom> requiring %ps.  Those could be restored with a bit of effort I guess.
> 
> I think we do need these.
> 
> I have been trying a patch to style gdb's meta-data output, stuff like
> <unavailable>.  There are cases where the <...> contains some other
> value, like:
> 
> 	  fprintf_filtered (stream, " <repeats %u times>", reps);
> 
> Here we want to rewrite as:
> 
> 	  fprintf_filtered (stream, " %pS<repeats %u times>%pN",
>                             metadata_style.style ().ptr (), reps, nullptr);
> 

Do you still see value in keeping %pN?  If we make nullptr mean
"keep the default", as you mentioned earlier, then the above can be
rewritten as:

	  fprintf_filtered (stream, " %pS<repeats %u times>%pS",
                            metadata_style.style ().ptr (), reps, nullptr);

Thanks,
Pedro Alves
Tom Tromey July 1, 2019, 12:25 p.m. UTC | #6 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Do you still see value in keeping %pN?  If we make nullptr mean
Pedro> "keep the default", as you mentioned earlier, then the above can be
Pedro> rewritten as:

Pedro> 	  fprintf_filtered (stream, " %pS<repeats %u times>%pS",
Pedro>                             metadata_style.style ().ptr (), reps, nullptr);

I was thinking that we'd change the spelling to some form of brackets,
in which case it would be good.  So like "%p[<repeats %u times>%p]".
We discussed this before but I don't recall whether there was some
counter-argument to it.

If we're just using letters, then there doesn't seem to be a reason to
have two %p suffixes.

Tom
Philippe Waroquiers July 1, 2019, 7:32 p.m. UTC | #7 
On Mon, 2019-07-01 at 06:25 -0600, Tom Tromey wrote:
> > > > > > "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Do you still see value in keeping %pN?  If we make nullptr mean
> Pedro> "keep the default", as you mentioned earlier, then the above can be
> Pedro> rewritten as:
> 
> Pedro> 	  fprintf_filtered (stream, " %pS<repeats %u times>%pS",
> Pedro>                             metadata_style.style ().ptr (), reps, nullptr);
> 
> I was thinking that we'd change the spelling to some form of brackets,
> in which case it would be good.  So like "%p[<repeats %u times>%p]".
> We discussed this before but I don't recall whether there was some
> counter-argument to it.
(Arriving late in the discussion ...)

If we have some markers/brackets in the format string to apply style,
why do we keep the style as an additional parameter of the 'printf' like functions ?

E.g. when looking at pango markup, changing the 'style' is done inside the string,
such as :
 "<span foreground=red>some string with red foreground</span>"

So, for GDB, we could have something like:
  
   some_output_function ("The filename is <style=filename>%s</style>.", some_filename);

where the low level of some_output_function would translate the <style=..> into
the real output of the control characters to do the styling.

The advantage of this approach is that the styling can be added for example
in the doc strings either statically and/or built dynamically
(think for example to the new option framework that builds a part of the doc
string:  we might e.g. put in bold the part of the option that is 'unique').

Philippe

> 
> If we're just using letters, then there doesn't seem to be a reason to
> have two %p suffixes.
> 
> To
Tom Tromey July 3, 2019, 12:11 p.m. UTC | #8 
Philippe> If we have some markers/brackets in the format string to apply
Philippe> style, why do we keep the style as an additional parameter of
Philippe> the 'printf' like functions ?

Philippe> E.g. when looking at pango markup, changing the 'style' is done inside the string,
Philippe> such as :
Philippe>  "<span foreground=red>some string with red foreground</span>"

Philippe> So, for GDB, we could have something like:
  
Philippe>    some_output_function ("The filename is <style=filename>%s</style>.", some_filename);

Philippe> where the low level of some_output_function would translate the <style=..> into
Philippe> the real output of the control characters to do the styling.

Philippe> The advantage of this approach is that the styling can be added for example
Philippe> in the doc strings either statically and/or built dynamically
Philippe> (think for example to the new option framework that builds a part of the doc
Philippe> string:  we might e.g. put in bold the part of the option that is 'unique').

I hadn't really given it much thought.

I suppose the main thing is that the lower levels -- ui-out and ui-file
-- are used for all output in gdb.  So if we had some kind of markup in
the output, we'd need to find all the places emitting unpredictable
output (say, file names or data from the inferior) and arrange for those
to quote the output somehow, to prevent unwanted effects.

On the other hand the programmatic approach doesn't require this, and is
closer to what gdb already does.

Nothing precludes us from doing this later on.  For instance, it could
be done for "help" without touching the rest of gdb.

Tom
diff mbox

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0c3c7df92fb..c5d3ccab1eb 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5836,14 +5836,14 @@  print_breakpoint_location (struct breakpoint *b,
 	{
 	  uiout->text ("in ");
 	  uiout->field_string ("func", SYMBOL_PRINT_NAME (sym),
-			       ui_out_style_kind::FUNCTION);
+			       function_name_style.style ());
 	  uiout->text (" ");
 	  uiout->wrap_hint (wrap_indent_at_field (uiout, "what"));
 	  uiout->text ("at ");
 	}
       uiout->field_string ("file",
 			   symtab_to_filename_for_display (loc->symtab),
-			   ui_out_style_kind::FILE);
+			   file_name_style.style ());
       uiout->text (":");
 
       if (uiout->is_mi_like_p ())
@@ -6693,9 +6693,9 @@  describe_other_breakpoints (struct gdbarch *gdbarch,
 			     (others > 1) ? "," 
 			     : ((others == 1) ? " and" : ""));
 	  }
-      current_uiout->message (_("also set at pc %pS%s%pN.\n"),
-			      ptr (ui_out_style_kind::ADDRESS),
-			      paddress (gdbarch, pc), nullptr);
+      current_uiout->message (_("also set at pc %ps.\n"),
+			      styled_string (address_style.style (),
+					     paddress (gdbarch, pc)).ptr ());
     }
 }
 
@@ -13355,12 +13355,12 @@  update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
 	  if (sym)
 	    {
 	      uiout->field_string ("func", SYMBOL_PRINT_NAME (sym),
-				   ui_out_style_kind::FUNCTION);
+				   function_name_style.style ());
 	      uiout->text (" at ");
 	    }
 	  uiout->field_string ("file",
 			       symtab_to_filename_for_display (sal2.symtab),
-			       ui_out_style_kind::FILE);
+			       file_name_style.style ());
 	  uiout->text (":");
 
 	  if (uiout->is_mi_like_p ())
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 71ed5c3cf33..aaea20e1512 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -73,7 +73,7 @@  cli_ui_out::do_table_header (int width, ui_align alignment,
     return;
 
   do_field_string (0, width, alignment, 0, col_hdr.c_str (),
-		   ui_out_style_kind::DEFAULT);
+		   ui_file_style ());
 }
 
 /* Mark beginning of a list */
@@ -102,7 +102,7 @@  cli_ui_out::do_field_int (int fldno, int width, ui_align alignment,
   std::string str = string_printf ("%d", value);
 
   do_field_string (fldno, width, alignment, fldname, str.c_str (),
-		   ui_out_style_kind::DEFAULT);
+		   ui_file_style ());
 }
 
 /* used to omit a field */
@@ -115,28 +115,7 @@  cli_ui_out::do_field_skip (int fldno, int width, ui_align alignment,
     return;
 
   do_field_string (fldno, width, alignment, fldname, "",
-		   ui_out_style_kind::DEFAULT);
-}
-
-static ui_file_style
-style_from_style_kind (ui_out_style_kind kind)
-{
-  switch (kind)
-    {
-    case ui_out_style_kind::DEFAULT:
-      /* Nothing.  */
-      return {};
-    case ui_out_style_kind::FILE:
-      return file_name_style.style ();
-    case ui_out_style_kind::FUNCTION:
-      return function_name_style.style ();
-    case ui_out_style_kind::VARIABLE:
-      return variable_name_style.style ();
-    case ui_out_style_kind::ADDRESS:
-      return address_style.style ();
-    default:
-      gdb_assert_not_reached ("missing case");
-    }
+		   ui_file_style ());
 }
 
 /* other specific cli_field_* end up here so alignment and field
@@ -145,7 +124,7 @@  style_from_style_kind (ui_out_style_kind kind)
 void
 cli_ui_out::do_field_string (int fldno, int width, ui_align align,
 			     const char *fldname, const char *string,
-			     ui_out_style_kind style)
+			     const ui_file_style &style)
 {
   int before = 0;
   int after = 0;
@@ -180,10 +159,7 @@  cli_ui_out::do_field_string (int fldno, int width, ui_align align,
     spaces (before);
 
   if (string)
-    {
-      ui_file_style fstyle = style_from_style_kind (style);
-      fputs_styled (string, fstyle, m_streams.back ());
-    }
+    fputs_styled (string, style, m_streams.back ());
 
   if (after)
     spaces (after);
@@ -205,7 +181,7 @@  cli_ui_out::do_field_fmt (int fldno, int width, ui_align align,
   std::string str = string_vprintf (format, args);
 
   do_field_string (fldno, width, align, fldname, str.c_str (),
-		   ui_out_style_kind::DEFAULT);
+		   ui_file_style ());
 }
 
 void
@@ -227,14 +203,13 @@  cli_ui_out::do_text (const char *string)
 }
 
 void
-cli_ui_out::do_message (ui_out_style_kind style,
+cli_ui_out::do_message (const ui_file_style &style,
 			const char *format, va_list args)
 {
   if (m_suppress_output)
     return;
 
-  ui_file_style fstyle = style_from_style_kind (style);
-  vfprintf_styled (m_streams.back (), fstyle, format, args);
+  vfprintf_styled (m_streams.back (), style, format, args);
 }
 
 void
diff --git a/gdb/cli-out.h b/gdb/cli-out.h
index 4ed7b396a57..eb519bb4a8d 100644
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -52,14 +52,14 @@  protected:
   virtual void do_field_string (int fldno, int width, ui_align align,
 				const char *fldname,
 				const char *string,
-				ui_out_style_kind style) override;
+				const ui_file_style &style) override;
   virtual void do_field_fmt (int fldno, int width, ui_align align,
 			     const char *fldname, const char *format,
 			     va_list args)
     override ATTRIBUTE_PRINTF (6,0);
   virtual void do_spaces (int numspaces) override;
   virtual void do_text (const char *string) override;
-  virtual void do_message (ui_out_style_kind style,
+  virtual void do_message (const ui_file_style &style,
 			   const char *format, va_list args) override
     ATTRIBUTE_PRINTF (3,0);
   virtual void do_wrap_hint (const char *identstring) override;
diff --git a/gdb/disasm.c b/gdb/disasm.c
index ed740c26e0f..54401a975b9 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -31,6 +31,7 @@ 
 #include <algorithm>
 #include "common/gdb_optional.h"
 #include "valprint.h"
+#include "cli/cli-style.h"
 
 /* Disassemble functions.
    FIXME: We should get rid of all the duplicate code in gdb that does
@@ -245,7 +246,7 @@  gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
 	uiout->text (" <");
 	if ((flags & DISASSEMBLY_OMIT_FNAME) == 0)
 	  uiout->field_string ("func-name", name.c_str (),
-			       ui_out_style_kind::FUNCTION);
+			       function_name_style.style ());
 	uiout->text ("+");
 	uiout->field_int ("offset", offset);
 	uiout->text (">:\t");
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index bb52360e540..b8ee8006018 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -70,9 +70,9 @@  mi_ui_out::do_table_header (int width, ui_align alignment,
   do_field_int (0, 0, ui_center, "width", width);
   do_field_int (0, 0, ui_center, "alignment", alignment);
   do_field_string (0, 0, ui_center, "col_name", col_name.c_str (),
-		   ui_out_style_kind::DEFAULT);
+		   ui_file_style ());
   do_field_string (0, width, alignment, "colhdr", col_hdr.c_str (),
-		   ui_out_style_kind::DEFAULT);
+		   ui_file_style ());
   close (ui_out_type_tuple);
 }
 
@@ -102,7 +102,7 @@  mi_ui_out::do_field_int (int fldno, int width, ui_align alignment,
 
   xsnprintf (buffer, sizeof (buffer), "%d", value);
   do_field_string (fldno, width, alignment, fldname, buffer,
-		   ui_out_style_kind::DEFAULT);
+		   ui_file_style ());
 }
 
 /* Used to omit a field.  */
@@ -119,7 +119,7 @@  mi_ui_out::do_field_skip (int fldno, int width, ui_align alignment,
 void
 mi_ui_out::do_field_string (int fldno, int width, ui_align align,
 			    const char *fldname, const char *string,
-			    ui_out_style_kind style)
+			    const ui_file_style &style)
 {
   ui_file *stream = m_streams.back ();
   field_separator ();
@@ -159,7 +159,7 @@  mi_ui_out::do_text (const char *string)
 }
 
 void
-mi_ui_out::do_message (ui_out_style_kind style,
+mi_ui_out::do_message (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 914a44194d4..f01d62f9bf3 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -58,13 +58,13 @@  protected:
 			   const char *fldname) override;
   virtual void do_field_string (int fldno, int width, ui_align align,
 				const char *fldname, const char *string,
-				ui_out_style_kind style) override;
+				const ui_file_style &style) override;
   virtual void do_field_fmt (int fldno, int width, ui_align align,
 			  const char *fldname, const char *format, va_list args)
     override ATTRIBUTE_PRINTF (6,0);
   virtual void do_spaces (int numspaces) override;
   virtual void do_text (const char *string) override;
-  virtual void do_message (ui_out_style_kind style,
+  virtual void do_message (const ui_file_style &style,
 			   const char *format, va_list args) override
     ATTRIBUTE_PRINTF (3,0);
   virtual void do_wrap_hint (const char *identstring) override;
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 95ad410f23f..0d084564c20 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -31,6 +31,7 @@ 
 #include "mi/mi-cmds.h"
 #include "python-internal.h"
 #include "common/gdb_optional.h"
+#include "cli/cli-style.h"
 
 enum mi_print_types
 {
@@ -898,7 +899,7 @@  py_print_frame (PyObject *filter, frame_filter_flags flags,
 	  if (function == NULL)
 	    out->field_skip ("func");
 	  else
-	    out->field_string ("func", function, ui_out_style_kind::FUNCTION);
+	    out->field_string ("func", function, function_name_style.style ());
 	}
     }
 
@@ -935,7 +936,7 @@  py_print_frame (PyObject *filter, frame_filter_flags flags,
 	      out->text (" at ");
 	      annotate_frame_source_file ();
 	      out->field_string ("file", filename.get (),
-				 ui_out_style_kind::FILE);
+				 file_name_style.style ());
 	      annotate_frame_source_file_end ();
 	    }
 	}
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 21085d5c62c..02bc43810f0 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -41,6 +41,7 @@ 
 #include "common/vec.h"
 #include "inferior.h"
 #include <algorithm>
+#include "cli/cli-style.h"
 
 static const target_info record_btrace_target_info = {
   "record-btrace",
@@ -1091,7 +1092,7 @@  btrace_call_history_src_line (struct ui_out *uiout,
 
   uiout->field_string ("file",
 		       symtab_to_filename_for_display (symbol_symtab (sym)),
-		       ui_out_style_kind::FILE);
+		       file_name_style.style ());
 
   btrace_compute_src_line_range (bfun, &begin, &end);
   if (end < begin)
@@ -1183,13 +1184,13 @@  btrace_call_history (struct ui_out *uiout,
 
       if (sym != NULL)
 	uiout->field_string ("function", SYMBOL_PRINT_NAME (sym),
-			     ui_out_style_kind::FUNCTION);
+			     function_name_style.style ());
       else if (msym != NULL)
 	uiout->field_string ("function", MSYMBOL_PRINT_NAME (msym),
-			     ui_out_style_kind::FUNCTION);
+			     function_name_style.style ());
       else if (!uiout->is_mi_like_p ())
 	uiout->field_string ("function", "??",
-			     ui_out_style_kind::FUNCTION);
+			     function_name_style.style ());
 
       if ((flags & RECORD_PRINT_INSN_RANGE) != 0)
 	{
diff --git a/gdb/skip.c b/gdb/skip.c
index 127b11dc443..0578422b99c 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -36,6 +36,7 @@ 
 #include "gdb_regex.h"
 #include "common/gdb_optional.h"
 #include <list>
+#include "cli/cli-style.h"
 
 /* True if we want to print debug printouts related to file/function
    skipping. */
@@ -414,7 +415,7 @@  info_skip_command (const char *arg, int from_tty)
       current_uiout->field_string ("file",
 				   e.file ().empty () ? "<none>"
 				   : e.file ().c_str (),
-				   ui_out_style_kind::FILE); /* 4 */
+				   file_name_style.style ()); /* 4 */
       if (e.function_is_regexp ())
 	current_uiout->field_string ("regexp", "y"); /* 5 */
       else
@@ -423,7 +424,7 @@  info_skip_command (const char *arg, int from_tty)
       current_uiout->field_string ("function",
 				   e.function ().empty () ? "<none>"
 				   : e.function ().c_str (),
-				   ui_out_style_kind::FUNCTION); /* 6 */
+				   function_name_style.style ()); /* 6 */
 
       current_uiout->text ("\n");
     }
diff --git a/gdb/solib.c b/gdb/solib.c
index e0b1a921f82..662433d33d1 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -47,6 +47,7 @@ 
 #include "gdb_bfd.h"
 #include "common/filestuff.h"
 #include "source.h"
+#include "cli/cli-style.h"
 
 /* Architecture-specific operations.  */
 
@@ -1104,7 +1105,7 @@  info_sharedlibrary_command (const char *pattern, int from_tty)
 	else
 	  uiout->field_string ("syms-read", so->symbols_loaded ? "Yes" : "No");
 
-	uiout->field_string ("name", so->so_name, ui_out_style_kind::FILE);
+	uiout->field_string ("name", so->so_name, file_name_style.style ());
 
 	uiout->text ("\n");
       }
diff --git a/gdb/source.c b/gdb/source.c
index 9a30209880b..3f6f64e1fdd 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -46,6 +46,7 @@ 
 #include <algorithm>
 #include "common/pathstuff.h"
 #include "source-cache.h"
+#include "cli/cli-style.h"
 
 #define OPEN_MODE (O_RDONLY | O_BINARY)
 #define FDOPEN_MODE FOPEN_RB
@@ -1306,7 +1307,7 @@  print_source_lines_base (struct symtab *s, int line, int stopline,
 	     not for TUI.  */
 	  if (uiout->is_mi_like_p () || uiout->test_flags (ui_source_list))
 	    uiout->field_string ("file", symtab_to_filename_for_display (s),
-				 ui_out_style_kind::FILE);
+				 file_name_style.style ());
 	  if (uiout->is_mi_like_p () || !uiout->test_flags (ui_source_list))
  	    {
 	      const char *s_fullname = symtab_to_fullname (s);
diff --git a/gdb/stack.c b/gdb/stack.c
index 547e82bbfb2..d8c6fdf550b 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -53,6 +53,7 @@ 
 #include "observable.h"
 #include "common/def-vector.h"
 #include "cli/cli-option.h"
+#include "cli/cli-style.h"
 
 /* The possible choices of "set print frame-arguments", and the value
    of this setting.  */
@@ -341,7 +342,7 @@  print_frame_arg (const frame_print_options &fp_opts,
   if (arg->entry_kind == print_entry_values_only
       || arg->entry_kind == print_entry_values_compact)
     stb.puts ("@entry");
-  uiout->field_stream ("name", stb, ui_out_style_kind::VARIABLE);
+  uiout->field_stream ("name", stb, variable_name_style.style ());
   annotate_arg_name_end ();
   uiout->text ("=");
 
@@ -909,18 +910,18 @@  print_frame_info (const frame_print_options &fp_opts,
         {
           annotate_function_call ();
           uiout->field_string ("func", "<function called from gdb>",
-			       ui_out_style_kind::FUNCTION);
+			       function_name_style.style ());
 	}
       else if (get_frame_type (frame) == SIGTRAMP_FRAME)
         {
 	  annotate_signal_handler_caller ();
           uiout->field_string ("func", "<signal handler called>",
-			       ui_out_style_kind::FUNCTION);
+			       function_name_style.style ());
         }
       else if (get_frame_type (frame) == ARCH_FRAME)
         {
           uiout->field_string ("func", "<cross-architecture call>",
-			       ui_out_style_kind::FUNCTION);
+			       function_name_style.style ());
 	}
       uiout->text ("\n");
       annotate_frame_end ();
@@ -1262,7 +1263,7 @@  print_frame (const frame_print_options &fp_opts,
 	    uiout->field_core_addr ("addr", gdbarch, pc);
 	  else
 	    uiout->field_string ("addr", "<unavailable>",
-				 ui_out_style_kind::ADDRESS);
+				 address_style.style ());
 	  annotate_frame_address_end ();
 	  uiout->text (" in ");
 	}
@@ -1271,7 +1272,7 @@  print_frame (const frame_print_options &fp_opts,
     string_file stb;
     fprintf_symbol_filtered (&stb, funname ? funname.get () : "??",
 			     funlang, DMGL_ANSI);
-    uiout->field_stream ("func", stb, ui_out_style_kind::FUNCTION);
+    uiout->field_stream ("func", stb, function_name_style.style ());
     uiout->wrap_hint ("   ");
     annotate_frame_args ();
 
@@ -1313,7 +1314,8 @@  print_frame (const frame_print_options &fp_opts,
 	uiout->wrap_hint ("   ");
 	uiout->text (" at ");
 	annotate_frame_source_file ();
-	uiout->field_string ("file", filename_display, ui_out_style_kind::FILE);
+	uiout->field_string ("file", filename_display,
+			     file_name_style.style ());
 	if (uiout->is_mi_like_p ())
 	  {
 	    const char *fullname = symtab_to_fullname (sal.symtab);
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 26762d289ca..903f2ff817b 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1115,11 +1115,9 @@  symbol_file_add_with_addrs (bfd *abfd, const char *name,
       if (deprecated_pre_add_symbol_hook)
 	deprecated_pre_add_symbol_hook (name);
       else
-	{
-	  puts_filtered (_("Reading symbols from "));
-	  fputs_styled (name, file_name_style.style (), gdb_stdout);
-	  puts_filtered ("...\n");
-	}
+	current_uiout->message (_("Reading symbols from %ps...\n"),
+				styled_string (file_name_style.style (),
+					       name).ptr ());
     }
   syms_from_objfile (objfile, addrs, add_flags);
 
@@ -1131,7 +1129,9 @@  symbol_file_add_with_addrs (bfd *abfd, const char *name,
   if ((flags & OBJF_READNOW))
     {
       if (should_print)
-	printf_filtered (_("Expanding full symbols from %s...\n"), name);
+	current_uiout->message (_("Expanding full symbols from %ps...\n"),
+				styled_string (file_name_style.style (),
+					       name).ptr ());
 
       if (objfile->sf)
 	objfile->sf->qf->expand_all_symtabs (objfile);
@@ -1143,7 +1143,9 @@  symbol_file_add_with_addrs (bfd *abfd, const char *name,
      file, and so printing it twice is just redundant.  */
   if (should_print && !objfile_has_symbols (objfile)
       && objfile->separate_debug_objfile == nullptr)
-    printf_filtered (_("(No debugging symbols found in %s)\n"), name);
+    current_uiout->message (_("(No debugging symbols found in %ps)\n"),
+			    styled_string (file_name_style.style (),
+					   name).ptr ());
 
   if (should_print)
     {
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 7dbf24eb7c5..e5f702ab804 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4605,7 +4605,7 @@  print_symbol_info (enum search_domain kind,
 	{
 	  current_uiout->message
 	    (_("\nFile %ps:\n"),
-	     styled_string (ui_out_style_kind::FILE, s_filename).ptr ());
+	     styled_string (file_name_style.style (), s_filename).ptr ());
 	}
 
       if (SYMBOL_LINE (sym) != 0)
@@ -4652,13 +4652,13 @@  print_msymbol_info (struct bound_minimal_symbol msymbol)
     tmp = hex_string_custom (BMSYMBOL_VALUE_ADDRESS (msymbol),
 			     16);
 
-  ui_out_style_kind sym_style = (msymbol.minsym->text_p ()
-				 ? ui_out_style_kind::FUNCTION
-				 : ui_out_style_kind::DEFAULT);
+  ui_file_style sym_style = (msymbol.minsym->text_p ()
+			     ? function_name_style.style ()
+			     : ui_file_style ());
 
   current_uiout->message
     (_("%ps  %ps\n"),
-     styled_string (ui_out_style_kind::ADDRESS, tmp).ptr (),
+     styled_string (address_style.style (), tmp).ptr (),
      styled_string (sym_style, MSYMBOL_PRINT_NAME (msymbol.minsym)).ptr ());
 }
 
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index c7585c67839..170c40a3046 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -57,6 +57,7 @@ 
 #include "tracefile.h"
 #include "location.h"
 #include <algorithm>
+#include "cli/cli-style.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -3690,7 +3691,7 @@  print_one_static_tracepoint_marker (int count,
     {
       uiout->text ("in ");
       uiout->field_string ("func", SYMBOL_PRINT_NAME (sym),
-			   ui_out_style_kind::FUNCTION);
+			   function_name_style.style ());
       uiout->wrap_hint (wrap_indent);
       uiout->text (" at ");
     }
@@ -3701,7 +3702,7 @@  print_one_static_tracepoint_marker (int count,
     {
       uiout->field_string ("file",
 			   symtab_to_filename_for_display (sal.symtab),
-			   ui_out_style_kind::FILE);
+			   file_name_style.style ());
       uiout->text (":");
 
       if (uiout->is_mi_like_p ())
diff --git a/gdb/tui/tui-out.c b/gdb/tui/tui-out.c
index 5fabff2cf1c..521c519cb75 100644
--- a/gdb/tui/tui-out.c
+++ b/gdb/tui/tui-out.c
@@ -52,7 +52,7 @@  tui_ui_out::do_field_int (int fldno, int width, ui_align alignment,
 void
 tui_ui_out::do_field_string (int fldno, int width, ui_align align,
 			     const char *fldname, const char *string,
-			     ui_out_style_kind style)
+			     const ui_file_style &style)
 {
   if (suppress_output ())
     return;
diff --git a/gdb/tui/tui-out.h b/gdb/tui/tui-out.h
index edf0b912bef..173027df525 100644
--- a/gdb/tui/tui-out.h
+++ b/gdb/tui/tui-out.h
@@ -35,7 +35,7 @@  protected:
   void do_field_int (int fldno, int width, ui_align align, const char *fldname,
 		  int value) override;
   void do_field_string (int fldno, int width, ui_align align, const char *fldname,
-			const char *string, ui_out_style_kind style) override;
+			const char *string, const ui_file_style &style) override;
   void do_field_fmt (int fldno, int width, ui_align align, const char *fldname,
 		  const char *format, va_list args) override
     ATTRIBUTE_PRINTF (6,0);
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 4169fdcb935..e74f685af02 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -25,6 +25,7 @@ 
 #include "language.h"
 #include "ui-out.h"
 #include "common/format.h"
+#include "cli/cli-style.h"
 
 #include <vector>
 #include <memory>
@@ -470,12 +471,12 @@  ui_out::field_core_addr (const char *fldname, struct gdbarch *gdbarch,
 			 CORE_ADDR address)
 {
   field_string (fldname, print_core_address (gdbarch, address),
-		ui_out_style_kind::ADDRESS);
+		address_style.style ());
 }
 
 void
 ui_out::field_stream (const char *fldname, string_file &stream,
-		      ui_out_style_kind style)
+		      const ui_file_style &style)
 {
   if (!stream.empty ())
     field_string (fldname, stream.c_str (), style);
@@ -500,7 +501,7 @@  ui_out::field_skip (const char *fldname)
 
 void
 ui_out::field_string (const char *fldname, const char *string,
-		      ui_out_style_kind style)
+		      const ui_file_style &style)
 {
   int fldno;
   int width;
@@ -548,7 +549,8 @@  ui_out::text (const char *string)
 }
 
 void
-ui_out::call_do_message (ui_out_style_kind style, const char *format, ...)
+ui_out::call_do_message (const ui_file_style &style, const char *format,
+			 ...)
 {
   va_list args;
 
@@ -562,7 +564,7 @@  ui_out::message (const char *format, ...)
 {
   format_pieces fpieces (&format, true);
 
-  ui_out_style_kind style = ui_out_style_kind::DEFAULT;
+  ui_file_style style;
 
   va_list args;
   va_start (args, format);
@@ -618,13 +620,13 @@  ui_out::message (const char *format, ...)
 		call_do_message (ss->style (), "%s", ss->str ());
 	      }
 	      break;
-	    case 'S':
-	      style = *va_arg (args, ui_out_style_kind *);
-	      break;
-	    case 'N':
-	      va_arg (args, void *);
-	      style = ui_out_style_kind::DEFAULT;
-	      break;
+	    /* case 'S': */
+	    /*   style = *va_arg (args, const ui_file_style *); */
+	    /*   break; */
+	    /* case 'N': */
+	    /*   va_arg (args, void *); */
+	    /*   style = nullptr; */
+	    /*   break; */
 	    default:
 	      call_do_message (style, current_substring, va_arg (args, void *));
 	      break;
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 059d1e376aa..f4c3857fe2d 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -26,6 +26,7 @@ 
 #include <vector>
 
 #include "common/enum-flags.h"
+#include "ui-style.h"
 
 class ui_out_level;
 class ui_out_table;
@@ -67,22 +68,6 @@  enum ui_out_type
     ui_out_type_list
   };
 
-/* Possible kinds of styling.  */
-
-enum class ui_out_style_kind
-{
-  /* The default (plain) style.  */
-  DEFAULT,
-  /* File name.  */
-  FILE,
-  /* Function name.  */
-  FUNCTION,
-  /* Variable name.  */
-  VARIABLE,
-  /* Address.  */
-  ADDRESS
-};
-
 struct int_field
 {
   int_field (const char *name, int val)
@@ -105,7 +90,7 @@  private:
 
 struct styled_string
 {
-  styled_string (ui_out_style_kind style, const char *str)
+  styled_string (const ui_file_style &style, const char *str)
     : m_style (style),
       m_str (str)
   {
@@ -115,32 +100,14 @@  struct styled_string
      va_args.  */
   const styled_string *ptr () const { return this; }
 
-  ui_out_style_kind style () const {return m_style; }
+  const ui_file_style &style () const { return m_style; }
   const char *str () const {return m_str; }
 
 private:
-  ui_out_style_kind m_style;
+  ui_file_style m_style;
   const char *m_str;
 };
 
-/* Wrap a ui_out_style_kind in a pointer to a temporary.  */
-
-/* XXX: Make a template?  */
-struct ptrS
-{
-  ptrS (ui_out_style_kind t) : m_t (t) {}
-
-  const ui_out_style_kind *ptr () const { return &m_t; }
-
-  ui_out_style_kind m_t;
-};
-
-static inline const ui_out_style_kind *
-ptr (ptrS &&t)
-{
-  return t.ptr ();
-}
-
 class ui_out
 {
  public:
@@ -171,10 +138,10 @@  class ui_out
   void field_core_addr (const char *fldname, struct gdbarch *gdbarch,
 			CORE_ADDR address);
   void field_string (const char *fldname, const char *string,
-		     ui_out_style_kind style = ui_out_style_kind::DEFAULT);
+		     const ui_file_style &style = ui_file_style ());
   void field_string (const char *fldname, const std::string &string);
   void field_stream (const char *fldname, string_file &stream,
-		     ui_out_style_kind style = ui_out_style_kind::DEFAULT);
+		     const ui_file_style &style = ui_file_style ());
   void field_skip (const char *fldname);
   void field_fmt (const char *fldname, const char *format, ...)
     ATTRIBUTE_PRINTF (3, 4);
@@ -219,14 +186,14 @@  class ui_out
 			      const char *fldname) = 0;
   virtual void do_field_string (int fldno, int width, ui_align align,
 				const char *fldname, const char *string,
-				ui_out_style_kind style) = 0;
+				const ui_file_style &style) = 0;
   virtual void do_field_fmt (int fldno, int width, ui_align align,
 			     const char *fldname, const char *format,
 			     va_list args)
     ATTRIBUTE_PRINTF (6,0) = 0;
   virtual void do_spaces (int numspaces) = 0;
   virtual void do_text (const char *string) = 0;
-  virtual void do_message (ui_out_style_kind style,
+  virtual void do_message (const ui_file_style &style,
 			   const char *format, va_list args)
     ATTRIBUTE_PRINTF (3,0) = 0;
   virtual void do_wrap_hint (const char *identstring) = 0;
@@ -240,7 +207,8 @@  class ui_out
   { return false; }
 
  private:
-  void call_do_message (ui_out_style_kind style, const char *format, ...);
+  void call_do_message (const ui_file_style &style, const char *format,
+			...);
 
   ui_out_flags m_flags;