[binutils-gdb] Fix/improve 'help CLASS' output

[binutils-gdb] Fix/improve 'help CLASS' output

[Philippe Waroquiers]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3b3aaacba15292f185b6e8ba5faba1ed89c9f908

commit 3b3aaacba15292f185b6e8ba5faba1ed89c9f908
Author: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date:   Sun May 10 18:31:03 2020 +0200

    Fix/improve 'help CLASS' output
    
    Currently, help CLASS possibly shows several times the same help,
    as it shows it once for the command, and once for each alias.
    
    The final objective of this patch series is to have class_alias used only
    for user defined aliases, not anymore for aliases predefined by GDB.
    The command 'help aliases' will then only show the user defined aliases.
    So, the idea is that GDB predefined aliases will be shown together
    with their aliased command.
    
    This commit changes 'help CLASS' so that a command is shown once in the output,
    with all its aliases.
    This ensures:
      * that the user has only to read once the same help text
      * and sees the command and all its aliases in a glance, a.o. allowing
        the user to choose the preferred way (e.g. the shortest one,
        or the most mnemonic one) to type the command.
    
    For example, the old output:
       (gdb) help stack
       ...
       List of commands:
    
       backtrace -- Print backtrace of all stack frames, or innermost COUNT frames.
       bt -- Print backtrace of all stack frames, or innermost COUNT frames.
       ...
    (note that 'where' is not shown in this output)
    
    becomes
       (gdb) help stack
       ...
       List of commands:
    
       backtrace, where, bt -- Print backtrace of all stack frames, or innermost COUNT frames.
       ...
    
    The output layout chosen is to have the command first, followed by all its
    aliases separated by a comma.  Note that the command and alias names are
    title-styled.  For sure, other layouts could be discussed, but this one is IMO
    readable and compact.
    
    The function 'help_cmd_list' can be simplified by removing the prefix argument,
    as the prefixname of a command can now be retrieved in the GDB command tree
    structure.
    
    This also fixes the fact that 'help aliases' wrongly shows a long
    list of (non-alias) when defining an alias for a prefix command.
    For example, after:
        (gdb) alias montre = show
      then
        (gdb) help aliases
      shows hundreds of sub-commands starting with the non aliased command,
      such as:
        montre -- Generic command for showing things about the debugger.
        show ada -- Generic command for showing Ada-specific settings.
        show ada print-signatures -- Show whether the output of formal ...
        ....
    
    'help_cmd_list' is also made static, as it is only used inside cli-decode.c.
    
    Note that the 'help CLASS' is somewhat broken, in the sense that it
    sometimes shows too many commands (commands not belonging to CLASS)
    and sometimes shows not enough commands (not showing some commands
    belonging to CLASS).
    For example, 'help breakpoints' shows the command
    'disable pretty-printer' and 'disable unwinder', not related to breakpoints.
    On the other end, 'help stack' does not show 'disable unwinder'
    while 'disable unwinder' is defined in unwinders.py as belonging to class_stack.
    Fixing the missing commands is easy to do,
    but fixing the excess commands is not straightforward, as many
    subcommands have a class 'no_class' or 'all_class'.
    Possibly, some of this might be improved/fixed in another patch series.
    
    With this patch series, the 'abbrev flag' has as only remaining purpose
    to avoid having the abbreviation alias appearing in the completion list,
    so change 'help alias' accordingly.
    
    gdb/ChangeLog
    
    2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
    
            * cli/cli-decode.h (help_cmd_list): Remove declaration.
            * cli/cli-decode.c (help_cmd_list): Declare as static,
            remove prefix argument, use bool for recurse arg, rework to show the aliases of
            a command together with the command.
            (fput_command_name_styled, fput_command_names_styled): New functions.
            (print_help_for_command): Remove prefix arg, use bool for recurse arg, use
            fput_command_name_styled.
            (help_list, help_all): Update callers to remove prefix arg and use bool recurse.
            * cli/cli-cmds.c (_initialize_cli_cmds): Update alias_command doc.
    
    gdb/testsuite/ChangeLog
    
    2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
    
            * gdb.base/alias.exp: Update help output check.

Diff:
---
 gdb/ChangeLog                    |  12 ++++
 gdb/cli/cli-cmds.c               |   2 +-
 gdb/cli/cli-decode.c             | 128 ++++++++++++++++++++++++++++++---------
 gdb/cli/cli-decode.h             |   3 -
 gdb/testsuite/ChangeLog          |   4 ++
 gdb/testsuite/gdb.base/alias.exp |   2 +-
 6 files changed, 118 insertions(+), 33 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 80c027df8ac..2298843350b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* cli/cli-decode.h (help_cmd_list): Remove declaration.
+	* cli/cli-decode.c (help_cmd_list): Declare as static,
+	remove prefix argument, use bool for recurse arg, rework to show the aliases of
+	a command together with the command.
+	(fput_command_name_styled, fput_command_names_styled): New functions.
+	(print_help_for_command): Remove prefix arg, use bool for recurse arg, use
+	fput_command_name_styled.
+	(help_list, help_all): Update callers to remove prefix arg and use bool recurse.
+	* cli/cli-cmds.c (_initialize_cli_cmds): Update alias_command doc.
+
 2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* cli/cli-setshow.h (cmd_show_list): Remove prefix argument.
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 8538fadd9c3..eb6e32b0469 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -2454,7 +2454,7 @@ Usage: alias [-a] [--] ALIAS = COMMAND\n\
 ALIAS is the name of the alias command to create.\n\
 COMMAND is the command being aliased to.\n\
 If \"-a\" is specified, the command is an abbreviation,\n\
-and will not appear in help command list output.\n\
+and will not be used in command completion.\n\
 \n\
 Examples:\n\
 Make \"spe\" an alias of \"set print elements\":\n\
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index c37dfaffb5c..093692e7ac0 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -43,6 +43,11 @@ static struct cmd_list_element *find_cmd (const char *command,
 					  int ignore_help_classes,
 					  int *nfound);
 
+static void help_cmd_list (struct cmd_list_element *list,
+			   enum command_class theclass,
+			   bool recurse,
+			   struct ui_file *stream);
+
 static void help_all (struct ui_file *stream);
 
 /* Look up a command whose 'prefixlist' is KEY.  Return the command if found,
@@ -76,8 +81,8 @@ lookup_cmd_for_prefixlist (struct cmd_list_element **key,
 }
 
 static void
-print_help_for_command (struct cmd_list_element *c, const char *prefix,
-			int recurse, struct ui_file *stream);
+print_help_for_command (struct cmd_list_element *c,
+			bool recurse, struct ui_file *stream);
 
 \f
 /* Set the callback function for the specified command.  For each both
@@ -1021,6 +1026,43 @@ add_com_suppress_notification (const char *name, enum command_class theclass,
 					&cmdlist, suppress_notification);
 }
 
+/* Print the prefix of C followed by name of C in title style.  */
+
+static void
+fput_command_name_styled (struct cmd_list_element *c, struct ui_file *stream)
+{
+  const char *prefixname
+    = c->prefix == nullptr ? "" : c->prefix->prefixname;
+
+  fprintf_styled (stream, title_style.style (), "%s%s", prefixname, c->name);
+}
+
+/* If C has one or more aliases, style print the name of C and
+   the name of its aliases, separated by commas.
+   If ALWAYS_FPUT_C_NAME, print the name of C even if it has no aliases.
+   If one or more names are printed, POSTFIX is printed after the last name.
+*/
+
+static void
+fput_command_names_styled (struct cmd_list_element *c,
+			   bool always_fput_c_name, const char *postfix,
+			   struct ui_file *stream)
+{
+  if (always_fput_c_name ||  c->aliases != nullptr)
+    fput_command_name_styled (c, stream);
+  if (c->aliases != nullptr)
+    {
+      for (cmd_list_element *iter = c->aliases; iter; iter = iter->alias_chain)
+	{
+	  fputs_filtered (", ", stream);
+	  wrap_here ("   ");
+	  fput_command_name_styled (iter, stream);
+	}
+    }
+  if (always_fput_c_name ||  c->aliases != nullptr)
+    fputs_filtered (postfix, stream);
+}
+
 /* 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.  */
@@ -1208,7 +1250,7 @@ help_list (struct cmd_list_element *list, const char *cmdtype,
   else
     fprintf_filtered (stream, "List of %scommands:\n\n", cmdtype2);
 
-  help_cmd_list (list, theclass, cmdtype, (int) theclass >= 0, stream);
+  help_cmd_list (list, theclass, theclass >= 0, stream);
 
   if (theclass == all_classes)
     {
@@ -1255,7 +1297,7 @@ help_all (struct ui_file *stream)
       if (c->func == NULL)
 	{
 	  fprintf_filtered (stream, "\nCommand class: %s\n\n", c->name);
-	  help_cmd_list (cmdlist, c->theclass, "", 1, stream);
+	  help_cmd_list (cmdlist, c->theclass, true, stream);
 	}
     }
 
@@ -1275,7 +1317,7 @@ help_all (struct ui_file *stream)
 	      fprintf_filtered (stream, "\nUnclassified commands\n\n");
 	      seen_unclassified = 1;
 	    }
-	  print_help_for_command (c, "", 1, stream);
+	  print_help_for_command (c, 1, stream);
 	}
     }
 
@@ -1327,12 +1369,10 @@ print_doc_line (struct ui_file *stream, const char *str,
    If RECURSE is non-zero, also print one-line descriptions
    of all prefixed subcommands.  */
 static void
-print_help_for_command (struct cmd_list_element *c, const char *prefix,
-			int recurse, struct ui_file *stream)
+print_help_for_command (struct cmd_list_element *c,
+			bool recurse, struct ui_file *stream)
 {
-  fprintf_styled (stream, title_style.style (),
-		  "%s%s", prefix, c->name);
-  fputs_filtered (" -- ", stream);
+  fput_command_names_styled (c, true, " -- ", stream);
   print_doc_line (stream, c->doc, false);
   fputs_filtered ("\n", stream);
 
@@ -1342,48 +1382,80 @@ print_help_for_command (struct cmd_list_element *c, const char *prefix,
     /* Subcommands of a prefix command typically have 'all_commands'
        as class.  If we pass CLASS to recursive invocation,
        most often we won't see anything.  */
-    help_cmd_list (*c->prefixlist, all_commands, c->prefixname, 1, stream);
+    help_cmd_list (*c->prefixlist, all_commands, true, stream);
 }
 
 /*
  * Implement a help command on command list LIST.
  * RECURSE should be non-zero if this should be done recursively on
  * all sublists of LIST.
- * PREFIX is the prefix to print before each command name.
  * STREAM is the stream upon which the output should be written.
  * THECLASS should be:
  *      A non-negative class number to list only commands in that
- * class.
  *      ALL_COMMANDS to list all commands in list.
  *      ALL_CLASSES  to list all classes in list.
  *
+ *   Note that aliases are only shown when THECLASS is class_alias.
+ *   In the other cases, the aliases will be shown together with their
+ *   aliased command.
+ *
  *   Note that RECURSE will be active on *all* sublists, not just the
  * ones selected by the criteria above (ie. the selection mechanism
  * is at the low level, not the high-level).
  */
-void
+
+static void
 help_cmd_list (struct cmd_list_element *list, enum command_class theclass,
-	       const char *prefix, int recurse, struct ui_file *stream)
+	       bool recurse, struct ui_file *stream)
 {
   struct cmd_list_element *c;
 
   for (c = list; c; c = c->next)
     {
-      if (c->abbrev_flag == 0
-	  && !c->cmd_deprecated
-	  && (theclass == all_commands
-	      || (theclass == all_classes && c->func == NULL)
-	      || (theclass == c->theclass && c->func != NULL)))
+      if (c->abbrev_flag == 1 || c->cmd_deprecated)
 	{
-	  print_help_for_command (c, prefix, recurse, stream);
+	  /* Do not show abbreviations or deprecated commands.  */
+	  continue;
 	}
-      else if (c->abbrev_flag == 0
-	       && recurse
-	       && !c->cmd_deprecated
-	       && theclass == class_user && c->prefixlist != NULL)
-	/* User-defined commands may be subcommands.  */
-	help_cmd_list (*c->prefixlist, theclass, c->prefixname,
-		       recurse, stream);
+
+      if (c->cmd_pointer != nullptr && theclass != class_alias)
+	{
+	  /* Do not show an alias, unless specifically showing the
+	     list of aliases:  for all other classes, an alias is
+	     shown (if needed) together with its aliased command.  */
+	  continue;
+	}
+
+      if (theclass == all_commands
+	  || (theclass == all_classes && c->func == NULL)
+	  || (theclass == c->theclass && c->func != NULL))
+	{
+	  /* show C when
+             - showing all commands
+	     - showing all classes and C is a help class
+	     - showing commands of THECLASS and C is not the help class  */
+
+	  /* If we show the class_alias and C is an alias, do not recurse,
+	     as this would show the (possibly very long) not very useful
+	     list of sub-commands of the aliased command.  */
+	  print_help_for_command
+	    (c,
+	     recurse && (theclass != class_alias || c->cmd_pointer == nullptr),
+	     stream);
+	  continue;
+	}
+
+      if (recurse
+	  && (theclass == class_user || theclass == class_alias)
+	  && c->prefixlist != NULL)
+	{
+	  /* User-defined commands or aliases may be subcommands.  */
+	  help_cmd_list (*c->prefixlist, theclass, recurse, stream);
+	  continue;
+	}
+
+      /* Do not show C or recurse on C, e.g. because C does not belong to
+	 THECLASS or because C is a help class.  */
     }
 }
 \f
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 4f7c7701e40..f4719bfac47 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -249,9 +249,6 @@ struct cmd_list_element
     int *suppress_notification = nullptr;
   };
 
-extern void help_cmd_list (struct cmd_list_element *, enum command_class,
-			   const char *, int, struct ui_file *);
-
 /* Functions that implement commands about CLI commands.  */
 
 extern void help_cmd (const char *, struct ui_file *);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index bff0ad05329..36afa3dbd3d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
+	* gdb.base/alias.exp: Update help output check.
+
+c2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
 	* gdb.base/default.exp: Update output following fixes.
 
 2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
diff --git a/gdb/testsuite/gdb.base/alias.exp b/gdb/testsuite/gdb.base/alias.exp
index 9a9557668e5..69cfde67c14 100644
--- a/gdb/testsuite/gdb.base/alias.exp
+++ b/gdb/testsuite/gdb.base/alias.exp
@@ -65,7 +65,7 @@ gdb_test "show print elements" "Limit .* is 50\[.\]" "verify spe"
 gdb_test_no_output "alias set pr elms = set p elem"
 gdb_test_no_output "set pr elms 51"
 gdb_test "show print elements" "Limit .* is 51\[.\]" "verify set pr elms"
-gdb_test "help set print" "set print elms .*"
+gdb_test "help set print" "set print elements, set print elms, spe .*"
 
 # Verify alias command detects a non existing prefix.
 gdb_test "alias assigne imprime limite-elements = set print elements" \