diff mbox

[RFA,1/4] Add highlight style, title style, fputs_highlighted. Improve 'show style'

Message ID 20190531131903.21203-2-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers May 31, 2019, 1:19 p.m. UTC 
  Have 'show style' and its subcommands using a style to style its output.
This allows the GDB user or developer to use 'show style' to visually see
with one command how all the current styles look like.

Add 2 new styles highlight style, title style and fputs_highlighted function.

Highlight style is used by fputs_highlighted to highlight the parts of
its char *STR argument that match a HIGHLIGHT regexp.
This (and the title style) will be used in a following patch.
---
 gdb/ChangeLog       |  14 ++++++
 gdb/cli/cli-style.c | 113 ++++++++++++++++++++++++++++++++------------
 gdb/cli/cli-style.h |  21 ++++++--
 gdb/utils.c         |  36 ++++++++++++++
 gdb/utils.h         |   6 +++
 5 files changed, 156 insertions(+), 34 deletions(-)

Comments

Tom Tromey May 31, 2019, 8:10 p.m. UTC | #1 
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Have 'show style' and its subcommands using a style to style its output.
Philippe> This allows the GDB user or developer to use 'show style' to visually see
Philippe> with one command how all the current styles look like.

Thanks.

Philippe> +static void
Philippe> +do_show (const char *what, struct ui_file *file,
Philippe> +	 struct cmd_list_element *cmd,
Philippe> +	 const char *value)

This needs some introductory comment.

Philippe> +{
Philippe> +  cli_style_option *cso = (cli_style_option *) get_cmd_context (cmd);
Philippe> +  fputs_filtered (_("The \""), file);
Philippe> +  fprintf_styled (file, cso->style (), "%s", cso->name ());

Can't this use fputs_styled instead?

It seems like the output might be a bit odd, in that the text will refer
to one aspect of the style, but it will be styled using the entire
style.

Philippe> +readibility."));

Typo, "readability".

Philippe> +      /* Output the part before pmatch with current style.  */
Philippe> +      while (pmatch.rm_so > 0)
Philippe> +	{
Philippe> +	  fputc_filtered (*str, stream);
Philippe> +	  pmatch.rm_so--;
Philippe> +	  str++;

Sometimes I wish the lower layers of the I/O system dealt with
string views instead of terminated strings...

I wondered if "highlight" was too generic a name, but I suppose it fits
well enough.  The only other idea I came up with was "search", which in
the end didn't really seem better.

Tom
Philippe Waroquiers June 1, 2019, 8:47 a.m. UTC | #2 
On Fri, 2019-05-31 at 14:10 -0600, Tom Tromey wrote:
> Philippe> +{
> Philippe> +  cli_style_option *cso = (cli_style_option *) get_cmd_context (cmd);
> Philippe> +  fputs_filtered (_("The \""), file);
> Philippe> +  fprintf_styled (file, cso->style (), "%s", cso->name ());
> 
> Can't this use fputs_styled instead?
> 
> It seems like the output might be a bit odd, in that the text will refer
> to one aspect of the style, but it will be styled using the entire
> style.

The idea is effectively to show the entire style in the output,
so as to see how all styles look like, close to each other.

To make it more clear, I have changed the output to be:
style address foreground:  The "address" style foreground color is: blue
                                ^^^^^^^^^^^^^^
So, I have added the word style after the style name and styled all what
is underlined.

Does that look less odd ?


> 
> Philippe> +readibility."));
> 
> Typo, "readability".
> 
> Philippe> +      /* Output the part before pmatch with current style.  */
> Philippe> +      while (pmatch.rm_so > 0)
> Philippe> +	{
> Philippe> +	  fputc_filtered (*str, stream);
> Philippe> +	  pmatch.rm_so--;
> Philippe> +	  str++;
> 
> Sometimes I wish the lower layers of the I/O system dealt with
> string views instead of terminated strings...
> 
> I wondered if "highlight" was too generic a name, but I suppose it fits
> well enough.  The only other idea I came up with was "search", which in
> the end didn't really seem better.
Yes, the name (for both title and highlight) were not straightforward
to choose.  The idea is that these styles should be relatively generic,
but not too much :).

The very first trial I did used the "header" instead of "title", but header
was too specific.

Otherwise, I hesitated between "attention" and "highlight" style.

Of course, easy to change if you deem "attention" (or something else)
is better.

Philippe
Tom Tromey June 3, 2019, 2:21 p.m. UTC | #3 
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> To make it more clear, I have changed the output to be:
Philippe> style address foreground:  The "address" style foreground color is: blue
Philippe>                                 ^^^^^^^^^^^^^^
Philippe> So, I have added the word style after the style name and styled all what
Philippe> is underlined.

Philippe> Does that look less odd ?

It seems like it should be fine; and anyway I suppose we can change it
if it looks weird in practice.

Tom
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index eec246de41..318275b34c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -7075,3 +7075,17 @@  version-control: never
 coding: utf-8
 End:
 
+2019-05-31  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* cli/cli-style.h (cli_style_option): Add name in constructor,
+	add m_name class member, add constructor with intensity,
+	add name class function.
+	(cli_style_option::add_setshow_commands): Remove name argument.
+	(highlight_style, title_style): New styles.
+	* cli/cli-style.c (do_show): New function that shows a style
+	characteristic styling the style name with itself.
+	(set_style_name): New function.
+	(STYLE_ADD_SETSHOW_COMMANDS): Remove NAME arguments.
+	Update all callers according to the changes in cli/cli-style.h.
+	* utils.h (fputs_highlighted): New function.
+	* utils.c (fputs_highlighted): Likewise.
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index f6f6c7be5d..7ea3777fec 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -61,29 +61,50 @@  static const char * const cli_intensities[] = {
 
 /* See cli-style.h.  */
 
-cli_style_option file_name_style (ui_file_style::GREEN);
+cli_style_option file_name_style ("filename", ui_file_style::GREEN);
 
 /* See cli-style.h.  */
 
-cli_style_option function_name_style (ui_file_style::YELLOW);
+cli_style_option function_name_style ("function", ui_file_style::YELLOW);
 
 /* See cli-style.h.  */
 
-cli_style_option variable_name_style (ui_file_style::CYAN);
+cli_style_option variable_name_style ("variable", ui_file_style::CYAN);
 
 /* See cli-style.h.  */
 
-cli_style_option address_style (ui_file_style::BLUE);
+cli_style_option address_style ("address", ui_file_style::BLUE);
 
 /* See cli-style.h.  */
 
-cli_style_option::cli_style_option (ui_file_style::basic_color fg)
-  : m_foreground (cli_colors[fg - ui_file_style::NONE]),
+cli_style_option highlight_style ("highlight", ui_file_style::RED);
+
+/* See cli-style.h.  */
+
+cli_style_option title_style ("title", ui_file_style::BOLD);
+
+/* See cli-style.h.  */
+
+cli_style_option::cli_style_option (const char *name,
+				    ui_file_style::basic_color fg)
+  : m_name (name),
+    m_foreground (cli_colors[fg - ui_file_style::NONE]),
     m_background (cli_colors[0]),
     m_intensity (cli_intensities[ui_file_style::NORMAL])
 {
 }
 
+/* See cli-style.h.  */
+
+cli_style_option::cli_style_option (const char *name,
+				    ui_file_style::intensity i)
+  : m_name (name),
+    m_foreground (cli_colors[0]),
+    m_background (cli_colors[0]),
+    m_intensity (cli_intensities[i])
+{
+}
+
 /* Return the color number corresponding to COLOR.  */
 
 static int
@@ -118,6 +139,17 @@  cli_style_option::style () const
   return ui_file_style (fg, bg, intensity);
 }
 
+static void
+do_show (const char *what, struct ui_file *file,
+	 struct cmd_list_element *cmd,
+	 const char *value)
+{
+  cli_style_option *cso = (cli_style_option *) get_cmd_context (cmd);
+  fputs_filtered (_("The \""), file);
+  fprintf_styled (file, cso->style (), "%s", cso->name ());
+  fprintf_filtered (file, _("\" %s is: %s\n"), what, value);
+}
+
 /* See cli-style.h.  */
 
 void
@@ -125,9 +157,7 @@  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);
+  do_show (_("foreground color"), file, cmd, value);
 }
 
 /* See cli-style.h.  */
@@ -137,9 +167,7 @@  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);
+  do_show (_("background color"), file, cmd, value);
 }
 
 /* See cli-style.h.  */
@@ -149,16 +177,13 @@  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);
+  do_show (_("display intensity"), file, cmd, value);
 }
 
 /* See cli-style.h.  */
 
 void
-cli_style_option::add_setshow_commands (const char *name,
-					enum command_class theclass,
+cli_style_option::add_setshow_commands (enum command_class theclass,
 					const char *prefix_doc,
 					struct cmd_list_element **set_list,
 					void (*do_set) (const char *args,
@@ -167,12 +192,12 @@  cli_style_option::add_setshow_commands (const char *name,
 					void (*do_show) (const char *args,
 							 int from_tty))
 {
-  m_set_prefix = std::string ("set style ") + name + " ";
-  m_show_prefix = std::string ("show style ") + name + " ";
+  m_set_prefix = std::string ("set style ") + m_name + " ";
+  m_show_prefix = std::string ("show style ") + m_name + " ";
 
-  add_prefix_cmd (name, no_class, do_set, prefix_doc, &m_set_list,
+  add_prefix_cmd (m_name, no_class, do_set, prefix_doc, &m_set_list,
 		  m_set_prefix.c_str (), 0, set_list);
-  add_prefix_cmd (name, no_class, do_show, prefix_doc, &m_show_list,
+  add_prefix_cmd (m_name, no_class, do_show, prefix_doc, &m_show_list,
 		  m_show_prefix.c_str (), 0, show_list);
 
   add_setshow_enum_cmd ("foreground", theclass, cli_colors,
@@ -182,7 +207,7 @@  cli_style_option::add_setshow_commands (const char *name,
 			nullptr,
 			nullptr,
 			do_show_foreground,
-			&m_set_list, &m_show_list, (void *) name);
+			&m_set_list, &m_show_list, (void *) this);
   add_setshow_enum_cmd ("background", theclass, cli_colors,
 			&m_background,
 			_("Set the background color for this property"),
@@ -190,7 +215,7 @@  cli_style_option::add_setshow_commands (const char *name,
 			nullptr,
 			nullptr,
 			do_show_background,
-			&m_set_list, &m_show_list, (void *) name);
+			&m_set_list, &m_show_list, (void *) this);
   add_setshow_enum_cmd ("intensity", theclass, cli_intensities,
 			&m_intensity,
 			_("Set the display intensity for this property"),
@@ -198,7 +223,7 @@  cli_style_option::add_setshow_commands (const char *name,
 			nullptr,
 			nullptr,
 			do_show_intensity,
-			&m_set_list, &m_show_list, (void *) name);
+			&m_set_list, &m_show_list, (void *) this);
 }
 
 static cmd_list_element *style_set_list;
@@ -245,6 +270,18 @@  show_style_sources (struct ui_file *file, int from_tty,
     fprintf_filtered (file, _("Source code styling is disabled.\n"));
 }
 
+/* Builds the "set style NAME " prefix.  */
+
+static std::string
+set_style_name (const char *name)
+{
+  std::string result ("set style ");
+
+  result += name;
+  result += " ";
+  return result;
+}
+
 void
 _initialize_cli_style ()
 {
@@ -278,14 +315,14 @@  it was not linked against GNU Source Highlight."
 			   ), set_style_enabled, show_style_sources,
 			   &style_set_list, &style_show_list);
 
-#define STYLE_ADD_SETSHOW_COMMANDS(STYLE, NAME, PREFIX_DOC)	  \
-  STYLE.add_setshow_commands (NAME, no_class, PREFIX_DOC,		\
+#define STYLE_ADD_SETSHOW_COMMANDS(STYLE, PREFIX_DOC)	  \
+  STYLE.add_setshow_commands (no_class, PREFIX_DOC,		\
 			      &style_set_list,				\
 			      [] (const char *args, int from_tty)	\
 			      {						\
 				help_list				\
 				  (STYLE.set_list (),			\
-				   "set style " NAME " ",		\
+				   set_style_name (STYLE.name ()).c_str (), \
 				   all_commands,			\
 				   gdb_stdout);				\
 			      },					\
@@ -298,23 +335,37 @@  it was not linked against GNU Source Highlight."
 				   "");					\
 			      })
 
-  STYLE_ADD_SETSHOW_COMMANDS (file_name_style, "filename",
+  STYLE_ADD_SETSHOW_COMMANDS (file_name_style,
 			      _("\
 Filename display styling\n\
 Configure filename colors and display intensity."));
 
-  STYLE_ADD_SETSHOW_COMMANDS (function_name_style, "function",
+  STYLE_ADD_SETSHOW_COMMANDS (function_name_style,
 			      _("\
 Function name display styling\n\
 Configure function name colors and display intensity"));
 
-  STYLE_ADD_SETSHOW_COMMANDS (variable_name_style, "variable",
+  STYLE_ADD_SETSHOW_COMMANDS (variable_name_style,
 			      _("\
 Variable name display styling\n\
 Configure variable name colors and display intensity"));
 
-  STYLE_ADD_SETSHOW_COMMANDS (address_style, "address",
+  STYLE_ADD_SETSHOW_COMMANDS (address_style,
 			      _("\
 Address display styling\n\
 Configure address colors and display intensity"));
+
+  STYLE_ADD_SETSHOW_COMMANDS (title_style,
+			      _("\
+Title display styling\n\
+Configure title colors and display intensity\n\
+Some commands (such as \"apropos -v REGEXP\") use the title style to improve\n\
+readibility."));
+
+  STYLE_ADD_SETSHOW_COMMANDS (highlight_style,
+			      _("\
+Highlight display styling\n\
+Configure highlight colors and display intensity\n\
+Some commands use the highlight style to draw the attention to a part\n\
+of their output."));
 }
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index c0520f9e23..6ae265e6ef 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -28,15 +28,20 @@  class cli_style_option
 public:
 
   /* Construct a CLI style option with a foreground color.  */
-  cli_style_option (ui_file_style::basic_color fg);
+  cli_style_option (const char *name, ui_file_style::basic_color fg);
+
+  /* Construct a CLI style option with an intensity.  */
+  cli_style_option (const char *name, ui_file_style::intensity i);
 
   /* Return a ui_file_style corresponding to the settings in this CLI
      style.  */
   ui_file_style style () const;
 
+  /* Return the style name.  */
+  const char *name () { return m_name; };
+
   /* Call once to register this CLI style with the CLI engine.  */
-  void add_setshow_commands (const char *name,
-			     enum command_class theclass,
+  void add_setshow_commands (enum command_class theclass,
 			     const char *prefix_doc,
 			     struct cmd_list_element **set_list,
 			     void (*do_set) (const char *args, int from_tty),
@@ -52,6 +57,9 @@  public:
 
 private:
 
+  /* The style name.  */
+  const char *m_name;
+
   /* The foreground.  */
   const char *m_foreground;
   /* The background.  */
@@ -93,6 +101,13 @@  extern cli_style_option variable_name_style;
 /* The address style.  */
 extern cli_style_option address_style;
 
+/* The highlight style.  */
+extern cli_style_option highlight_style;
+
+/* The title style.  */
+extern cli_style_option title_style;
+
+
 /* True if source styling is enabled.  */
 extern int source_styling;
 
diff --git a/gdb/utils.c b/gdb/utils.c
index 9686927473..9d7f8fffc4 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1869,6 +1869,42 @@  fputs_styled (const char *linebuffer, const ui_file_style &style,
     }
 }
 
+/* See utils.h.  */
+
+void
+fputs_highlighted (const char *str, compiled_regex &highlight,
+		   struct ui_file *stream)
+{
+  regmatch_t pmatch;
+
+  while (*str && highlight.exec (str, 1, &pmatch, 0) == 0)
+    {
+      size_t n_highlight = pmatch.rm_eo - pmatch.rm_so;
+
+      /* Output the part before pmatch with current style.  */
+      while (pmatch.rm_so > 0)
+	{
+	  fputc_filtered (*str, stream);
+	  pmatch.rm_so--;
+	  str++;
+	}
+
+      /* Output pmatch with the highlight style.  */
+      set_output_style (stream, highlight_style.style ());
+      while (n_highlight > 0)
+	{
+	  fputc_filtered (*str, stream);
+	  n_highlight--;
+	  str++;
+	}
+      set_output_style (stream, ui_file_style ());
+    }
+
+  /* Output the trailing part of STR not matching HIGHLIGHT.  */
+  if (*str)
+    fputs_filtered (str, stream);
+}
+
 int
 putchar_unfiltered (int c)
 {
diff --git a/gdb/utils.h b/gdb/utils.h
index 76c10049a7..a06f41b3e5 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -436,6 +436,12 @@  extern void fputs_styled (const char *linebuffer,
 			  const ui_file_style &style,
 			  struct ui_file *stream);
 
+/* Like fputs_styled, but uses highlight_style to highlight the
+   parts of STR that matches HIGHLIGHT.  */
+
+extern void fputs_highlighted (const char *str, compiled_regex &highlight,
+			       struct ui_file *stream);
+
 /* Reset the terminal style to the default, if needed.  */
 
 extern void reset_terminal_style (struct ui_file *stream);