[RFC,3/8] Add output styles to gdb

Message ID 20180906211303.11029-4-tom@tromey.com
State New
Headers show
Series
  • add terminal styling to gdb
Related show

Commit Message

Tom Tromey Sept. 6, 2018, 9:12 p.m.
This adds some output styling to the CLI.

A style is currently a foreground color, a background color, and an
intensity (dim or bold).  (This list could be expanded depending on
terminal capabilities.)

A style can be applied while printing.  This patch changes cli-out.c
to apply styles just to certain fields, recognized by name.  In
particular, function names and file names are stylized.

This seemed like a reasonable approach, because the names are fixed
due to their use in MI.  That is, the CLI is just as exposed to future
changes as MI is.

Users can control the style via a number of new set/show commands.  In
the interest of not typing many nearly-identical documentation
strings, I automated this.  On the down side, this is not very
i18n-friendly.

I've chose some default colors to use.  I think it would be good to
enable this by default, so that when users start the new gdb, they
will see the new feature.

Stylizing is done if TERM is set and is not "dumb".  This could be
improved when the TUI is available by using the curses has_colors
call.  That is, the lowest layer could call this without committing to
using curses everywhere; see my other patch for TUI colorizing.

I considered adding a new "set_style" method to ui_file.  However,
because the implementation had to interact with the pager code, I
didn't take this approach.  But, one idea might be to put the isatty
check there and then have it defer to the lower layers.
---
 gdb/Makefile.in     |   2 +
 gdb/cli-out.c       |  19 ++++-
 gdb/cli/cli-style.c | 207 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/cli/cli-style.h |  67 +++++++++++++++++
 gdb/ui-file.h       |  42 +++++++++++
 gdb/utils.c         |  69 +++++++++++++++++-
 gdb/utils.h         |   4 +
 7 files changed, 408 insertions(+), 2 deletions(-)
 create mode 100644 gdb/cli/cli-style.c
 create mode 100644 gdb/cli/cli-style.h

-- 
2.13.6

Comments

Simon Marchi Oct. 6, 2018, 3:53 p.m. | #1
On 2018-09-06 5:12 p.m., Tom Tromey wrote:
> This adds some output styling to the CLI.

> 

> A style is currently a foreground color, a background color, and an

> intensity (dim or bold).  (This list could be expanded depending on

> terminal capabilities.)

> 

> A style can be applied while printing.  This patch changes cli-out.c

> to apply styles just to certain fields, recognized by name.  In

> particular, function names and file names are stylized.

> 

> This seemed like a reasonable approach, because the names are fixed

> due to their use in MI.  That is, the CLI is just as exposed to future

> changes as MI is.

> 

> Users can control the style via a number of new set/show commands.  In

> the interest of not typing many nearly-identical documentation

> strings, I automated this.  On the down side, this is not very

> i18n-friendly.

> 

> I've chose some default colors to use.  I think it would be good to

> enable this by default, so that when users start the new gdb, they

> will see the new feature.

> 

> Stylizing is done if TERM is set and is not "dumb".  This could be

> improved when the TUI is available by using the curses has_colors

> call.  That is, the lowest layer could call this without committing to

> using curses everywhere; see my other patch for TUI colorizing.

> 

> I considered adding a new "set_style" method to ui_file.  However,

> because the implementation had to interact with the pager code, I

> didn't take this approach.  But, one idea might be to put the isatty

> check there and then have it defer to the lower layers.


Hi Tom,

Overall this looks great.  I'm not too worried about internationalization.
I think in this case for example:

  _("The \"%s\" display intensity is: %s\n"), name, value

If "name" corresponds to a field name or sub-command name (like "filename"),
it's probably better to leave it in english.  If it refers to the concept
of a filename, then we would want to translate it.

In the latter case, I guess we could do it in two steps, and also pass the value
through gettext:

  std::string tmpl = string_printf ("The %s display intensity is: %%s\n", name);
  printf (_(tmpl.c_str ()), _(value));

So tmpl would contain "The filename display intensity is: %s\n", which could be
looked up by gettext to something that has the proper translation for "filename".
Then, the color name would be translated too.

Or maybe I don't understand how gettext works.


I'm just not sure about choosing styles using the field name.  For a filename, you could
have a bunch of different field names, file filename, original_filename, absolute_filename,
etc.  So it can quickly become a bit crowded here.

Could we pass an additional enum parameter to do_field_string to indicate the type of
element this field represents?  If this parameter has a default value of "NOTHING",
then we can add then incrementally.  Or it can even be an hybrid approach, where we
try to match field names, which works 95% of the time, and then have this enum parameter
to override the auto-detection if needed.

Simon
Tom Tromey Oct. 6, 2018, 7:06 p.m. | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> Could we pass an additional enum parameter to do_field_string to
Simon> indicate the type of element this field represents?  If this
Simon> parameter has a default value of "NOTHING", then we can add then
Simon> incrementally.

Yes, either this or, as you mentioned in another note, passing the style
directly, could be done.  I suppose I tend to lean more toward the
semantic approach than the style approach, because maybe having some
kind of "type" attached to a field could be useful in other situations.

Be sure to read this as well:

https://sourceware.org/ml/gdb-patches/2018-10/msg00107.html

It presents another alternative -- one I rejected but maybe it can be
rehabilitated.

These questions about the overall approach and the API are the main
things to resolve up front, since they have the highest cost to change.

Tom
Simon Marchi Oct. 7, 2018, 9:58 p.m. | #3
On 2018-10-06 3:06 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

> 

> Simon> Could we pass an additional enum parameter to do_field_string to

> Simon> indicate the type of element this field represents?  If this

> Simon> parameter has a default value of "NOTHING", then we can add then

> Simon> incrementally.

> 

> Yes, either this or, as you mentioned in another note, passing the style

> directly, could be done.  I suppose I tend to lean more toward the

> semantic approach than the style approach, because maybe having some

> kind of "type" attached to a field could be useful in other situations.

> 

> Be sure to read this as well:

> 

> https://sourceware.org/ml/gdb-patches/2018-10/msg00107.html

> 

> It presents another alternative -- one I rejected but maybe it can be

> rehabilitated.


I guess you refer to Pedro's suggestion of having custom format strings.  I am not
against that, I am just afraid that because it's a lot more efforts than something
like you did, it would postpone the results indefinitely (unless somebody is super
motivated to do it right now).

Also, I think these custom format strings (LLVM uses its own style too, as mentioned)
need to bring significantly more value (in terms of code simplicity) than the other
alternatives, because they are harder to learn, modify and debug than simple function
calls.
> These questions about the overall approach and the API are the main

> things to resolve up front, since they have the highest cost to change.


Indeed, but at least it's not external API, so we have the option to change.

Simon
Tom Tromey Oct. 8, 2018, 12:23 a.m. | #4
>> These questions about the overall approach and the API are the main

>> things to resolve up front, since they have the highest cost to change.


Simon> Indeed, but at least it's not external API, so we have the option to change.

The main thing I am concerned about is that if we pick the approach I've
implemented, then changing our minds to a format-based approach will be
harder.  I think that's the case because the user-facing "set"s would be
hard to migrate to a format-based approach.

And, FWIW, I think the format-based approach isn't super hard to
implement.  The main problems there are design problems: whether to
remap the names of tables and fields; how to handle the trouble cases I
found in my initial investigation; and of course whether there are more
difficult cases lurking.

Overall I think I'm in favor of the approach I've posted, but I think
it's a good to examine this idea critically.

Tom
Simon Marchi Oct. 8, 2018, 2:02 a.m. | #5
On 2018-10-07 20:23, Tom Tromey wrote:
> Simon> Indeed, but at least it's not external API, so we have the

> option to change.

> 

> The main thing I am concerned about is that if we pick the approach 

> I've

> implemented, then changing our minds to a format-based approach will be

> harder.  I think that's the case because the user-facing "set"s would 

> be

> hard to migrate to a format-based approach.

> 

> And, FWIW, I think the format-based approach isn't super hard to

> implement.  The main problems there are design problems: whether to

> remap the names of tables and fields; how to handle the trouble cases I

> found in my initial investigation; and of course whether there are more

> difficult cases lurking.


Ok, so I'd like to understand better the idea of the format-based 
approach, maybe I don't quite get what you mean.  It would be easier to 
comment on something concrete (even without code, just a relatively 
well-defined description of the format).

Simon
Tom Tromey Oct. 8, 2018, 2:49 a.m. | #6
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> Ok, so I'd like to understand better the idea of the format-based
Simon> approach, maybe I don't quite get what you mean.  It would be easier
Simon> to comment on something concrete (even without code, just a relatively
Simon> well-defined description of the format).

The minimum, I think, is to have something that encompasses the
formatting, but not the titling, emitted by table headers.  So instead
of:

  uiout->table_header (10, ui_left, "refcount", "Refcount");

the example I gave said

  "{refcount<10}"

where the idea is that "refcount" is the field name, "<" says to
left-justify, and "10" is the field size.  I pictured using ">" for
right justification, and something like "|" for center and ":" or "="
for "none".

Anything outside of {} would be plain text that is just emitted
directly.

Exactly how to spell the formatting can be debated, like should there
be a lead-in character ("set extended-prompt" uses backslash, C uses %).

So, something like maintenance_info_bfds currently reads:

  ui_out_emit_table table_emitter (uiout, 3, -1, "bfds");
  uiout->table_header (10, ui_left, "refcount", "Refcount");
  uiout->table_header (18, ui_left, "addr", "Address");
  uiout->table_header (40, ui_left, "filename", "Filename");

... but in the format approach the first 2 arguments of each call could
be removed.

Then when printing the body:

  ui_out_emit_tuple tuple_emitter (uiout, NULL);
  uiout->field_int ("refcount", gdata->refc);
  uiout->field_string ("addr", host_address_to_string (abfd));
  uiout->field_string ("filename", bfd_get_filename (abfd));
  uiout->text ("\n");

Here the ->text call could be removed and the text put into the format
string.


I didn't do an exhaustive survey of lists or tuples in the way that I
looked at tables.  There are probably difficult cases lurking in there,
just as there were for tables.  And, the difficult table cases remain
unsolved -- I haven't really given them much thought.

Maybe another tricky case is handling indentation of elided stack
frames.


Something I like about the approach I implemented is that it's
relatively easy to explain and use.

The upside of the foramtting approach is that it gives more power to
users.  For example maybe there are things that are currently hidden
behind mi-like-p that could be exposed; or at least users could arrange
to drop fields they are not interested in.

Tom
Simon Marchi Oct. 8, 2018, 11:10 a.m. | #7
On 2018-10-07 22:49, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

> 

> Simon> Ok, so I'd like to understand better the idea of the 

> format-based

> Simon> approach, maybe I don't quite get what you mean.  It would be 

> easier

> Simon> to comment on something concrete (even without code, just a 

> relatively

> Simon> well-defined description of the format).

> 

> The minimum, I think, is to have something that encompasses the

> formatting, but not the titling, emitted by table headers.  So instead

> of:

> 

>   uiout->table_header (10, ui_left, "refcount", "Refcount");

> 

> the example I gave said

> 

>   "{refcount<10}"

> 

> where the idea is that "refcount" is the field name, "<" says to

> left-justify, and "10" is the field size.  I pictured using ">" for

> right justification, and something like "|" for center and ":" or "="

> for "none".

> 

> Anything outside of {} would be plain text that is just emitted

> directly.


Do we pass in the values as with printf?  Something like

   ui_out->format ("{refcount<10}{addr<18}{filename<40}\n", refcount, 
addr, filename);

How does the underlying implementation of format knows that the refcount 
variable is an int, and that addr and filename are char*?  We go by 
field name too?  Or should the format specifier contain the data type?

> Exactly how to spell the formatting can be debated, like should there

> be a lead-in character ("set extended-prompt" uses backslash, C uses 

> %).

> 

> So, something like maintenance_info_bfds currently reads:

> 

>   ui_out_emit_table table_emitter (uiout, 3, -1, "bfds");

>   uiout->table_header (10, ui_left, "refcount", "Refcount");

>   uiout->table_header (18, ui_left, "addr", "Address");

>   uiout->table_header (40, ui_left, "filename", "Filename");

> 

> ... but in the format approach the first 2 arguments of each call could

> be removed.

> 

> Then when printing the body:

> 

>   ui_out_emit_tuple tuple_emitter (uiout, NULL);

>   uiout->field_int ("refcount", gdata->refc);

>   uiout->field_string ("addr", host_address_to_string (abfd));

>   uiout->field_string ("filename", bfd_get_filename (abfd));

>   uiout->text ("\n");

> 

> Here the ->text call could be removed and the text put into the format

> string.

> 

> 

> I didn't do an exhaustive survey of lists or tuples in the way that I

> looked at tables.  There are probably difficult cases lurking in there,

> just as there were for tables.  And, the difficult table cases remain

> unsolved -- I haven't really given them much thought.

> 

> Maybe another tricky case is handling indentation of elided stack

> frames.

> 

> 

> Something I like about the approach I implemented is that it's

> relatively easy to explain and use.

> 

> The upside of the foramtting approach is that it gives more power to

> users.  For example maybe there are things that are currently hidden

> behind mi-like-p that could be exposed; or at least users could arrange

> to drop fields they are not interested in.


I tend to think that introducing a format-based system like this is 
relatively orthogonal to the coloring/styling issue.  I see it as 
syntactic sugar over the field_* methods.  Something would parse that 
format string and do the appropriate field_* calls for you under the 
hood.  If we color based on field names, as the current patch does, then 
it wouldn't make any difference.  If we provide a way for the caller to 
override the element type (on which the color/style choice is based), 
then we need to consider that in the format string syntax.  If field 
"foo" is a filename, we can have something like

     "{foo<40:filename}"

So my opinion is that introducing something like this isn't in the way 
of introducing coloring with the current system.

Simon
Tom Tromey Oct. 8, 2018, 10:17 p.m. | #8
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


>> "{refcount<10}"


Simon> Do we pass in the values as with printf?  Something like
Simon> ui_out-> format ("{refcount<10}{addr<18}{filename<40}\n", refcount,
Simon> addr, filename);

Nope, the idea is to continue to use the existing ui-out methods.  So
continuing the gdb_bfd example, this code in print_one_bfd:

  uiout->field_int ("refcount", gdata->refc);
  uiout->field_string ("addr", host_address_to_string (abfd));
  uiout->field_string ("filename", bfd_get_filename (abfd));

... would not change at all.

The idea is that the format would be registered as a set/show command
somewhere.  For tables, cli-out could look up the appropriate format
string using the table name; not sure how tuples or lists would work.

Simon> How does the underlying implementation of format knows that the
Simon> refcount variable is an int, and that addr and filename are char*?  We
Simon> go by field name too?  Or should the format specifier contain the data
Simon> type?

The above answers, but basically it would work like today -- the type is
implicit in the ui-out calls, though really at the bottom layer it is
all just strings.

Simon> I tend to think that introducing a format-based system like this is
Simon> relatively orthogonal to the coloring/styling issue.

Part of the overall idea was to let users put styling directly in the
format strings -- sorry, maybe I didn't really spell this out.  So
another example would be

  "{fg=red}{field<40}{fg=default}"

... or something along those lines.  This would conflict a bit with the
idea of styling elements the way that the current patch does it, since
then you'd have to decide what takes precedence.

Though, your note makes me think that this potential conflict really
isn't so bad and we could push forward with the current approach and
deal with more customizable formatting later.

Tom

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c76a4e4394c..3117bf74094 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -239,6 +239,7 @@  SUBDIR_CLI_SRCS = \
 	cli/cli-logging.c \
 	cli/cli-script.c \
 	cli/cli-setshow.c \
+	cli/cli-style.c \
 	cli/cli-utils.c
 
 SUBDIR_CLI_OBS = $(patsubst %.c,%.o,$(SUBDIR_CLI_SRCS))
@@ -1415,6 +1416,7 @@  HFILES_NO_SRCDIR = \
 	cli/cli-decode.h \
 	cli/cli-script.h \
 	cli/cli-setshow.h \
+	cli/cli-style.h \
 	cli/cli-utils.h \
 	common/buffer.h \
 	common/cleanups.h \
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index ad0a34ed39f..eeb3a64cbae 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -25,6 +25,7 @@ 
 #include "cli-out.h"
 #include "completer.h"
 #include "readline/readline.h"
+#include "cli/cli-style.h"
 
 /* These are the CLI output functions */
 
@@ -156,7 +157,23 @@  cli_ui_out::do_field_string (int fldno, int width, ui_align align,
     spaces (before);
 
   if (string)
-    fputs_filtered (string, m_streams.back ());
+    {
+      cli_style_option *style = nullptr;
+      if (fldname == nullptr)
+	{
+	  /* Nothing.  */
+	}
+      else if (!strcmp (fldname, "file"))
+	style = &file_name_style;
+      else if (!strcmp (fldname, "func") || !strcmp (fldname, "function"))
+	style = &function_name_style;
+
+      if (style != nullptr)
+	set_output_style (m_streams.back (), style->style ());
+      fputs_filtered (string, m_streams.back ());
+      if (style != nullptr)
+	set_output_style (m_streams.back (), ui_file_style ());
+    }
 
   if (after)
     spaces (after);
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
new file mode 100644
index 00000000000..9b210863bd3
--- /dev/null
+++ b/gdb/cli/cli-style.c
@@ -0,0 +1,207 @@ 
+/* CLI colorizing
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "cli/cli-cmds.h"
+#include "cli/cli-style.h"
+
+static const char * const cli_colors[] = {
+  "none",
+  "black",
+  "red",
+  "green",
+  "yellow",
+  "blue",
+  "magenta",
+  "cyan",
+  "white",
+  nullptr
+};
+
+static const char * const cli_intensities[] = {
+  "normal",
+  "bold",
+  "dim",
+  nullptr
+};
+
+cli_style_option file_name_style (ui_file_style::GREEN);
+cli_style_option function_name_style (ui_file_style::YELLOW);
+
+cli_style_option::cli_style_option (ui_file_style::color fg)
+  : m_foreground (cli_colors[fg - ui_file_style::NONE]),
+    m_background (cli_colors[0]),
+    m_intensity (cli_intensities[ui_file_style::NORMAL])
+{
+}
+
+static int
+color_number (const char *color)
+{
+  for (int i = 0; i < ARRAY_SIZE (cli_colors); ++i)
+    {
+      if (color == cli_colors[i])
+	return i - 1;
+    }
+  gdb_assert_not_reached ("color not found");
+}
+
+ui_file_style
+cli_style_option::style () const
+{
+  ui_file_style result;
+
+  result.m_foreground = (ui_file_style::color) color_number (m_foreground);
+  result.m_background = (ui_file_style::color) color_number (m_background);
+
+  for (int i = 0; i < ARRAY_SIZE (cli_intensities); ++i)
+    {
+      if (m_intensity == cli_intensities[i])
+	{
+	  result.m_intensity = (ui_file_style::intensity) i;
+	  break;
+	}
+    }
+
+  return result;
+}
+
+void
+cli_style_option::do_set (const char *args, int from_tty)
+{
+}
+
+void
+cli_style_option::do_show (const char *args, int from_tty)
+{
+}
+
+void
+cli_style_option::do_show_foreground (struct ui_file *file, int from_tty,
+				      struct cmd_list_element *cmd,
+				      const char *value)
+{
+  const char *name = (const char *) get_cmd_context (cmd);
+  fprintf_filtered (file, _("The \"%s\" foreground color is: %s\n"),
+		    name, value);
+}
+
+void
+cli_style_option::do_show_background (struct ui_file *file, int from_tty,
+				      struct cmd_list_element *cmd,
+				      const char *value)
+{
+  const char *name = (const char *) get_cmd_context (cmd);
+  fprintf_filtered (file, _("The \"%s\" background color is: %s\n"),
+		    name, value);
+}
+
+void
+cli_style_option::do_show_intensity (struct ui_file *file, int from_tty,
+				     struct cmd_list_element *cmd,
+				     const char *value)
+{
+  const char *name = (const char *) get_cmd_context (cmd);
+  fprintf_filtered (file, _("The \"%s\" display intensity is: %s\n"),
+		    name, value);
+}
+
+void
+cli_style_option::add_setshow_commands (const char *name,
+					enum command_class theclass,
+					const char *prefix_doc,
+					const char *prefixname,
+					struct cmd_list_element **set_list,
+					struct cmd_list_element **show_list)
+{
+  m_show_prefix = std::string ("set ") + prefixname + " ";
+  m_show_prefix = std::string ("show ") + prefixname + " ";
+
+  add_prefix_cmd (name, no_class, do_set, prefix_doc, &m_set_list, 
+		  m_show_prefix.c_str (), 0, set_list);
+  add_prefix_cmd (name, no_class, do_show, prefix_doc, &m_show_list,
+		  m_set_prefix.c_str (), 0, show_list);
+
+  add_setshow_enum_cmd ("foreground", theclass, cli_colors,
+			&m_foreground,
+			_("Set the foreground color for this property"),
+			_("Show the foreground color for this property"),
+			nullptr,
+			nullptr,
+			do_show_foreground,
+			&m_set_list, &m_show_list, (void *) name);
+  add_setshow_enum_cmd ("background", theclass, cli_colors,
+			&m_background,
+			_("Set the background color for this property"),
+			_("Show the background color for this property"),
+			nullptr,
+			nullptr,
+			do_show_background,
+			&m_set_list, &m_show_list, (void *) name);
+  add_setshow_enum_cmd ("intensity", theclass, cli_intensities,
+			&m_intensity,
+			_("Set the display intensity color for this property"),
+			_("\
+Show the display intensity color for this property"),
+			nullptr,
+			nullptr,
+			do_show_intensity,
+			&m_set_list, &m_show_list, (void *) name);
+}
+
+static void
+set_style (const char *arg, int from_tty)
+{
+}
+
+static void
+show_style (const char *arg, int from_tty)
+{
+}
+
+void
+_initialize_cli_style ()
+{
+  static cmd_list_element *style_set_list;
+  static cmd_list_element *style_show_list;
+
+  add_prefix_cmd ("style", no_class, set_style, _("\
+Style-specific settings\n\
+Configure various style-related variables, such as colors"),
+		  &style_set_list, "set style ", 0, &setlist);
+  add_prefix_cmd ("style", no_class, show_style, _("\
+Style-specific settings\n\
+Configure various style-related variables, such as colors"),
+		  &style_show_list, "show style ", 0, &showlist);
+
+  file_name_style.add_setshow_commands ("filename", no_class,
+					_("\
+Filename display styling\n\
+Configure filename colors and display intensity."),
+					"style filename",
+					&style_set_list,
+					&style_show_list);
+  function_name_style.add_setshow_commands ("function", no_class,
+					    _("\
+Function name display styling\n\
+Configure function name colors and display intensity"),
+					    "style function",
+					    &style_set_list,
+					    &style_show_list);
+}
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
new file mode 100644
index 00000000000..d9c00003bd4
--- /dev/null
+++ b/gdb/cli/cli-style.h
@@ -0,0 +1,67 @@ 
+/* CLI stylizing
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef CLI_STYLE_H
+#define CLI_STYLE_H
+
+#include "ui-file.h"
+
+class cli_style_option
+{
+public:
+
+  cli_style_option (ui_file_style::color fg);
+
+  ui_file_style style () const;
+
+  void add_setshow_commands (const char *name,
+			     enum command_class theclass,
+			     const char *prefix_doc,
+			     const char *prefixname,
+			     struct cmd_list_element **set_list,
+			     struct cmd_list_element **show_list);
+
+private:
+
+  const char *m_foreground;
+  const char *m_background;
+  const char *m_intensity;
+
+  std::string m_show_prefix;
+  std::string m_set_prefix;
+  struct cmd_list_element *m_set_list = nullptr;
+  struct cmd_list_element *m_show_list = nullptr;
+
+  static void do_set (const char *args, int from_tty);
+  static void do_show (const char *args, int from_tty);
+  static void do_show_foreground (struct ui_file *file, int from_tty,
+				  struct cmd_list_element *cmd,
+				  const char *value);
+  static void do_show_background (struct ui_file *file, int from_tty,
+				  struct cmd_list_element *cmd,
+				  const char *value);
+  static void do_show_intensity (struct ui_file *file, int from_tty,
+				 struct cmd_list_element *cmd,
+				 const char *value);
+};
+
+extern cli_style_option file_name_style;
+extern cli_style_option function_name_style;
+
+#endif /* CLI_STYLE_H */
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 2cf5f83d473..630fcffef69 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -21,6 +21,48 @@ 
 
 #include <string>
 
+/* Styles that can be applied to a ui_file.  */
+struct ui_file_style
+{
+  enum color
+  {
+    NONE = -1,
+    BLACK,
+    RED,
+    GREEN,
+    YELLOW,
+    BLUE,
+    MAGENTA,
+    CYAN,
+    WHITE
+  };
+
+  enum intensity
+  {
+    NORMAL = 0,
+    BOLD,
+    DIM
+  };
+
+
+  enum color m_foreground = NONE;
+  enum color m_background = NONE;
+  enum intensity m_intensity = NORMAL;
+
+  bool operator== (const ui_file_style &other) const
+  {
+    return (m_foreground == other.m_foreground
+	    && m_background == other.m_background
+	    && m_intensity == other.m_intensity);
+  }
+
+  bool operator!= (const ui_file_style &other) const
+  {
+    return !(*this == other);
+  }
+};
+
+
 /* The abstract ui_file base class.  */
 
 class ui_file
diff --git a/gdb/utils.c b/gdb/utils.c
index 1982fa20e64..ca69d5fc57c 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1422,6 +1422,63 @@  set_screen_width_and_height (int width, int height)
   set_width ();
 }
 
+static ui_file_style applied_style;
+static ui_file_style desired_style;
+
+static void
+emit (const char *str)
+{
+  if (wrap_column)
+    wrap_buffer.append (str);
+  else
+    puts_unfiltered (str);
+}
+
+static void
+emit_style_escape (const ui_file_style &style)
+{
+  if (applied_style == style)
+    return;
+  applied_style = style;
+
+  emit ("\e[");
+  bool need_semi = false;
+  if (style.m_foreground != ui_file_style::NONE)
+    {
+      emit (std::to_string (30 + style.m_foreground).c_str ());
+      need_semi = true;
+    }
+  if (style.m_background != ui_file_style::NONE)
+    {
+      if (need_semi)
+	emit (";");
+      emit (std::to_string (40 + style.m_background).c_str ());
+      need_semi = true;
+    }
+  if (style.m_intensity != ui_file_style::NORMAL)
+    {
+      if (need_semi)
+	emit (";");
+      emit (std::to_string (style.m_intensity).c_str ());
+    }
+  emit ("m");
+}
+
+void
+set_output_style (struct ui_file *stream, const ui_file_style &style)
+{
+  if (stream != gdb_stdout
+      || style == desired_style
+      || !ui_file_isatty (stream))
+    return;
+  const char *term = getenv ("TERM");
+  if (term == nullptr || !strcmp (term, "dumb"))
+    return;
+
+  desired_style = style;
+  emit_style_escape (style);
+}
+
 /* Wait, so the user can read what's on the screen.  Prompt the user
    to continue by pressing RETURN.  'q' is also provided because
    telling users what to do in the prompt is more user-friendly than
@@ -1437,6 +1494,9 @@  prompt_for_continue (void)
   steady_clock::time_point prompt_started = steady_clock::now ();
   bool disable_pagination = pagination_disabled_for_command;
 
+  /* Clear the current styling.  */
+  emit_style_escape (ui_file_style ());
+
   if (annotation_level > 1)
     printf_unfiltered (("\n\032\032pre-prompt-for-continue\n"));
 
@@ -1481,6 +1541,9 @@  prompt_for_continue (void)
   reinitialize_more_filter ();
   pagination_disabled_for_command = disable_pagination;
 
+  /* Restore the current styling.  */
+  emit_style_escape (desired_style);
+
   dont_repeat ();		/* Forget prev cmd -- CR won't repeat it.  */
 }
 
@@ -1709,7 +1772,10 @@  fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
 	         if chars_per_line is right, we probably just overflowed
 	         anyway; if it's wrong, let us keep going.  */
 	      if (wrap_column)
-		fputc_unfiltered ('\n', stream);
+		{
+		  emit_style_escape (ui_file_style ());
+		  fputc_unfiltered ('\n', stream);
+		}
 
 	      /* Possible new page.  Note that
 		 PAGINATION_DISABLED_FOR_COMMAND might be set during
@@ -1722,6 +1788,7 @@  fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
 	      if (wrap_column)
 		{
 		  fputs_unfiltered (wrap_indent, stream);
+		  emit_style_escape (desired_style);
 		  fputs_unfiltered (wrap_buffer.c_str (), stream);
 		  /* FIXME, this strlen is what prevents wrap_indent from
 		     containing tabs.  However, if we recurse to print it
diff --git a/gdb/utils.h b/gdb/utils.h
index 68523994b94..0c8a2e1da16 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -423,6 +423,10 @@  extern void fputstrn_unfiltered (const char *str, int n, int quotr,
 /* Return nonzero if filtered printing is initialized.  */
 extern int filtered_printing_initialized (void);
 
+class ui_file_style;
+extern void set_output_style (struct ui_file *stream,
+			      const ui_file_style &style);
+
 /* Display the host ADDR on STREAM formatted as ``0x%x''.  */
 extern void gdb_print_host_address_1 (const void *addr, struct ui_file *stream);