[RFA 0/4] Improve "show style", use style in "help" and "apropos".

[RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Philippe Waroquiers]

Improve "show style", have "help" and "apropos" styling their output.

This patch series improves the "show style" output to let the
user visualize all the styles with one command.

"help" and "apropos" commands are now styling their output.

The "help" testing is also better factorized.

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

[Philippe Waroquiers]

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(-)

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);
-- 
2.20.1

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

[Tom Tromey]

>>>>> "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

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

[Philippe Waroquiers]

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

 

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

[Tom Tromey]

>>>>> "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

[RFA 3/4] Update tests following changes to "help" and "apropos"

[Philippe Waroquiers]

Factorizes the testing of the help output, by having a single place
that defines the common help trailer and/or prefix messages.
---
 gdb/testsuite/ChangeLog              | 13 +++++++
 gdb/testsuite/gdb.base/help.exp      | 30 ++++++++--------
 gdb/testsuite/gdb.guile/scm-cmd.exp  |  4 +--
 gdb/testsuite/gdb.python/py-cmd.exp  |  2 +-
 gdb/testsuite/gdb.trace/tracecmd.exp |  2 +-
 gdb/testsuite/lib/gdb.exp            | 51 +++++++++++++++++++++-------
 6 files changed, 69 insertions(+), 33 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 661c93860c..7b485a8f6b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -16111,3 +16111,16 @@ For older changes see ChangeLog-1993-2013.
     Copyright 2014-2019 Free Software Foundation, Inc.
   Copying and distribution of this file, with or without modification,
   are permitted provided the copyright notice and this notice are preserved.
+
+2019-05-31  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* gdb.lib/gdb.exp (help_list_trailer): New regexp variable
+	factorizing the help trailer message.
+	(test_class_help): Use help_list_trailer.  Add LIST_OF_COMMANDS
+	arg to allow to better factorize various tests.
+	(test_user_defined_class_help): Factorizes testing 'help user-defined'.
+	(test_prefix_command_help): Use help_list_trailer.
+	* gdb.python/py-cmd.exp: Use test_user_defined_class_help.
+	* gdb.guile/scm-cmd.exp: Likewise.
+	* gdb.trace/tracecmd.exp: Update caller of test_class_help.
+	* gdb.base/help.exp: Likewise.  Use test_user_defined_class_help.
diff --git a/gdb/testsuite/gdb.base/help.exp b/gdb/testsuite/gdb.base/help.exp
index 511e7d6238..0a8ad8e54d 100644
--- a/gdb/testsuite/gdb.base/help.exp
+++ b/gdb/testsuite/gdb.base/help.exp
@@ -25,37 +25,35 @@ gdb_start
 gdb_test_no_output "set height 0" "disable pagination"
 
 # Test all the help classes.
-test_class_help "aliases" {"Aliases of other commands\.\[\r\n\]+"}
+test_class_help "aliases" {"Aliases of other commands\.\[\r\n\]+"} {}
 test_class_help "breakpoints" {
     "Making program stop at certain points\.\[\r\n\]+"
-}
-test_class_help "data" {"Examining data\.\[\r\n\]+"}
-test_class_help "files" {"Specifying and examining files\.\[\r\n\]+"}
+} {}
+test_class_help "data" {"Examining data\.\[\r\n\]+"} {}
+test_class_help "files" {"Specifying and examining files\.\[\r\n\]+"} {}
 test_class_help "internals" {
     "Maintenance commands\.\[\r\n\]+"
     "Some gdb commands are provided just for use by gdb maintainers\.\[\r\n\]+"
     "These commands are subject to frequent change, and may not be as\[\r\n\]+"
     "well documented as user commands\.\[\r\n\]+"
-}
-test_class_help "obscure" {"Obscure features\.\[\r\n\]+"}
-test_class_help "running" {"Running the program\.\[\r\n\]+"}
+} {}
+test_class_help "obscure" {"Obscure features\.\[\r\n\]+"} {}
+test_class_help "running" {"Running the program\.\[\r\n\]+"} {}
 test_class_help "stack" {
     "Examining the stack\..*\[\r\n\]+"
     "When the program being debugged stops, gdb selects the innermost frame\.\[\r\n\]+"
     "The commands below can be used to select other frames by number or address\.\[\r\n\]+"
-}
+} {}
 test_class_help "status" {
     "Status inquiries\.\[\r\n\]+"
-}
-test_class_help "support" {"Support facilities\.\[\r\n\]+"}
+} {}
+test_class_help "support" {"Support facilities\.\[\r\n\]+"} {}
 test_class_help "tracepoints" {
     "Tracing of program execution without stopping the program\.\[\r\n\]+"
-}
-test_class_help "user-defined" {
-    "User-defined commands\.\[\r\n\]+"
-    "The commands in this class are those defined by the user\.\[\r\n\]+"
-    "Use the \"define\" command to define a command\.\[\r\n\]+"
-}
+} {}
+
+# Test "help user-defined", not checking user-defined command list.
+test_user_defined_class_help {}
 
 # Test help of an abbreviated command.  "break" is picked at random.
 set help_breakpoint_text "Set breakpoint at specified location\..*"
diff --git a/gdb/testsuite/gdb.guile/scm-cmd.exp b/gdb/testsuite/gdb.guile/scm-cmd.exp
index 3f24df3c9e..518da660c2 100644
--- a/gdb/testsuite/gdb.guile/scm-cmd.exp
+++ b/gdb/testsuite/gdb.guile/scm-cmd.exp
@@ -130,9 +130,7 @@ gdb_test "test-help ugh" "test-cmd output, arg = ugh" \
     "call simple user-defined command"
 
 # Make sure the command shows up in `help user-defined`.
-gdb_test "help user-defined" \
-    "User-defined commands.\[\r\n\]+The commands in this class are those defined by the user.\[\r\n\]+Use the \"define\" command to define a command.\[\r\n\]+List of commands:\[\r\n\]+test-help -- Docstring\[\r\n\]+Type \"help\" followed by command name for full documentation.\[\r\n\]+Type \"apropos word\" to search for commands related to \"word\".\[\r\n\]+Command name abbreviations are allowed if unambiguous.\[\r\n\]+" \
-    "see user-defined command in `help user-defined`"
+test_user_defined_class_help {"test-help -- Docstring[\r\n]"}
 
 # Make sure the command does not show up in `show user`.
 gdb_test "show user test-help" "Not a user command\." \
diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp
index 9abdb0f39e..c7c7b8c899 100644
--- a/gdb/testsuite/gdb.python/py-cmd.exp
+++ b/gdb/testsuite/gdb.python/py-cmd.exp
@@ -159,7 +159,7 @@ gdb_py_test_multiple "input simple user-defined command" \
 gdb_test "test_help ugh" "test_cmd output, arg = ugh" "call simple user-defined command"
 
 # Make sure the command shows up in `help user-defined`.
-gdb_test "help user-defined" "User-defined commands.\[\r\n\]+The commands in this class are those defined by the user.\[\r\n\]+Use the \"define\" command to define a command.\[\r\n\]+\[\r\n\]+List of commands:\[\r\n\]+\[\r\n\]+test_help -- Docstring\[\r\n\]+\[\r\n\]+Type \"help\" followed by command name for full documentation.\[\r\n\]+Type \"apropos word\" to search for commands related to \"word\".\[\r\n\]+Command name abbreviations are allowed if unambiguous.\[\r\n\]+" "see user-defined command in `help user-defined`"
+test_user_defined_class_help {"test_help -- Docstring[\r\n]"}
 
 # Make sure the command does not show up in `show user`.
 gdb_test "show user test_help" "Not a user command\." \
diff --git a/gdb/testsuite/gdb.trace/tracecmd.exp b/gdb/testsuite/gdb.trace/tracecmd.exp
index 6aa3eb18a4..05350665db 100644
--- a/gdb/testsuite/gdb.trace/tracecmd.exp
+++ b/gdb/testsuite/gdb.trace/tracecmd.exp
@@ -55,7 +55,7 @@ set testline2 [expr $baseline + 3]
 set helpcnt 0
 test_class_help "tracepoints" {
     "Tracing of program execution without stopping the program\.[\r\n\]+" 
-} "1.0: help tracepoints"
+} {} "1.0: help tracepoints"
 
 #
 # test trace command:
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index c703a7e633..d11a95c442 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5561,24 +5561,52 @@ proc help_test_raw { gdb_command expected_lines args } {
     gdb_test "${gdb_command}" "${expected_output}" $message
 }
 
-# Test the output of "help COMMAND_CLASS". EXPECTED_INITIAL_LINES
+# A regexp that matches the end of help CLASS|PREFIX_COMMAND
+set help_list_trailer {
+    "Type \"apropos word\" to search for commands related to \"word\"\.[\r\n]+"
+    "Type \"apropos -v word\" for full documentation of commands related to \"word\"\.[\r\n]+"
+    "Command name abbreviations are allowed if unambiguous\."
+}
+
+# Test the output of "help COMMAND_CLASS".  EXPECTED_INITIAL_LINES
 # are regular expressions that should match the beginning of output,
-# before the list of commands in that class.  The presence of 
-# command list and standard epilogue will be tested automatically.
+# before the list of commands in that class.
+# LIST_OF_COMMANDS are regular expressions that should match the
+# list of commands in that class.  If empty, the command list will be
+# matched automatically.  The presence of standard epilogue will be tested
+# automatically.
 # Notice that the '[' and ']' characters don't need to be escaped for strings
 # wrapped in {} braces.
-proc test_class_help { command_class expected_initial_lines args } {
+proc test_class_help { command_class expected_initial_lines list_of_commands args } {
+    global help_list_trailer
+    if {[llength $list_of_commands]>0} {
+	set l_list_of_commands {"List of commands:[\r\n]+[\r\n]+"}
+        set l_list_of_commands [concat $l_list_of_commands $list_of_commands]
+	set l_list_of_commands [concat $l_list_of_commands {"[\r\n]+[\r\n]+"}]
+    } else {
+        set l_list_of_commands {"List of commands\:.*[\r\n]+"}
+    }
     set l_stock_body {
-        "List of commands\:.*[\r\n]+"
         "Type \"help\" followed by command name for full documentation\.[\r\n]+"
-        "Type \"apropos word\" to search for commands related to \"word\"\.[\r\n]+"
-        "Command name abbreviations are allowed if unambiguous\." 
     }
-    set l_entire_body [concat $expected_initial_lines $l_stock_body]
+    set l_entire_body [concat $expected_initial_lines $l_list_of_commands \
+		       $l_stock_body $help_list_trailer]
 
     eval [list help_test_raw "help ${command_class}" $l_entire_body] $args
 }
 
+# Like test_class_help but specialised to test "help user-defined".
+proc test_user_defined_class_help { list_of_commands args } {
+    test_class_help "user-defined" {
+	"User-defined commands\.[\r\n]+"
+	"The commands in this class are those defined by the user\.[\r\n]+"
+	"Use the \"define\" command to define a command\.[\r\n]+"
+    } $list_of_commands [list $args]
+    # Unclear why, but if [list $args] is rather $args, the above is tested
+    # but gives no PASS message (but gives a FAIL message if it fails) ???
+}
+
+
 # COMMAND_LIST should have either one element -- command to test, or
 # two elements -- abbreviated command to test, and full command the first
 # element is abbreviation of.
@@ -5587,6 +5615,7 @@ proc test_class_help { command_class expected_initial_lines args } {
 # before the list of subcommands.  The presence of 
 # subcommand list and standard epilogue will be tested automatically.
 proc test_prefix_command_help { command_list expected_initial_lines args } {
+    global help_list_trailer
     set command [lindex $command_list 0]   
     if {[llength $command_list]>1} {        
         set full_command [lindex $command_list 1]
@@ -5597,10 +5626,8 @@ proc test_prefix_command_help { command_list expected_initial_lines args } {
     # be expanded in this list.
     set l_stock_body [list\
          "List of $full_command subcommands\:.*\[\r\n\]+"\
-         "Type \"help $full_command\" followed by $full_command subcommand name for full documentation\.\[\r\n\]+"\
-         "Type \"apropos word\" to search for commands related to \"word\"\.\[\r\n\]+"\
-         "Command name abbreviations are allowed if unambiguous\."]
-    set l_entire_body [concat $expected_initial_lines $l_stock_body]
+         "Type \"help $full_command\" followed by $full_command subcommand name for full documentation\.\[\r\n\]+"]
+    set l_entire_body [concat $expected_initial_lines $l_stock_body $help_list_trailer]
     if {[llength $args]>0} {
         help_test_raw "help ${command}" $l_entire_body [lindex $args 0]
     } else {
-- 
2.20.1

Re: [RFA 3/4] Update tests following changes to "help" and "apropos"

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Factorizes the testing of the help output, by having a single place
Philippe> that defines the common help trailer and/or prefix messages.

Philippe> -test_class_help "aliases" {"Aliases of other commands\.\[\r\n\]+"}
Philippe> +test_class_help "aliases" {"Aliases of other commands\.\[\r\n\]+"} {}

Wow, test_class_help is ugly!

Or more precisely, help_test_raw is pretty bad, following the "args"
convention that is confusing:

    proc help_test_raw { gdb_command expected_lines args } {
        set message $gdb_command
        if [llength $args]>0 then {
            set message [lindex $args 0]
        } 

It seems like it would be way better to just have an optional argument
there.

Anyway, could the new argument to test_class_help be optional?  Then
some of the changes, like the one above, would not be needed.  It could
also change from using args to using an optional final param:

    proc test_class_help {command_class expected_initial_lines {list_of_commands {}} {test_name {}}} {

Then at the end instead of eval it could just make a direct call:

-    eval [list help_test_raw "help ${command_class}" $l_entire_body] $args
+    help_test_raw "help ${command_class}" $l_entire_body $test_name

To make this work, the help_test_raw change would have to be made.

Philippe> +# Like test_class_help but specialised to test "help user-defined".
Philippe> +proc test_user_defined_class_help { list_of_commands args } {
Philippe> +    test_class_help "user-defined" {
Philippe> +	"User-defined commands\.[\r\n]+"
Philippe> +	"The commands in this class are those defined by the user\.[\r\n]+"
Philippe> +	"Use the \"define\" command to define a command\.[\r\n]+"
Philippe> +    } $list_of_commands [list $args]
Philippe> +    # Unclear why, but if [list $args] is rather $args, the above is tested
Philippe> +    # but gives no PASS message (but gives a FAIL message if it fails) ???

With those changes you could make an optional name parameter here,
and then remove the [list] and the comment.

Tom

[RFA 4/4] Document in NEWS and gdb.texinfo the "help", "apropos" and "show style" changes.

[Philippe Waroquiers]

---
 gdb/ChangeLog       |  5 +++++
 gdb/NEWS            | 30 ++++++++++++++++++++++++++
 gdb/doc/ChangeLog   |  6 ++++++
 gdb/doc/gdb.texinfo | 52 +++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7018c3347a..ce226c1808 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -7100,3 +7100,8 @@ End:
 	(print_help_for_command): Style the command name using title style.
 	* cli/cli-cmds.c (apropos_command): Parse optional -v flag.
 	(_initialize_cli_cmds): Describe -v in apropos_command help.
+
+2019-05-31  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* NEWS: Mention the new set|show style [title|highlight].
+	Mention changes to "show style", "help" and "apropos".
diff --git a/gdb/NEWS b/gdb/NEWS
index ab582c036d..7e03de1b89 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -57,6 +57,36 @@ set logging debugredirect [on|off]
   By default, GDB debug output will go to both the terminal and the logfile.
   Set if you want debug output to go only to the log file.
 
+set style title foreground COLOR
+set style title background COLOR
+set style title intensity VALUE
+  Control the styling of titles.
+
+set style highlight foreground COLOR
+set style highlight background COLOR
+set style highlight intensity VALUE
+  Control the styling of highlightings.
+
+* Changed commands
+
+help
+  The "help" command uses the title style to enhance the
+  readibility of its output by styling the classes and
+  command names.
+
+apropos [-v] REGEXP
+  Similarly to "help", the "apropos" command also uses the
+  title style for the command names.  "apropos" accepts now
+  a flag "-v" (verbose) to show the full documentation
+  of matching commands and to use the highlight style to mark
+  the documentation parts matching REGEXP.
+
+show style
+  The "show style" and its subcommands are now styling
+  a style name in their output using its own style, to help
+  the user visualize the different styles.
+
+
 * New MI commands
 
 -complete
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index c075d74eec..392569e497 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -12868,3 +12868,9 @@ fill-column: 74
 version-control: never
 coding: utf-8
 End:
+
+2019-05-31  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* gdb.texinfo (Help): Document the new -v apropos flag.
+	(Output Styling): Document the style name styling done by "show style".
+	Document the new title and highlight styles.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f2d8710d7d..1a5fbff056 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1831,10 +1831,13 @@ With a command name as @code{help} argument, @value{GDBN} displays a
 short paragraph on how to use that command.
 
 @kindex apropos
-@item apropos @var{args}
+@item apropos [-v] @var{regexp}
 The @code{apropos} command searches through all of the @value{GDBN}
 commands, and their documentation, for the regular expression specified in
-@var{args}.  It prints out all matches found.  For example:
+@var{args}.  It prints out all matches found.  The optional flag  @samp{-v},
+which stands for @samp{verbose}, indicates to output the full documentation
+of the matching commands and highlight the parts of the documentation
+matching @var{regexp}.  For example:
 
 @smallexample
 apropos alias
@@ -1853,6 +1856,29 @@ delete -- Delete some breakpoints or auto-display expressions
 @c @end group
 @end smallexample
 
+@noindent
+while
+
+@smallexample
+apropos -v cut.*thread apply
+@end smallexample
+
+@noindent
+results in the below output, where @samp{cut for 'thread apply}
+is highlighted if styling is enabled.
+
+@smallexample
+@c @group
+taas -- Apply a command to all threads (ignoring errors and empty output).
+Usage: taas COMMAND
+shortcut for 'thread apply all -s COMMAND'
+
+tfaas -- Apply a command to all frames of all threads (ignoring errors and empty output).
+Usage: tfaas COMMAND
+shortcut for 'thread apply all -s frame apply all -s COMMAND'
+@c @end group
+@end smallexample
+
 @kindex complete
 @item complete @var{args}
 The @code{complete @var{args}} command lists all the possible completions
@@ -24705,6 +24731,11 @@ Set the intensity to @var{value}.  Valid intensities are @samp{normal}
 (the default), @samp{bold}, and @samp{dim}.
 @end table
 
+The @code{show style} command and its subcommands are styling
+a style name in their output using its own style.
+So, use @command{show style} to see the complete list of styles,
+their characteristics and the visual aspect of each style.
+
 The style-able objects are:
 @table @code
 @item filename
@@ -24725,6 +24756,23 @@ foreground color is cyan.
 Control the styling of addresses.  These are managed with the
 @code{set style address} family of commands.  By default, this style's
 foreground color is blue.
+
+@item title
+Control the styling of titles.  These are managed with the
+@code{set style title} family of commands.  By default, this style's
+intensity is bold.  Commands are using the title style to improve
+the readibility of large output.  For example, the commands
+@command{apropos} and @command{help} are using the title style
+for the command names.
+
+@item highlight
+Control the styling of highlightings.  These are managed with the
+@code{set style highlight} family of commands.  By default, this style's
+foreground color is red.  Commands are using the highlight style to draw
+the user attention to some specific parts of their output.  For example,
+the command @command{apropos -v REGEXP} uses the highlight style to
+mark the documentation parts matching @var{regexp}.
+
 @end table
 
 @node Numbers
-- 
2.20.1

Re: [RFA 4/4] Document in NEWS and gdb.texinfo the "help", "apropos" and "show style" changes.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Fri, 31 May 2019 15:19:03 +0200
> 
> ---
>  gdb/ChangeLog       |  5 +++++
>  gdb/NEWS            | 30 ++++++++++++++++++++++++++
>  gdb/doc/ChangeLog   |  6 ++++++
>  gdb/doc/gdb.texinfo | 52 +++++++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 91 insertions(+), 2 deletions(-)

This part is OK, thanks.

[RFA 2/4] Improve usability and/or readibility of help and apropos output.

[Philippe Waroquiers]

The "help" command can output long list of command names or classes.
Use the title style to style the command names or classes to make
the output more readable.

Similarly, change "apropos" command to also style the command names.

It is sometimes unclear why "apropos REGEXP" lists some commands,
and then the user has to manually do 'help command' for all commands
listed by "apropos" to see more details about the matching commands.

=> Add an optional flag -v so that "apropos -v REGEXP":
  * outputs the full documentation of matching commands.
  * highlights the documentation parts matching REGEXP.
---
 gdb/ChangeLog        | 11 +++++++++
 gdb/cli/cli-cmds.c   | 22 ++++++++++++------
 gdb/cli/cli-decode.c | 55 +++++++++++++++++++++++++++++++++-----------
 gdb/cli/cli-decode.h |  1 +
 4 files changed, 69 insertions(+), 20 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 318275b34c..7018c3347a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -7089,3 +7089,14 @@ End:
 	Update all callers according to the changes in cli/cli-style.h.
 	* utils.h (fputs_highlighted): New function.
 	* utils.c (fputs_highlighted): Likewise.
+
+2019-05-31  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* cli/cli-decode.h (apropos_cmd): Add verbose argument.
+	* cli/cli-decode.c (apropos_cmd): Likewise.  Use print_doc_of_command
+	instead of print_help_for_command.
+	(print_doc_of_command): New function.
+	(help_list): Add 'apropos -v word' suggestion.
+	(print_help_for_command): Style the command name using title style.
+	* cli/cli-cmds.c (apropos_command): Parse optional -v flag.
+	(_initialize_cli_cmds): Describe -v in apropos_command help.
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index daf409a558..5f36814467 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1289,16 +1289,21 @@ show_user (const char *args, int from_tty)
 /* Search through names of commands and documentations for a certain
    regular expression.  */
 
-static void 
-apropos_command (const char *searchstr, int from_tty)
+static void
+apropos_command (const char *arg, int from_tty)
 {
-  if (searchstr == NULL)
+  bool verbose = arg && check_for_argument (&arg, "-v", 2);
+
+  if (verbose)
+    arg = skip_spaces (arg);
+
+  if (arg == NULL || *arg == '\0')
     error (_("REGEXP string is empty"));
 
-  compiled_regex pattern (searchstr, REG_ICASE,
+  compiled_regex pattern (arg, REG_ICASE,
 			  _("Error in regular expression"));
 
-  apropos_cmd (gdb_stdout, cmdlist, pattern, "");
+  apropos_cmd (gdb_stdout, cmdlist, verbose, pattern, "");
 }
 
 /* Subroutine of alias_command to simplify it.
@@ -1877,8 +1882,11 @@ Run the ``make'' program using the rest of the line as arguments."));
 Show definitions of non-python/scheme user defined commands.\n\
 Argument is the name of the user defined command.\n\
 With no argument, show definitions of all user defined commands."), &showlist);
-  add_com ("apropos", class_support, apropos_command,
-	   _("Search for commands matching a REGEXP"));
+  add_com ("apropos", class_support, apropos_command, _("\
+Search for commands matching a REGEXP\n\
+Usage: apropos [-v] REGEXP\n\
+Flag -v indicates to produce a verbose output, showing full documentation\n\
+of the matching commands."));
 
   add_setshow_uinteger_cmd ("max-user-call-depth", no_class,
 			   &max_user_call_depth, _("\
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 72e2a97009..07e36b3132 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -23,6 +23,7 @@
 #include "ui-out.h"
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
+#include "cli/cli-style.h"
 #include "common/gdb_optional.h"
 
 /* Prototypes for local functions.  */
@@ -937,13 +938,42 @@ add_com_suppress_notification (const char *name, enum command_class theclass,
 					&cmdlist, suppress_notification);
 }
 
+/* If VERBOSE, print the full help for command C and highlight the
+   documentation parts matching HIGHLIGHT,
+   otherwise print only one-line help for command C.  */
+
+static void
+print_doc_of_command (struct cmd_list_element *c, const char *prefix,
+		      bool verbose, compiled_regex &highlight,
+		      struct ui_file *stream)
+{
+  /* When printing the full documentation, add a line to separate
+     this documentation from the previous command help, in the likely
+     case that apropos finds several commands.  */
+  if (verbose)
+    fputs_filtered ("\n", stream);
+
+  fprintf_styled (stream, title_style.style (),
+		  "%s%s", prefix, c->name);
+  fputs_filtered (" -- ", stream);
+  if (verbose)
+    fputs_highlighted (c->doc, highlight, stream);
+  else
+    print_doc_line (stream, c->doc);
+  fputs_filtered ("\n", stream);
+}
+
 /* Recursively walk the commandlist structures, and print out the
    documentation of commands that match our regex in either their
    name, or their documentation.
+   If VERBOSE, prints the complete documentation and highlight the
+   documentation parts matching REGEX, otherwise prints only
+   the first line.
 */
-void 
-apropos_cmd (struct ui_file *stream, 
+void
+apropos_cmd (struct ui_file *stream,
 	     struct cmd_list_element *commandlist,
+	     bool verbose,
 	     compiled_regex &regex, const char *prefix)
 {
   struct cmd_list_element *c;
@@ -960,10 +990,7 @@ apropos_cmd (struct ui_file *stream,
 	  /* Try to match against the name.  */
 	  returnvalue = regex.search (c->name, name_len, 0, name_len, NULL);
 	  if (returnvalue >= 0)
-	    {
-	      print_help_for_command (c, prefix, 
-				      0 /* don't recurse */, stream);
-	    }
+	    print_doc_of_command (c, prefix, verbose, regex, stream);
 	}
       if (c->doc != NULL && returnvalue < 0)
 	{
@@ -971,10 +998,7 @@ apropos_cmd (struct ui_file *stream,
 
 	  /* Try to match against documentation.  */
 	  if (regex.search (c->doc, doc_len, 0, doc_len, NULL) >= 0)
-	    {
-	      print_help_for_command (c, prefix, 
-				      0 /* don't recurse */, stream);
-	    }
+	    print_doc_of_command (c, prefix, verbose, regex, stream);
 	}
       /* Check if this command has subcommands and is not an
 	 abbreviation.  We skip listing subcommands of abbreviations
@@ -983,7 +1007,7 @@ apropos_cmd (struct ui_file *stream,
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
-	  apropos_cmd (stream,*c->prefixlist,regex,c->prefixname);
+	  apropos_cmd (stream, *c->prefixlist, verbose, regex, c->prefixname);
 	}
     }
 }
@@ -1126,6 +1150,9 @@ Type \"help all\" for the list of all commands.");
   fputs_filtered ("documentation.\n", stream);
   fputs_filtered ("Type \"apropos word\" to search "
 		  "for commands related to \"word\".\n", stream);
+  fputs_filtered ("Type \"apropos -v word\" for full documentation", stream);
+  wrap_here ("");
+  fputs_filtered (" of commands related to \"word\".\n", stream);
   fputs_filtered ("Command name abbreviations are allowed if unambiguous.\n",
 		  stream);
 }
@@ -1212,10 +1239,12 @@ static void
 print_help_for_command (struct cmd_list_element *c, const char *prefix,
 			int recurse, struct ui_file *stream)
 {
-  fprintf_filtered (stream, "%s%s -- ", prefix, c->name);
+  fprintf_styled (stream, title_style.style (),
+		  "%s%s", prefix, c->name);
+  fputs_filtered (" -- ", stream);
   print_doc_line (stream, c->doc);
   fputs_filtered ("\n", stream);
-  
+
   if (recurse
       && c->prefixlist != 0
       && c->abbrev_flag == 0)
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index c53683d95c..f68b4c120f 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -249,6 +249,7 @@ extern void help_cmd_list (struct cmd_list_element *, enum command_class,
 extern void help_cmd (const char *, struct ui_file *);
 
 extern void apropos_cmd (struct ui_file *, struct cmd_list_element *,
+			 bool verbose,
                          compiled_regex &, const char *);
 
 /* Used to mark commands that don't do anything.  If we just leave the
-- 
2.20.1

Re: [RFA 2/4] Improve usability and/or readibility of help and apropos output.

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> The "help" command can output long list of command names or classes.
Philippe> Use the title style to style the command names or classes to make
Philippe> the output more readable.

Thanks.

Philippe>  extern void apropos_cmd (struct ui_file *, struct cmd_list_element *,
Philippe> +			 bool verbose,
Philippe>                           compiled_regex &, const char *);

It's more normal to reflow the lines here, like maybe combine the last two.

Tom

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Improve "show style", have "help" and "apropos" styling their output.
Philippe> This patch series improves the "show style" output to let the
Philippe> user visualize all the styles with one command.

Philippe> "help" and "apropos" commands are now styling their output.

Philippe> The "help" testing is also better factorized.

This all seems basically ok, just a few nits here and there.

However, it's missing any testing of the actual styling.  I think that
should be done.

Tom

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Pedro Alves]

On 5/31/19 2:18 PM, Philippe Waroquiers wrote:
> Improve "show style", have "help" and "apropos" styling their output.
> 
> This patch series improves the "show style" output to let the
> user visualize all the styles with one command.
> 
> "help" and "apropos" commands are now styling their output.
> 
> The "help" testing is also better factorized.

I tried this new feature today, and I admit that I felt a bit lost
with the output.  I think the reason why is that I have pagination
disabled in my .gdbinit, usually preferring to scroll up/down
than to press enter to move pagination to the next page.  So
while scrolling up, it isn't immediately obvious to me where
each of the command's docs are split.

(
 another use for "with":
  (gdb) with pagination off -- apropos -v breakpoint
)

I tried the patchlet below, to add a "-------" separator line
between commands, and to me, the guidelines help.
It also helps when styling is disabled, as with:

  (gdb) | with pagination off -- apropos -v breakpoint | less

WDYT?

From ce57941ee6e1240f1b7c16374d6f6c18ea12053e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 18 Jun 2019 13:25:15 +0100
Subject: [PATCH] apropos

---
 gdb/cli/cli-decode.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index a6ddd8cc6d8..ca5751dc2f6 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -25,6 +25,7 @@
 #include "cli/cli-decode.h"
 #include "cli/cli-style.h"
 #include "common/gdb_optional.h"
+#include "readline/readline.h"
 
 /* Prototypes for local functions.  */
 
@@ -970,7 +971,18 @@ print_doc_of_command (struct cmd_list_element *c, const char *prefix,
      this documentation from the previous command help, in the likely
      case that apropos finds several commands.  */
   if (verbose)
-    fputs_filtered ("\n", stream);
+    {
+      fputc_filtered ('\n', stream);
+
+      int width;
+      rl_get_screen_size (nullptr, &width);
+      if (width > 80)
+	width = 80;
+
+      for (int i = 0; i < width; i++)
+	fputc_filtered ('-', stream);
+      fputc_filtered ('\n', stream);
+    }
 
   fprintf_styled (stream, title_style.style (),
 		  "%s%s", prefix, c->name);
-- 
2.14.5

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I tried the patchlet below, to add a "-------" separator line
Pedro> between commands, and to me, the guidelines help.
Pedro> It also helps when styling is disabled, as with:

This seems reasonable to me.

Pedro> +      rl_get_screen_size (nullptr, &width);

I was going to ask why not chars_per_line, but I see that can be
UINT_MAX.

Tom

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Philippe Waroquiers]

On Tue, 2019-06-18 at 14:09 +0100, Pedro Alves wrote:
> I tried the patchlet below, to add a "-------" separator line
> between commands, and to me, the guidelines help.
> It also helps when styling is disabled, as with:
> 
>   (gdb) | with pagination off -- apropos -v breakpoint | less
> 
> WDYT?
Effectively, 'pagination off' and event worse 'style enabled off'
make it significantly more difficult to see where apropos -v 
starts the doc of a new command.

So, adding a separator line of ----------------- helps for such
cases.

I am just wondering if we then still need an empty line
before the ------ separator line.

I have a slight preference for the more compact one separator line
of ------------ (but of course, I will not have any nausea if we
keep the 2 lines separator).

Philippe

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

>> (gdb) | with pagination off -- apropos -v breakpoint | less

Philippe> Effectively, 'pagination off' and event worse 'style enabled off'
Philippe> make it significantly more difficult to see where apropos -v 
Philippe> starts the doc of a new command.

Maybe this command should still use styling?  I wonder.

Tom

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Philippe Waroquiers]

On Tue, 2019-06-18 at 14:46 -0600, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> > > (gdb) | with pagination off -- apropos -v breakpoint | less
> 
> Philippe> Effectively, 'pagination off' and event worse 'style enabled off'
> Philippe> make it significantly more difficult to see where apropos -v 
> Philippe> starts the doc of a new command.
> 
> Maybe this command should still use styling?  I wonder.

Wondering ...

At shell level, coloring seems to be disabled when piped.
E.g.  
  $ ls -l
  .... files with some colors
  $ ls -l | cat
  .... same output, but no colors
  $

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Philippe Waroquiers]

On Tue, 2019-06-18 at 22:52 +0200, Philippe Waroquiers wrote:
> On Tue, 2019-06-18 at 14:46 -0600, Tom Tromey wrote:
> > > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> > > > 
> > > > (gdb) | with pagination off -- apropos -v breakpoint | less
> > 
> > Philippe> Effectively, 'pagination off' and event worse 'style enabled off'
> > Philippe> make it significantly more difficult to see where apropos -v 
> > Philippe> starts the doc of a new command.
> > 
> > Maybe this command should still use styling?  I wonder.
> 
> Wondering ...
> 
> At shell level, coloring seems to be disabled when piped.
> E.g.  
>   $ ls -l
>   .... files with some colors
>   $ ls -l | cat
>   .... same output, but no colors
>   $

ls has an option to control coloring.
   ls -l --color=always|cat
will still be colored.

So, we might add an option -style [on|off] to the GDB pipe_command,
if we believe it should output styling sequence.

Philippe

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Pedro Alves]

On 6/18/19 9:57 PM, Philippe Waroquiers wrote:
>> Wondering ...
>>
>> At shell level, coloring seems to be disabled when piped.
>> E.g.  
>>   $ ls -l
>>   .... files with some colors
>>   $ ls -l | cat
>>   .... same output, but no colors
>>   $
> ls has an option to control coloring.
>    ls -l --color=always|cat
> will still be colored.
> 
> So, we might add an option -style [on|off] to the GDB pipe_command,
> if we believe it should output styling sequence.

Agreed.

In other words, this would let us pipe with style.  :-D

In addition, "set style enabled" could be made an auto-boolean
setting, with default "auto", enable styling iff outputting to
a terminal.  Then, "on" would force styling even if outputting
to a pipe.  This would let you force styling with:

 (gdb) with style enabled on -- | apropos -v breakpoint | less

Thanks,
Pedro Alves

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Philippe Waroquiers]

On Tue, 2019-06-18 at 23:40 +0100, Pedro Alves wrote:
> On 6/18/19 9:57 PM, Philippe Waroquiers wrote:
> > > Wondering ...
> > > 
> > > At shell level, coloring seems to be disabled when piped.
> > > E.g.  
> > >   $ ls -l
> > >   .... files with some colors
> > >   $ ls -l | cat
> > >   .... same output, but no colors
> > >   $
> > 
> > ls has an option to control coloring.
> >    ls -l --color=always|cat
> > will still be colored.
> > 
> > So, we might add an option -style [on|off] to the GDB pipe_command,
> > if we believe it should output styling sequence.
> 
> Agreed.
> 
> In other words, this would let us pipe with style.  :-D
> 
> In addition, "set style enabled" could be made an auto-boolean
> setting, with default "auto", enable styling iff outputting to
> a terminal.  Then, "on" would force styling even if outputting
> to a pipe.  This would let you force styling with:
> 
>  (gdb) with style enabled on -- | apropos -v breakpoint | less

When we have this 'set style enabled auto|on|off',
wondering if the user will often have to override the behavior
in the pipe command:
The global setting will be enough for most of the users,
and the 'with' command should be good enough for the
unfrequent need to override styling for the pipe command.


Note that I am envisaging to do a small addition
to the 'define' command so as to make it slightly
easier to write a 'define wrapper shortcut' around
a 'with command'.

Something like:
define Lc
  with language c -- $arg@
end
document Lc
  Usage: Lc [COMMAND [ARGS]...]
  Shortcut for:
     with language c -- [COMMAND [ARGS]...]
end

This new '$arg@' will allow a user (like me) that
prefers to type as little as possible to define
very small shortcuts for often used 'with commands',
without expanding all arguments inside the 'define'
command with a bunch of 'if's depending on $argc.

Note that interactively, the 'with' command
combines properly with the 'if' command.

But the 'define' parser seems to not understand that
a 'with' line can start an 'if' that needs a
corresponding 'end'.

In other words, interactively, the below works
(gdb) with language c -- if $xxx == 0
 >echo zero\n
 >else
 >echo not zero\n
 >end
zero
(gdb) 

but inside a 'define', the parser does not understand
the above:

(gdb) define nnnn
Type commands for definition of "nnnn".
End with a line saying just "end".
>with language c -- if $xxx == 0
>echo zero\n
>else
(gdb) show user nnnn
User command "nnnn":
(gdb) 

Maybe worth adding a 'mini with command' detector/parser
in the 'define' parser, so as to allow user defined
commands to be less dependent of the current language
when evaluating expressions.

Alternatively, we should allow alias to accept 'arguments':
Instead of:
   (gdb) alias Lc = with language c --
   Invalid command to alias to: with language c --
   (gdb)
GDB should do:
   (gdb) alias Lc = with language c --
   => Defined Lc as an alias of 'with' command, using prefix args 'language c --'
   (gdb)
and then
   (gdb) Lc somecommand somearg someotherarg
should be the equivalent of:
   (gdb) with language c -- somecommand somearg someotherarg

No idea if the above is easy to do.

I have in a corner a patch that allows to add default arguments to
a command or an alias, which gives more or less the above, but using
2 successive actions:
  (gdb) alias bt2 = bt
  (gdb) help add-args 
  Specify additional arguments to preprend to user provided command arguments.
  Usage:  add-args COMMAND [= ADDITIONAL_ARGS...]
  Allows to specify or clear the additional arguments automatically
  prepended to the user provided arguments when COMMAND is run.
  Note that COMMAND can be an alias.  Aliases and their aliased commands
  do not share their additional arguments, so you can specify different
  additional arguments for a command and for each of its aliases.
  Without the [= ADDITIONAL_ARGS...], clears COMMAND additional arguments.
  (gdb) add-args bt2 = 2
  (gdb) bt2
  #0  0x00007ffff78fe603 in select () at ../sysdeps/unix/syscall-template.S:84
  #1  0x0000555555554f5e in sleeper_or_burner (v=0x7fffffffdf30) at sleepers.c:86
  (More stack frames follow...)
  (gdb) 

(add-args allows to implement Lc and similar, but also allows
a user to specify 'default' arguments for any command (or alias).
So, maybe I should finalize an RFA for this add-args idea in preference
to the 'alias argument idea' and/or to the '$arg@' idea.

Also maybe 'alias xxx = somecommand someargs'
can just be a fast way to type:
     alias xxx = somecommand
     add-args xxx = someargs

Too many ideas, too little time ... :(

Philippe

NB: the above discussion can be summarized as
   'the return of the slash command abbreviations for those that liked them' :)

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Note that I am envisaging to do a small addition
Philippe> to the 'define' command so as to make it slightly
Philippe> easier to write a 'define wrapper shortcut' around
Philippe> a 'with command'.

Philippe> Something like:
Philippe> define Lc
Philippe>   with language c -- $arg@
Philippe> end

Philippe> But the 'define' parser seems to not understand that
Philippe> a 'with' line can start an 'if' that needs a
Philippe> corresponding 'end'.

I'm surprised "with if" is intended to work, but if it is, then that's
just a bug.

If it needs special handling for "if" it will probably also need it for
the other multi-line commands.

Philippe> Alternatively, we should allow alias to accept 'arguments':
Philippe> Instead of:
Philippe>    (gdb) alias Lc = with language c --
Philippe>    Invalid command to alias to: with language c --

I think that would be a reasonable addition.  I was surprised this
wasn't done initially.

Philippe> I have in a corner a patch that allows to add default arguments to
Philippe> a command or an alias, which gives more or less the above, but using
Philippe> 2 successive actions:
Philippe>   (gdb) alias bt2 = bt
Philippe>   (gdb) help add-args 
Philippe>   Specify additional arguments to preprend to user provided command arguments.
Philippe>   Usage:  add-args COMMAND [= ADDITIONAL_ARGS...]
Philippe>   Allows to specify or clear the additional arguments automatically
Philippe>   prepended to the user provided arguments when COMMAND is run.

I am not so sure about this.  It seems pretty obscure.  Maybe a more
gdb-ish way of doing this would be to introduce "set" parameters to
control the defaults of the relevant commands.

Philippe> So, maybe I should finalize an RFA for this add-args idea in preference
Philippe> to the 'alias argument idea' and/or to the '$arg@' idea.

I'm also not sure about the $arg@ idea, or maybe just the spelling of
it.

Tom

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> In other words, this would let us pipe with style.  :-D

Ouch.

Pedro> In addition, "set style enabled" could be made an auto-boolean
Pedro> setting, with default "auto", enable styling iff outputting to
Pedro> a terminal.  Then, "on" would force styling even if outputting
Pedro> to a pipe.  This would let you force styling with:

Pedro>  (gdb) with style enabled on -- | apropos -v breakpoint | less

I like this idea.

Tom

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Pedro Alves]

On 6/19/19 8:26 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> In other words, this would let us pipe with style.  :-D
> 
> Ouch.

???

/me wonders, surprised, what he might unknowingly might have said, and googles.

and... ouch...  I had no idea.  :-P

Always keep learning.

Pedro Alves

Re: [RFA 0/4] Improve "show style", use style in "help" and "apropos".

[Tom Tromey]

Pedro> In other words, this would let us pipe with style.  :-D

>> Ouch.

Pedro> ???
Pedro> /me wonders, surprised, what he might unknowingly might have said, and googles.
Pedro> and... ouch...  I had no idea.  :-P

Pedro> Always keep learning.

Haha.  It's nothing really, it just fell into "bad pun" territory for me.

Tom