public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos
@ 2020-05-10 20:55 Philippe Waroquiers
  2020-05-10 20:55 ` [RFA 01/10] Add a selftest that detects a 'corrupted' command tree structure in GDB Philippe Waroquiers
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

This patch series started with the objective to cleanup the 'class_alias'
usages, so as to have this class only used for user-defined aliases.

While doing the above change, various problems were then discovered in the
command list structure and command help.

So, this patch series fixes these problems, to end up with the
removal of the usage of 'class_alias' for the aliases predefined
by GDB.

Using 'class_alias' only for user-defined aliases was deemed the best approach
to handle the comment of Simon related to "default args for commands and
aliases" that pre-defined commands and aliases should not have their default
changeable via default args. Otherwise, the user would be confused by having
e.g. 'backtrace' and 'bt' behaving differently, while most users
will assume these are the same commands.
So, the patch series "default args for commands and aliases" must differentiate
predefined aliases from user defined aliases, and the idea is to reserve
class_alias for these user-defined aliases.

Note that the concept of command classes in GDB is only used for
the help system:  typing "help" without arguments gives a list of
command classes.  Typing "help <classname>" is (supposed to) list
the commands having that class.

"help aliases" is today giving a very strange functionality.
It only shows a very small subset of the GDB predefined aliases
(e.g. it shows none of the abbreviation aliases) and does
not show some other aliases (such as 'disable breakpoints').
Various other strange behaviour such as: when defining the alias:
   alias deactive = disable
then 'help aliases' shows this new alias (this is normal)
but also starts to show a whole bunch of 'disable' subcommands,
which is all pretty useless in the list of aliases:

  (gdb) help aliases
  Aliases of other commands.

  List of commands:

  deactive -- Disable all or some breakpoints.
  disable breakpoints -- Disable all or some breakpoints.
  disable display -- Disable some expressions to be displayed when program stops.
  disable frame-filter -- GDB command to disable the specified frame-filter.
  ...
So, a bunch of "disable" subcommands are appearing due to that.
It is interesting to try "help aliases" after having done:
  "alias is = info set"

So, the idea is to only show the user defined aliases in "help aliases",
and show the predefined aliases together with the help of the command
they are aliasing.
This is deemed to help the user to see how to use aliases to type faster
the commands they are asking some help for, compared to the current
very partial list of aliases shown by "help aliases".

Note that if deemed necessary, we could implement a (fake) class
  "help predefined-aliases"
that would show the GDB predefined aliases, if ever that would be
deemed needed.

Note also that only remaining usage of 'abbreviation alias' concept is
to avoid completion proposing an abbreviation alias.  The other usage
to not shown an alias in the GDB help system is not needed anymore,
as now aliases and abbreviations are shown as part of the aliased
command help.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 01/10] Add a selftest that detects a 'corrupted' command tree structure in GDB.
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 15:51   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 02/10] Fix the only incorrect case found by command_structure_invariants selftest Philippe Waroquiers
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

The GDB data structure that records the GDB commands is made of
'struct cmd_list_element' defined in cli-decode.h.

A cmd_list_element has various pointers to other cmd_list_element structures,
All these pointers are together building a graph of commands.

However, when following the 'next' and '*prefixlist' pointers of
cmd_list_element, the structure must better be a tree.

If such pointers do not form a tree, then some other elements of
cmd_list_element cannot get a correct semantic.  In particular, the prefixname
has no correct meaning if the same prefix command can be reached via 2 different
paths.

This commit introduces a selftest that detects (at least some cases of) errors
leading to 'next' and '*prefixlist' not giving a tree structure.

The new 'command_structure_invariants' selftest detects one single case where
the command structure is not a tree:

  (gdb) maintenance selftest command_structure_invariants
  Running selftest command_structure_invariants.
  list 0x56362e204b98 duplicated, reachable via prefix 'show ' and 'info set '.  Duplicated list first command is 'ada'
  Self test failed: self-test failed at ../../classfix/gdb/unittests/command-def-selftests.c:160
  Ran 1 unit tests, 1 failed
  (gdb)

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* unittests/help-doc-selftests.c: Rename to
	unittests/command-def-selftests.c
	* unittests/command-def-selftests.c (help_doc_tests): Update some
	comments.
	(command_structure_tests, traverse_command_structure): New namespace
	and function.
	(command_structure_invariants_tests): New function.
	(_initialize_command_def_selftests) Renamed from
	_initialize_help_doc_selftests, register command_structure_invariants
	selftest.
---
 gdb/Makefile.in                               |  2 +-
 ...oc-selftests.c => command-def-selftests.c} | 81 ++++++++++++++++++-
 2 files changed, 79 insertions(+), 4 deletions(-)
 rename gdb/unittests/{help-doc-selftests.c => command-def-selftests.c} (62%)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index e3ce6a285f..32d0eee7c6 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -427,13 +427,13 @@ SELFTESTS_SRCS = \
 	unittests/array-view-selftests.c \
 	unittests/child-path-selftests.c \
 	unittests/cli-utils-selftests.c \
+	unittests/command-def-selftests.c \
 	unittests/common-utils-selftests.c \
 	unittests/copy_bitwise-selftests.c \
 	unittests/environ-selftests.c \
 	unittests/filtered_iterator-selftests.c \
 	unittests/format_pieces-selftests.c \
 	unittests/function-view-selftests.c \
-	unittests/help-doc-selftests.c \
 	unittests/lookup_name_info-selftests.c \
 	unittests/memory-map-selftests.c \
 	unittests/memrange-selftests.c \
diff --git a/gdb/unittests/help-doc-selftests.c b/gdb/unittests/command-def-selftests.c
similarity index 62%
rename from gdb/unittests/help-doc-selftests.c
rename to gdb/unittests/command-def-selftests.c
index 16ffc4f990..55e1278fc3 100644
--- a/gdb/unittests/help-doc-selftests.c
+++ b/gdb/unittests/command-def-selftests.c
@@ -1,4 +1,4 @@
-/* Self tests for help doc for GDB, the GNU debugger.
+/* Self tests for GDB command definitions for GDB, the GNU debugger.
 
    Copyright (C) 2019-2020 Free Software Foundation, Inc.
 
@@ -22,7 +22,12 @@
 #include "cli/cli-decode.h"
 #include "gdbsupport/selftest.h"
 
+#include <map>
+
 namespace selftests {
+
+/* Verify some invariants of GDB commands documentation.  */
+
 namespace help_doc_tests {
 
 static unsigned int nr_failed_invariants;
@@ -96,13 +101,83 @@ help_doc_invariants_tests ()
 }
 
 } /* namespace help_doc_tests */
+
+/* Verify some invariants of GDB command structure.  */
+
+namespace command_structure_tests {
+
+unsigned int nr_duplicates = 0;
+
+/* A map associating a list with the prefix leading to it.  */
+
+std::map<cmd_list_element **, const char *> lists;
+
+/* Store each command list in lists, associated with the prefix to reach it.  A
+   list must only be found once.  */
+
+static void
+traverse_command_structure (struct cmd_list_element **list,
+			    const char *prefix)
+{
+  struct cmd_list_element *c;
+
+  auto dupl = lists.find (list);
+  if (dupl != lists.end ())
+    {
+      fprintf_filtered (gdb_stdout,
+			"list %p duplicated,"
+			" reachable via prefix '%s' and '%s'."
+			"  Duplicated list first command is '%s'\n",
+			list,
+			prefix, dupl->second,
+			(*list)->name);
+      nr_duplicates++;
+      return;
+    }
+
+  lists.insert ({list, prefix});
+
+  /* Walk through the commands.  */
+  for (c = *list; c; c = c->next)
+    {
+      /* If this command has subcommands and is not an alias,
+	 traverse the subcommands.  */
+      if (c->prefixlist != NULL && c->cmd_pointer == nullptr)
+	{
+	  /* Recursively call ourselves on the subcommand list,
+	     passing the right prefix in.  */
+	  traverse_command_structure (c->prefixlist, c->prefixname);
+	}
+    }
+}
+
+/* Verify that a list of commands is present in the tree only once.  */
+
+static void
+command_structure_invariants_tests ()
+{
+
+  traverse_command_structure (&cmdlist, "");
+
+  /* Release memory, be ready to be re-run.  */
+  lists.clear ();
+
+  SELF_CHECK (nr_duplicates == 0);
+}
+
+}
+
 } /* namespace selftests */
 
-void _initialize_help_doc_selftests ();
+void _initialize_command_def_selftests ();
 void
-_initialize_help_doc_selftests ()
+_initialize_command_def_selftests ()
 {
   selftests::register_test
     ("help_doc_invariants",
      selftests::help_doc_tests::help_doc_invariants_tests);
+
+  selftests::register_test
+    ("command_structure_invariants",
+     selftests::command_structure_tests::command_structure_invariants_tests);
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 02/10] Fix the only incorrect case found by command_structure_invariants selftest.
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
  2020-05-10 20:55 ` [RFA 01/10] Add a selftest that detects a 'corrupted' command tree structure in GDB Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 15:52   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 03/10] Fix problem that alias can be defined or not depending on the order Philippe Waroquiers
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

The command 'info set' was defined as a specific prefix command,
but re-using the command list already used for the 'show' command.
This leads to the command tree 'next/*prefixlist' to not be a tree.

This change defines 'info set ' as an alias, thereby fixing the selftest.

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-cmds.c (_initialize_cli_cmds): Define 'info set' as
	an alias of 'show'.
---
 gdb/cli/cli-cmds.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 1b677f5d7a..d34395f0a4 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -2198,12 +2198,11 @@ Generic command for showing things about the program being debugged."),
   add_com ("complete", class_obscure, complete_command,
 	   _("List the completions for the rest of the line as a command."));
 
-  add_show_prefix_cmd ("show", class_info, _("\
+  c = add_show_prefix_cmd ("show", class_info, _("\
 Generic command for showing things about the debugger."),
-		       &showlist, "show ", 0, &cmdlist);
+			   &showlist, "show ", 0, &cmdlist);
   /* Another way to get at the same thing.  */
-  add_show_prefix_cmd ("set", class_info, _("Show all GDB settings."),
-		       &showlist, "info set ", 0, &infolist);
+  add_alias_cmd ("set", c, class_info, 0, &infolist);
 
   c = add_com ("with", class_vars, with_command, _("\
 Temporarily set SETTING to VALUE, run COMMAND, and restore SETTING.\n\
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 03/10] Fix problem that alias can be defined or not depending on the order.
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
  2020-05-10 20:55 ` [RFA 01/10] Add a selftest that detects a 'corrupted' command tree structure in GDB Philippe Waroquiers
  2020-05-10 20:55 ` [RFA 02/10] Fix the only incorrect case found by command_structure_invariants selftest Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 16:00   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 04/10] command-def-selftests.c: detect missing or wrong prefix cmd in subcommands Philippe Waroquiers
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

When an alias name starts with the name of another alias,
GDB was accepting to define the aliases in one order (short first, long after),
but refused it the other way around.

So, fix the logic to recognise an already existing alias by using
lookup_cmd_composition.

Also, this revealed a bug in lookup_cmd_composition:
when the searched command is a prefix command, lookup_cmd_composition
was not returning the fact that a command was found even if the
TEXT to parse was fully consumed.

gdb/ChangeLog
YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-cmds.c (alias_command): Check for an existing alias
	using lookup_cmd_composition, as valid_command_p is too strict
	and forbids aliases that are the prefix of an existing alias
	or command.
	* cli/cli-decode.c (lookup_cmd_composition): Ensure a prefix
	command is properly recognised as a valid command.

gdb/testsuite/ChangeLog
YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/alias.exp: Test aliases starting with a prefix of
	another alias.
---
 gdb/cli/cli-cmds.c               | 25 +++++++++++++++++++++++--
 gdb/cli/cli-decode.c             | 15 ++++++++-------
 gdb/testsuite/gdb.base/alias.exp |  6 ++++++
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index d34395f0a4..ee07c300c9 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1683,8 +1683,29 @@ alias_command (const char *args, int from_tty)
   /* ALIAS must not exist.  */
   std::string alias_string (argv_to_string (alias_argv, alias_argc));
   alias = alias_string.c_str ();
-  if (valid_command_p (alias))
-    error (_("Alias already exists: %s"), alias);
+  {
+    cmd_list_element *alias_cmd, *prefix_cmd, *cmd;
+
+    if (lookup_cmd_composition (alias, &alias_cmd, &prefix_cmd, &cmd))
+      {
+	const char *alias_name = alias_argv[alias_argc-1];
+
+	/* If we found an existing ALIAS_CMD, check that the prefix differ or
+	   the name differ.  */
+
+	if (alias_cmd != nullptr
+	    && alias_cmd->prefix == prefix_cmd
+	    && strcmp (alias_name, alias_cmd->name) == 0)
+	  error (_("Alias already exists: %s"), alias);
+
+	/* Check ALIAS differs from the found CMD.  */
+
+	if (cmd->prefix == prefix_cmd
+	    && strcmp (alias_name, cmd->name) == 0)
+	  error (_("Alias %s is the name of an existing command"), alias);
+      }
+  }
+
 
   /* If ALIAS is one word, it is an alias for the entire COMMAND.
      Example: alias spe = set print elements
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index d951ead1c9..78b8901084 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1843,6 +1843,8 @@ lookup_cmd_composition (const char *text,
 
   cur_list = cmdlist;
 
+  text = skip_spaces (text);
+
   while (1)
     {
       /* Go through as many command lists as we need to,
@@ -1850,9 +1852,6 @@ lookup_cmd_composition (const char *text,
 
       prev_cmd = *cmd;
 
-      while (*text == ' ' || *text == '\t')
-	(text)++;
-
       /* Identify the name of the command.  */
       len = find_command_name_length (text);
 
@@ -1861,7 +1860,7 @@ lookup_cmd_composition (const char *text,
 	return 0;
 
       /* TEXT is the start of the first command word to lookup (and
-	 it's length is len).  We copy this into a local temporary.  */
+	 it's length is LEN).  We copy this into a local temporary.  */
 
       command = (char *) alloca (len + 1);
       memcpy (command, text, len);
@@ -1890,12 +1889,14 @@ lookup_cmd_composition (const char *text,
 	    }
 	  *prefix_cmd = prev_cmd;
 	}
-      if ((*cmd)->prefixlist)
+
+      text += len;
+      text = skip_spaces (text);
+
+      if ((*cmd)->prefixlist && *text != '\0')
 	cur_list = *(*cmd)->prefixlist;
       else
 	return 1;
-
-      text += len;
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/alias.exp b/gdb/testsuite/gdb.base/alias.exp
index be78d9e936..9a9557668e 100644
--- a/gdb/testsuite/gdb.base/alias.exp
+++ b/gdb/testsuite/gdb.base/alias.exp
@@ -116,3 +116,9 @@ gdb_test "show print elements" "Limit .* is 56\[.\]" "verify 56"
 
 gdb_test_no_output "set print max-elements 57"
 gdb_test "show print elements" "Limit .* is 57\[.\]" "verify 57"
+
+# Test aliases having a common prefix.
+gdb_test_no_output "alias abcd  = backtrace"
+gdb_test_no_output "alias abcde = backtrace"
+gdb_test_no_output "alias fghij = backtrace"
+gdb_test_no_output "alias fghi  = backtrace"
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 04/10] command-def-selftests.c: detect missing or wrong prefix cmd in subcommands.
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
                   ` (2 preceding siblings ...)
  2020-05-10 20:55 ` [RFA 03/10] Fix problem that alias can be defined or not depending on the order Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 15:54   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 05/10] Fix the problems reported by prefix check of command-def-selftests.c Philippe Waroquiers
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

This test reveals a number of problems fixed in the next commit.

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* unittests/command-def-selftests.c (traverse_command_structure):
	Verify all commands of a list have the same prefix command and
	that only the top cmdlist commands have a null prefix.
---
 gdb/unittests/command-def-selftests.c | 35 +++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/gdb/unittests/command-def-selftests.c b/gdb/unittests/command-def-selftests.c
index 55e1278fc3..d8489d8bcd 100644
--- a/gdb/unittests/command-def-selftests.c
+++ b/gdb/unittests/command-def-selftests.c
@@ -106,20 +106,26 @@ help_doc_invariants_tests ()
 
 namespace command_structure_tests {
 
+/* Nr of commands in which a duplicated list is found.  */
 unsigned int nr_duplicates = 0;
+/* Nr of commands in a list having no valid prefix cmd.  */
+unsigned int nr_invalid_prefixcmd = 0;
 
 /* A map associating a list with the prefix leading to it.  */
 
 std::map<cmd_list_element **, const char *> lists;
 
 /* Store each command list in lists, associated with the prefix to reach it.  A
-   list must only be found once.  */
+   list must only be found once.
+
+   Verifies that all elements of the list have the same non-full prefix
+   command.  */
 
 static void
 traverse_command_structure (struct cmd_list_element **list,
 			    const char *prefix)
 {
-  struct cmd_list_element *c;
+  struct cmd_list_element *c, *prefixcmd;
 
   auto dupl = lists.find (list);
   if (dupl != lists.end ())
@@ -137,6 +143,13 @@ traverse_command_structure (struct cmd_list_element **list,
 
   lists.insert ({list, prefix});
 
+  /* All commands of *list must have a prefix command equal to PREFIXCMD,
+     the prefix command of the first command.  */
+  if (*list == nullptr)
+    prefixcmd = nullptr; /* A prefix command with an empty subcommand list.  */
+  else
+    prefixcmd = (*list)->prefix;
+
   /* Walk through the commands.  */
   for (c = *list; c; c = c->next)
     {
@@ -148,6 +161,23 @@ traverse_command_structure (struct cmd_list_element **list,
 	     passing the right prefix in.  */
 	  traverse_command_structure (c->prefixlist, c->prefixname);
 	}
+      if (prefixcmd != c->prefix
+	  || (prefixcmd == nullptr && *list != cmdlist))
+	{
+	  if (c->prefix == nullptr)
+	    fprintf_filtered (gdb_stdout,
+			      "list %p reachable via prefix '%s'."
+			      "  command '%s' has null prefixcmd\n",
+			      list,
+			      prefix, c->name);
+	  else
+	    fprintf_filtered (gdb_stdout,
+			      "list %p reachable via prefix '%s'."
+			      "  command '%s' has a different prefixcmd\n",
+			      list,
+			      prefix, c->name);
+	  nr_invalid_prefixcmd++;
+	}
     }
 }
 
@@ -163,6 +193,7 @@ command_structure_invariants_tests ()
   lists.clear ();
 
   SELF_CHECK (nr_duplicates == 0);
+  SELF_CHECK (nr_invalid_prefixcmd == 0);
 }
 
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 05/10] Fix the problems reported by prefix check of command-def-selftests.c
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
                   ` (3 preceding siblings ...)
  2020-05-10 20:55 ` [RFA 04/10] command-def-selftests.c: detect missing or wrong prefix cmd in subcommands Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 16:07   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 06/10] Fix inconsistent output of prefix and bugs in 'show' command Philippe Waroquiers
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

The command structure selftest detects a series of problems
that are fixed by this commit.

Many commands have a null prefix command, e.g.
    (gdb) maintenance selftest command_str
    Running selftest command_structure_invariants.
    list 0x560417949cb8 reachable via prefix 'append binary '.  command 'memory' has null prefixcmd
    list 0x560417949cb8 reachable via prefix 'append binary '.  command 'value' has null prefixcmd
    ...

Most of these are fixed by the following changes:
  * do_add_cmd searches the prefix command having the list
    in which the command is added.
    This ensures that a command defined after its prefix command
    gets the correct prefix command.
  * Due to the GDB initialization order, a GDB file can define
    a subcommand before the prefix command is defined.
    So, have add_prefix_cmd calling a new recursive function
    'update_prefix_field_of_prefix_commands' to set the prefix
    command of all sub-commands that are now reachable from
    this newly defined prefix command.  Note that this recursive
    call replaces the function 'set_prefix_cmd' that was providing
    a partial solution to this problem.

Following that, 2 python commands (defined after all the other GDB
commands) got a wrong prefix command, e.g. "info frame-filter" has
as prefix command the "i" alias of "info".  This is fixed by having
lookup_cmd_for_prefixlist returning the aliased command rather than
the alias.

After that, one remaining problem:
    (gdb) maintenance selftest command_str
    Running selftest command_structure_invariants.
    list 0x55f320272298 reachable via prefix 'set remote '.  command 'system-call-allowed' has null prefixcmd
    Self test failed: self-test failed at ../../classfix/gdb/unittests/command-def-selftests.c:196
    Ran 1 unit tests, 1 failed
    (gdb)

Caused by initialize_remote_fileio that was taking the address of
its arguments remote_set_cmdlist and remote_show_cmdlist instead
of receiving the correct values to use as list.

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-decode.c (lookup_cmd_for_prefix): Return the aliased command
	as prefix, not one of its aliases.
	(set_cmd_prefix): Remove.
	(do_add_cmd): Centralize the setting of the prefix of a command, when
	command is defined after its full chain of prefix commands.
	(add_alias_cmd): Remove call to set_cmd_prefix, as do_add_cmd does it.
	(add_setshow_cmd_full): Likewise.
	(update_prefix_field_of_prefixed_commands): New function.
	(add_prefix_cmd): Replace non working call to set_cmd_prefix by
	update_prefix_field_of_prefixed_commands.
	* gdb/remote-fileio.c (initialize_remote_fileio): Use the real
	addresses of remote_set_cmdlist and remote_show_cmdlist given
	as argument, not the address of an argument.
	* gdb/remote-fileio.h (initialize_remote_fileio): Likewise.
	* gdb/remote.c (_initialize_remote): Likewise.
---
 gdb/cli/cli-decode.c | 80 +++++++++++++++++++++++++-------------------
 gdb/remote-fileio.c  |  8 ++---
 gdb/remote-fileio.h  |  4 +--
 gdb/remote.c         |  2 +-
 4 files changed, 53 insertions(+), 41 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 78b8901084..be36eb6732 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -61,7 +61,11 @@ lookup_cmd_for_prefixlist (struct cmd_list_element **key,
       if (p->prefixlist == NULL)
 	continue;
       else if (p->prefixlist == key)
-	return p;
+	{
+	  /* If we found an alias, we must return the aliased
+	     command.  */
+	  return p->cmd_pointer ? p->cmd_pointer : p;
+	}
 
       q = lookup_cmd_for_prefixlist (key, *(p->prefixlist));
       if (q != NULL)
@@ -71,27 +75,6 @@ lookup_cmd_for_prefixlist (struct cmd_list_element **key,
   return NULL;
 }
 
-static void
-set_cmd_prefix (struct cmd_list_element *c, struct cmd_list_element **list)
-{
-  struct cmd_list_element *p;
-
-  /* Check to see if *LIST contains any element other than C.  */
-  for (p = *list; p != NULL; p = p->next)
-    if (p != c)
-      break;
-
-  if (p == NULL)
-    {
-      /* *SET_LIST only contains SET.  */
-      p = lookup_cmd_for_prefixlist (list, setlist);
-
-      c->prefix = p ? (p->cmd_pointer ? p->cmd_pointer : p) : p;
-    }
-  else
-    c->prefix = p->prefix;
-}
-
 static void
 print_help_for_command (struct cmd_list_element *c, const char *prefix,
 			int recurse, struct ui_file *stream);
@@ -229,6 +212,13 @@ do_add_cmd (const char *name, enum command_class theclass,
       p->next = c;
     }
 
+  /* Search the prefix cmd of C, and assigns it to C->prefix.
+     See also add_prefix_cmd and update_prefix_field_of_prefixed_commands.  */
+  struct cmd_list_element *prefixcmd = lookup_cmd_for_prefixlist (list,
+								  cmdlist);
+  c->prefix = prefixcmd;
+
+
   return c;
 }
 
@@ -330,7 +320,6 @@ add_alias_cmd (const char *name, cmd_list_element *old,
   c->alias_chain = old->aliases;
   old->aliases = c;
 
-  set_cmd_prefix (c, list);
   return c;
 }
 
@@ -349,6 +338,37 @@ add_alias_cmd (const char *name, const char *oldname,
 }
 
 
+/* Update the prefix field of all sub-commands of the prefix command C.
+   We must do this when a prefix command is defined as the GDB init sequence
+   does not guarantee that a prefix command is created before its sub-commands.
+   For example, break-catch-sig.c initialization runs before breakpoint.c
+   initialization, but it is breakpoint.c that creates the "catch" command used
+   by the "catch signal" command created by break-catch-sig.c.  */
+
+static void
+update_prefix_field_of_prefixed_commands (struct cmd_list_element *c)
+{
+  for (cmd_list_element *p = *c->prefixlist; p != NULL; p = p->next)
+    {
+      p->prefix = c;
+
+      /* We must recursively update the prefix field to cover
+	 e.g.  'info auto-load libthread-db' where the creation
+	 order was:
+           libthread-db
+           auto-load
+           info
+	 In such a case, when 'auto-load' was created by do_add_cmd,
+         the 'libthread-db' prefix field could not be updated, as the
+	 'auto-load' command was not yet reachable by
+	    lookup_cmd_for_prefixlist (list, cmdlist)
+	    that searches from the top level 'cmdlist'.  */
+      if (p->prefixlist != nullptr)
+	update_prefix_field_of_prefixed_commands (p);
+    }
+}
+
+
 /* Like add_cmd but adds an element for a command prefix: a name that
    should be followed by a subcommand to be looked up in another
    command list.  PREFIXLIST should be the address of the variable
@@ -362,20 +382,14 @@ add_prefix_cmd (const char *name, enum command_class theclass,
 		struct cmd_list_element **list)
 {
   struct cmd_list_element *c = add_cmd (name, theclass, fun, doc, list);
-  struct cmd_list_element *p;
 
   c->prefixlist = prefixlist;
   c->prefixname = prefixname;
   c->allow_unknown = allow_unknown;
 
-  if (list == &cmdlist)
-    c->prefix = NULL;
-  else
-    set_cmd_prefix (c, list);
-
-  /* Update the field 'prefix' of each cmd_list_element in *PREFIXLIST.  */
-  for (p = *prefixlist; p != NULL; p = p->next)
-    p->prefix = c;
+  /* Now that prefix command C is defined, we need to set the prefix field
+     of all prefixed commands that were defined before C itself was defined.  */
+  update_prefix_field_of_prefixed_commands (c);
 
   return c;
 }
@@ -555,8 +569,6 @@ add_setshow_cmd_full (const char *name,
   if (set_func != NULL)
     set_cmd_sfunc (set, set_func);
 
-  set_cmd_prefix (set, set_list);
-
   show = add_set_or_show_cmd (name, show_cmd, theclass, var_type, var,
 			      full_show_doc, show_list);
   show->doc_allocated = 1;
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index df470fd86d..7450e84860 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -1295,15 +1295,15 @@ show_system_call_allowed (const char *args, int from_tty)
 }
 
 void
-initialize_remote_fileio (struct cmd_list_element *remote_set_cmdlist,
-			  struct cmd_list_element *remote_show_cmdlist)
+initialize_remote_fileio (struct cmd_list_element **remote_set_cmdlist,
+			  struct cmd_list_element **remote_show_cmdlist)
 {
   add_cmd ("system-call-allowed", no_class,
 	   set_system_call_allowed,
 	   _("Set if the host system(3) call is allowed for the target."),
-	   &remote_set_cmdlist);
+	   remote_set_cmdlist);
   add_cmd ("system-call-allowed", no_class,
 	   show_system_call_allowed,
 	   _("Show if the host system(3) call is allowed for the target."),
-	   &remote_show_cmdlist);
+	   remote_show_cmdlist);
 }
diff --git a/gdb/remote-fileio.h b/gdb/remote-fileio.h
index ab73f9ba7d..f206b04091 100644
--- a/gdb/remote-fileio.h
+++ b/gdb/remote-fileio.h
@@ -37,8 +37,8 @@ extern void remote_fileio_reset (void);
 
 /* Called from _initialize_remote ().  */
 extern void initialize_remote_fileio (
-  struct cmd_list_element *remote_set_cmdlist,
-  struct cmd_list_element *remote_show_cmdlist);
+  struct cmd_list_element **remote_set_cmdlist,
+  struct cmd_list_element **remote_show_cmdlist);
 
 /* Unpack a struct fio_stat.  */
 extern void remote_fileio_to_host_stat (struct fio_stat *fst,
diff --git a/gdb/remote.c b/gdb/remote.c
index 5db406e045..e51b79ec09 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -14856,5 +14856,5 @@ Specify \"unlimited\" to display all the characters."),
 				       &setdebuglist, &showdebuglist);
 
   /* Eventually initialize fileio.  See fileio.c */
-  initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);
+  initialize_remote_fileio (&remote_set_cmdlist, &remote_show_cmdlist);
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 06/10] Fix inconsistent output of prefix and bugs in 'show' command
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
                   ` (4 preceding siblings ...)
  2020-05-10 20:55 ` [RFA 05/10] Fix the problems reported by prefix check of command-def-selftests.c Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 16:16   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 07/10] Fix/improve 'help CLASS' output Philippe Waroquiers
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

cmd_show_list function implements the 'show' command.

cmd_show_list output is inconsistent: it sometimes shows a prefix
and sometimes does not.
For example, in the below, you see that there is a prefix before
each value, except for 'enabled'.

    (gdb) show style
    style address background:  The "address" style background color is: none
    style address foreground:  The "address" style foreground color is: blue
    style address intensity:  The "address" style display intensity is: normal
    enabled:  CLI output styling is enabled.
    style filename background:  The "filename" style background color is: none
    ...

There are other inconsistencies or bugs e.g. in
the below we see twice insn-number-max, once with a prefix
and once without prefix : last line, just before the value of
instruction-history-size which is itself without prefix.

    (gdb) show record
    record btrace bts buffer-size:  The record/replay bts buffer size is 65536.
    record btrace cpu:  btrace cpu is 'auto'.
    record btrace pt buffer-size:  The record/replay pt buffer size is 16384.
    record btrace replay-memory-access:  Replay memory access is read-only.
    record full insn-number-max:  Record/replay buffer limit is 200000.
    record full memory-query:  Whether query if PREC cannot record memory change of next instruction is off.
    record full stop-at-limit:  Whether record/replay stops when record/replay buffer becomes full is on.
    function-call-history-size:  Number of functions to print in "record function-call-history" is 10.
    insn-number-max:  instruction-history-size:  Number of instructions to print in "record instruction-history" is 10.
    (gdb)

Also, some values are output several times due to some aliases, so avoid outputting duplicated
values by skipping all aliases.

Now that the command structure has a correct 'back-pointer' from a command
to its prefix command, we can simplify cmd_show_list by removing its prefix argument
and at the same time fix the output inconsistencies and bugs.

gdb/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-setshow.h (cmd_show_list): Remove prefix argument.
	* cli/cli-decode.c (do_show_prefix_cmd): Likewise.
	* command.h (cmd_show_list): Likewise.
	* dwarf2/index-cache.c (show_index_cache_command): Likewise.
	* cli/cli-setshow.c (cmd_show_list): Use the prefix to produce the output.  Skip aliases.

gdb/testsuite/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/default.exp: Update output following fixes.
---
 gdb/cli/cli-decode.c               |  2 +-
 gdb/cli/cli-setshow.c              | 38 +++++++++++++++++-------------
 gdb/cli/cli-setshow.h              |  3 +--
 gdb/command.h                      |  2 +-
 gdb/dwarf2/index-cache.c           |  2 +-
 gdb/testsuite/gdb.base/default.exp |  4 ++--
 6 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index be36eb6732..c37dfaffb5 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -428,7 +428,7 @@ add_basic_prefix_cmd (const char *name, enum command_class theclass,
 static void
 do_show_prefix_cmd (const char *args, int from_tty, struct cmd_list_element *c)
 {
-  cmd_show_list (*c->prefixlist, from_tty, "");
+  cmd_show_list (*c->prefixlist, from_tty);
 }
 
 /* See command.h.  */
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 2a2f905bdf..19a5565bfe 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -733,39 +733,45 @@ do_show_command (const char *arg, int from_tty, struct cmd_list_element *c)
 /* Show all the settings in a list of show commands.  */
 
 void
-cmd_show_list (struct cmd_list_element *list, int from_tty, const char *prefix)
+cmd_show_list (struct cmd_list_element *list, int from_tty)
 {
   struct ui_out *uiout = current_uiout;
 
   ui_out_emit_tuple tuple_emitter (uiout, "showlist");
   for (; list != NULL; list = list->next)
     {
+      /* We skip show command aliases to avoid showing duplicated values.  */
+
       /* If we find a prefix, run its list, prefixing our output by its
          prefix (with "show " skipped).  */
-      if (list->prefixlist && !list->abbrev_flag)
+      if (list->prefixlist && list->cmd_pointer == nullptr)
 	{
 	  ui_out_emit_tuple optionlist_emitter (uiout, "optionlist");
 	  const char *new_prefix = strstr (list->prefixname, "show ") + 5;
 
 	  if (uiout->is_mi_like_p ())
 	    uiout->field_string ("prefix", new_prefix);
-	  cmd_show_list (*list->prefixlist, from_tty, new_prefix);
+	  cmd_show_list (*list->prefixlist, from_tty);
 	}
-      else
+      else if (list->theclass != no_set_class && list->cmd_pointer == nullptr)
 	{
-	  if (list->theclass != no_set_class)
-	    {
-	      ui_out_emit_tuple option_emitter (uiout, "option");
-
-	      uiout->text (prefix);
-	      uiout->field_string ("name", list->name);
-	      uiout->text (":  ");
-	      if (list->type == show_cmd)
-		do_show_command (NULL, from_tty, list);
-	      else
-		cmd_func (list, NULL, from_tty);
-	    }
+	  ui_out_emit_tuple option_emitter (uiout, "option");
+
+	  {
+	    /* If we find a prefix, output it (with "show " skipped).  */
+	    const char *prefixname
+	      = (list->prefix == nullptr ? ""
+		 : strstr (list->prefix->prefixname, "show ") + 5);
+	    uiout->text (prefixname);
+	  }
+	  uiout->field_string ("name", list->name);
+	  uiout->text (":  ");
+	  if (list->type == show_cmd)
+	    do_show_command (NULL, from_tty, list);
+	  else
+	    cmd_func (list, NULL, from_tty);
 	}
     }
 }
 
+
diff --git a/gdb/cli/cli-setshow.h b/gdb/cli/cli-setshow.h
index 9f0492bd32..83e4984ed6 100644
--- a/gdb/cli/cli-setshow.h
+++ b/gdb/cli/cli-setshow.h
@@ -60,7 +60,6 @@ extern void do_show_command (const char *arg, int from_tty,
 /* Get a string version of C's current value.  */
 extern std::string get_setshow_command_value_string (const cmd_list_element *c);
 
-extern void cmd_show_list (struct cmd_list_element *list, int from_tty,
-			   const char *prefix);
+extern void cmd_show_list (struct cmd_list_element *list, int from_tty);
 
 #endif /* CLI_CLI_SETSHOW_H */
diff --git a/gdb/command.h b/gdb/command.h
index 81c35c98d2..0a1706c545 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -464,7 +464,7 @@ extern void
 
 /* Do a "show" command for each thing on a command list.  */
 
-extern void cmd_show_list (struct cmd_list_element *, int, const char *);
+extern void cmd_show_list (struct cmd_list_element *, int);
 
 /* Used everywhere whenever at least one parameter is required and
    none is specified.  */
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index 23e938b010..76e7ce6ab7 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -259,7 +259,7 @@ show_index_cache_command (const char *arg, int from_tty)
   auto restore_flag = make_scoped_restore (&in_show_index_cache_command, true);
 
   /* Call all "show index-cache" subcommands.  */
-  cmd_show_list (show_index_cache_prefix_list, from_tty, "");
+  cmd_show_list (show_index_cache_prefix_list, from_tty);
 
   printf_unfiltered ("\n");
   printf_unfiltered
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 95b92c4871..c34bb9a92a 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -559,7 +559,7 @@ gdb_test "show args" "Argument list to give program being debugged when it is st
 
 # test show check abbreviations
 foreach x {"c" "ch" "check"} {
-    gdb_test "show $x" "range:  *Range checking is \"auto; currently off\".(\[^\r\n\]*\[\r\n\])+type:  *Strict type checking is on\..*" \
+    gdb_test "show $x" "check range:  *Range checking is \"auto; currently off\".(\[^\r\n\]*\[\r\n\])+check type:  *Strict type checking is on\..*" \
 	"show check \"$x\" abbreviation"
 }
 
@@ -645,7 +645,7 @@ gdb_test "show history save" "Saving of the history record on exit is on."
 #test show history size
 gdb_test "show history size" "The size of the command history is.*"
 #test show history
-gdb_test "show history" "expansion:  *History expansion on command input is o(\[^\r\n\]*\[\r\n\])+filename:  *The filename in which to record the command history is.*.gdb_history(\[^\r\n\]*\[\r\n\])+save: *Saving of the history record on exit is o(\[^\r\n\]*\[\r\n\])+size: * The size of the command history is.*"
+gdb_test "show history" "history expansion:  *History expansion on command input is o(\[^\r\n\]*\[\r\n\])+history filename:  *The filename in which to record the command history is.*.gdb_history(\[^\r\n\]*\[\r\n\])+history save: *Saving of the history record on exit is o(\[^\r\n\]*\[\r\n\])+history size: * The size of the command history is.*"
 #test show language
 gdb_test "show language" "The current source language is \"auto; currently c\"."
 #test show listsize
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 07/10] Fix/improve 'help CLASS' output
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
                   ` (5 preceding siblings ...)
  2020-05-10 20:55 ` [RFA 06/10] Fix inconsistent output of prefix and bugs in 'show' command Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 16:26   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 08/10] Fix/improve 'apropos' output Philippe Waroquiers
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

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

YYYY-MM-DD  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, 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
	fput_command_name_styled.
	(help_list, help_all): Update callers to remove prefix arg.
	* cli/cli-cmds.c (_initialize_cli_cmds): Update alias_command doc.

gdb/testsuite/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/alias.exp: Update help output check.
---
 gdb/cli/cli-cmds.c               |   2 +-
 gdb/cli/cli-decode.c             | 122 ++++++++++++++++++++++++-------
 gdb/cli/cli-decode.h             |   3 -
 gdb/testsuite/gdb.base/alias.exp |   2 +-
 4 files changed, 98 insertions(+), 31 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index ee07c300c9..e112b18e62 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -2443,7 +2443,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 c37dfaffb5..aeaee3db30 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -43,6 +43,9 @@ 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,
+			   int 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,7 +79,7 @@ lookup_cmd_for_prefixlist (struct cmd_list_element **key,
 }
 
 static void
-print_help_for_command (struct cmd_list_element *c, const char *prefix,
+print_help_for_command (struct cmd_list_element *c,
 			int recurse, struct ui_file *stream);
 
 \f
@@ -1021,6 +1024,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 +1248,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, (int) theclass >= 0, stream);
 
   if (theclass == all_classes)
     {
@@ -1255,7 +1295,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, 1, stream);
 	}
     }
 
@@ -1275,7 +1315,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 +1367,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,
+print_help_for_command (struct cmd_list_element *c,
 			int 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 +1380,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, 1, 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)
+	       int 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 4f7c7701e4..f4719bfac4 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/gdb.base/alias.exp b/gdb/testsuite/gdb.base/alias.exp
index 9a9557668e..69cfde67c1 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" \
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 08/10] Fix/improve 'apropos' output
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
                   ` (6 preceding siblings ...)
  2020-05-10 20:55 ` [RFA 07/10] Fix/improve 'help CLASS' output Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 16:30   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 09/10] Ensure class_alias is only used for user-defined aliases Philippe Waroquiers
  2020-05-10 20:55 ` [RFA 10/10] Update NEWS and documentation for help and apropos changes Philippe Waroquiers
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

Similarly to 'help CLASS', apropos possibly shows several
times the same help (for the command and for each of its aliases).

This patch changes 'apropos' so that the help for a command and
all its aliases is shown once.

So, apropos_cmd now skips all aliases/abbreviations, as these are printed
as part of the help of the aliased command.

When 'apropos' prints the help of a command, function 'help_cmd' now
unconditionally print the command name and its possible aliases (as we must
indicate to the user the command/aliases for which the help is printed).

When 'help somecommand' prints the help of a command, if the command is not
aliased, the command name is not printed (to avoid a useless first line), but if
it has aliases, then the command name and all its aliases are now printed.
In addition to provide to the user the choice of the best way to
type a command, it also avoids the strange behaviour that the output
of 'help somealias' does not mention somealias.

gdb/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-decode.c (apropos_cmd): Produce output for aliases
	when their aliased command is traversed.
	(help_cmd): Add fput_command_names_styled call to
	output command name and aliases when command has an alias.

gdb/testsuite/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/help.exp: Test apropos and help for commands
	having aliases.
---
 gdb/cli/cli-decode.c            | 37 ++++++++++++++++++++++++++-------
 gdb/testsuite/gdb.base/help.exp |  7 +++++++
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index aeaee3db30..48187e8583 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1076,9 +1076,7 @@ print_doc_of_command (struct cmd_list_element *c, const char *prefix,
   if (verbose)
     fputs_filtered ("\n", stream);
 
-  fprintf_styled (stream, title_style.style (),
-		  "%s%s", prefix, c->name);
-  fputs_filtered (" -- ", stream);
+  fput_command_names_styled (c, true, " -- ", stream);
   if (verbose)
     fputs_highlighted (c->doc, highlight, stream);
   else
@@ -1104,6 +1102,14 @@ apropos_cmd (struct ui_file *stream,
   /* Walk through the commands.  */
   for (c=commandlist;c;c=c->next)
     {
+      if (c->cmd_pointer != nullptr)
+	{
+	  /*  Command aliases/abbreviations are skipped to ensure we print the
+	      doc of a command only once, when encountering the aliased
+	      command.  */
+	  continue;
+	}
+
       returnvalue = -1; /* Needed to avoid double printing.  */
       if (c->name != NULL)
 	{
@@ -1113,6 +1119,17 @@ apropos_cmd (struct ui_file *stream,
 	  returnvalue = regex.search (c->name, name_len, 0, name_len, NULL);
 	  if (returnvalue >= 0)
 	    print_doc_of_command (c, prefix, verbose, regex, stream);
+
+	  /* Try to match against the name of the aliases.  */
+	  for (cmd_list_element *iter = c->aliases;
+	       returnvalue < 0 && iter;
+	       iter = iter->alias_chain)
+	    {
+	      name_len = strlen (iter->name);
+	      returnvalue = regex.search (iter->name, name_len, 0, name_len, NULL);
+	      if (returnvalue >= 0)
+		print_doc_of_command (c, prefix, verbose, regex, stream);
+	    }
 	}
       if (c->doc != NULL && returnvalue < 0)
 	{
@@ -1122,10 +1139,8 @@ apropos_cmd (struct ui_file *stream,
 	  if (regex.search (c->doc, doc_len, 0, doc_len, NULL) >= 0)
 	    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
-	 in order to avoid duplicates in the output.  */
-      if (c->prefixlist != NULL && !c->abbrev_flag)
+      /* Check if this command has subcommands.  */
+      if (c->prefixlist != NULL)
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
@@ -1148,7 +1163,7 @@ apropos_cmd (struct ui_file *stream,
 void
 help_cmd (const char *command, struct ui_file *stream)
 {
-  struct cmd_list_element *c;
+  struct cmd_list_element *c, *alias, *prefix_cmd, *c_cmd;
 
   if (!command)
     {
@@ -1162,11 +1177,14 @@ help_cmd (const char *command, struct ui_file *stream)
       return;
     }
 
+  const char *orig_command = command;
   c = lookup_cmd (&command, cmdlist, "", 0, 0);
 
   if (c == 0)
     return;
 
+  lookup_cmd_composition (orig_command, &alias, &prefix_cmd, &c_cmd);
+
   /* There are three cases here.
      If c->prefixlist is nonzero, we have a prefix command.
      Print its documentation, then list its subcommands.
@@ -1179,6 +1197,9 @@ help_cmd (const char *command, struct ui_file *stream)
      number of this class so that the commands in the class will be
      listed.  */
 
+  /* If the user asked 'help somecommand' and there is no alias,
+     the false indicates to not output the (single) command name.  */
+  fput_command_names_styled (c, false, "\n", stream);
   fputs_filtered (c->doc, stream);
   fputs_filtered ("\n", stream);
 
diff --git a/gdb/testsuite/gdb.base/help.exp b/gdb/testsuite/gdb.base/help.exp
index 9316a705b1..a2d33ba95c 100644
--- a/gdb/testsuite/gdb.base/help.exp
+++ b/gdb/testsuite/gdb.base/help.exp
@@ -126,3 +126,10 @@ gdb_test "apropos \\\(print\[\^\[ bsiedf\\\".-\]\\\)" "handle -- Specify how to
 gdb_test "apropos handle signal" "handle -- Specify how to handle signals\."
 # test apropos apropos
 gdb_test "apropos apropos" "apropos -- Search for commands matching a REGEXP.*"
+
+# test apropos for commands having aliases.
+gdb_test "apropos Print backtrace of all stack frames, or innermost COUNT frames\." \
+    "backtrace, where, bt -- Print backtrace of all stack frames, or innermost COUNT frames\."
+
+# test help for commands having aliases.
+gdb_test "help bt" "backtrace, where, bt\[\r\n\]+Print backtrace of all stack frames, or innermost COUNT frames\..*"
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 09/10] Ensure class_alias is only used for user-defined aliases.
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
                   ` (7 preceding siblings ...)
  2020-05-10 20:55 ` [RFA 08/10] Fix/improve 'apropos' output Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-14 17:33   ` Tom Tromey
  2020-05-10 20:55 ` [RFA 10/10] Update NEWS and documentation for help and apropos changes Philippe Waroquiers
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

This commit finally does the (small) change that started this patch
series.

It ensures that the class_alias is only used for user-defined aliases.
So, the few GDB pre-defined aliases that were using the 'class_alias'
class are now using a real help class, typically the class of
the aliased command.

gdb/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* command.h (enum command_class): Improve comments, document
	that class_alias is for user-defined aliases, give the class
	name for each class, remove unused class_xdb.
	* cli/cli-decode.c (add_com_alias): Document THECLASS intended usage.
	* breakpoint.c (_initialize_breakpoint): Replace class_alias
	by a precise class.
	* infcmd.c (_initialize_infcmd): Likewise.
	* reverse.c (_initialize_reverse): Likewise.
	* stack.c (_initialize_stack): Likewise.
	* symfile.c (_initialize_symfile): Likewise.
	* tracepoint.c (_initialize_tracepoint): Likewise.

gdb/testsuite/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/alias.exp: Verify 'help aliases' shows user defined aliases.
---
 gdb/breakpoint.c                 | 12 ++++----
 gdb/cli/cli-decode.c             |  5 +++-
 gdb/command.h                    | 47 ++++++++++++++++++++++++--------
 gdb/infcmd.c                     |  6 ++--
 gdb/reverse.c                    | 10 +++----
 gdb/stack.c                      |  2 +-
 gdb/symfile.c                    |  4 +--
 gdb/testsuite/gdb.base/alias.exp |  3 ++
 gdb/tracepoint.c                 |  4 +--
 9 files changed, 62 insertions(+), 31 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 22ddb3d5e8..9f237cee1f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -15478,7 +15478,7 @@ A disabled breakpoint is not forgotten, but has no effect until re-enabled."),
   add_com_alias ("dis", "disable", class_breakpoint, 1);
   add_com_alias ("disa", "disable", class_breakpoint, 1);
 
-  add_cmd ("breakpoints", class_alias, disable_command, _("\
+  add_cmd ("breakpoints", class_breakpoint, disable_command, _("\
 Disable all or some breakpoints.\n\
 Usage: disable breakpoints [BREAKPOINTNUM]...\n\
 Arguments are breakpoint numbers with spaces in between.\n\
@@ -15498,7 +15498,7 @@ Also a prefix command for deletion of other GDB objects."),
   add_com_alias ("d", "delete", class_breakpoint, 1);
   add_com_alias ("del", "delete", class_breakpoint, 1);
 
-  add_cmd ("breakpoints", class_alias, delete_command, _("\
+  add_cmd ("breakpoints", class_breakpoint, delete_command, _("\
 Delete all or some breakpoints or auto-display expressions.\n\
 Usage: delete breakpoints [BREAKPOINTNUM]...\n\
 Arguments are breakpoint numbers with spaces in between.\n\
@@ -15686,10 +15686,10 @@ BREAK_ARGS_HELP ("trace") "\n\
 Do \"help tracepoints\" for info on other tracepoint commands."));
   set_cmd_completer (c, location_completer);
 
-  add_com_alias ("tp", "trace", class_alias, 0);
-  add_com_alias ("tr", "trace", class_alias, 1);
-  add_com_alias ("tra", "trace", class_alias, 1);
-  add_com_alias ("trac", "trace", class_alias, 1);
+  add_com_alias ("tp", "trace", class_breakpoint, 0);
+  add_com_alias ("tr", "trace", class_breakpoint, 1);
+  add_com_alias ("tra", "trace", class_breakpoint, 1);
+  add_com_alias ("trac", "trace", class_breakpoint, 1);
 
   c = add_com ("ftrace", class_breakpoint, ftrace_command, _("\
 Set a fast tracepoint at specified location.\n\
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 48187e8583..67758356cb 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1004,7 +1004,10 @@ add_com (const char *name, enum command_class theclass,
   return add_cmd (name, theclass, fun, doc, &cmdlist);
 }
 
-/* Add an alias or abbreviation command to the list of commands.  */
+/* Add an alias or abbreviation command to the list of commands.
+   For aliases predefined by GDB (such as bt), THECLASS must be
+   different of class_alias, as class_alias is used to identify
+   user defined aliases.  */
 
 struct cmd_list_element *
 add_com_alias (const char *name, const char *oldname, enum command_class theclass,
diff --git a/gdb/command.h b/gdb/command.h
index 0a1706c545..04a380cba4 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -29,21 +29,46 @@ struct completion_tracker;
 /* Command classes are top-level categories into which commands are
    broken down for "help" purposes.
 
-   Notes on classes: class_alias is for alias commands which are not
-   abbreviations of the original command.  class-pseudo is for
-   commands which are not really commands nor help topics ("stop").  */
+   The class_alias is used for the user-defined aliases, defined
+   using the "alias" command.
+
+   Aliases pre-defined by GDB (e.g. the alias "bt" of the "backtrace" command)
+   are not using the class_alias.
+   Different pre-defined aliases of the same command do not necessarily
+   have the same classes.  For example, class_stack is used for the
+   "backtrace" and its "bt" alias", while "info stack" (also an alias
+   of "backtrace" uses class_info.  */
 
 enum command_class
 {
-  /* Special args to help_list */
-  class_deprecated = -3, all_classes = -2, all_commands = -1,
+  /* Classes of commands followed by a comment giving the name
+     to use in "help <classname>".
+     Note that help accepts unambiguous abbreviated class names.  */
+
+  /* Special classes to help_list */
+  class_deprecated = -3,
+  all_classes = -2,  /* help without <classname> */
+  all_commands = -1, /* all */
+
   /* Classes of commands */
-  no_class = -1, class_run = 0, class_vars, class_stack, class_files,
-  class_support, class_info, class_breakpoint, class_trace,
-  class_alias, class_bookmark, class_obscure, class_maintenance,
-  class_tui, class_user, class_xdb,
-  no_set_class	/* Used for "show" commands that have no corresponding
-		   "set" command.  */
+  no_class = -1,
+  class_run = 0,     /* running */
+  class_vars,        /* data */
+  class_stack,       /* stack */
+  class_files,       /* files */
+  class_support,     /* support */
+  class_info,        /* status */
+  class_breakpoint,  /* breakpoints */
+  class_trace,       /* tracepoints */
+  class_alias,       /* aliases */
+  class_bookmark,
+  class_obscure,     /* obscure */
+  class_maintenance, /* internals */
+  class_tui,
+  class_user,        /* user-defined */
+
+  /* Used for "show" commands that have no corresponding "set" command.  */
+  no_set_class
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 9bbb413d4e..3e32e89d99 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3180,7 +3180,7 @@ is restored."),
   cmd_name = "inferior-tty";
   c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
   gdb_assert (c != NULL);
-  add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
+  add_alias_cmd ("tty", c, class_run, 0, &cmdlist);
 
   cmd_name = "args";
   add_setshow_string_noescape_cmd (cmd_name, class_run,
@@ -3318,14 +3318,14 @@ Step one instruction exactly.\n\
 Usage: stepi [N]\n\
 Argument N means step N times (or till program stops for another \
 reason)."));
-  add_com_alias ("si", "stepi", class_alias, 0);
+  add_com_alias ("si", "stepi", class_run, 0);
 
   add_com ("nexti", class_run, nexti_command, _("\
 Step one instruction, but proceed through subroutine calls.\n\
 Usage: nexti [N]\n\
 Argument N means step N times (or till program stops for another \
 reason)."));
-  add_com_alias ("ni", "nexti", class_alias, 0);
+  add_com_alias ("ni", "nexti", class_run, 0);
 
   add_com ("finish", class_run, finish_command, _("\
 Execute until selected stack frame returns.\n\
diff --git a/gdb/reverse.c b/gdb/reverse.c
index 1ccb9d2797..583e0d02da 100644
--- a/gdb/reverse.c
+++ b/gdb/reverse.c
@@ -330,7 +330,7 @@ _initialize_reverse ()
 Step program backward until it reaches the beginning of another source line.\n\
 Argument N means do this N times (or till program stops for another reason).")
 	   );
-  add_com_alias ("rs", "reverse-step", class_alias, 1);
+  add_com_alias ("rs", "reverse-step", class_run, 1);
 
   add_com ("reverse-next", class_run, reverse_next, _("\
 Step program backward, proceeding through subroutine calls.\n\
@@ -338,26 +338,26 @@ Like the \"reverse-step\" command as long as subroutine calls do not happen;\n\
 when they do, the call is treated as one instruction.\n\
 Argument N means do this N times (or till program stops for another reason).")
 	   );
-  add_com_alias ("rn", "reverse-next", class_alias, 1);
+  add_com_alias ("rn", "reverse-next", class_run, 1);
 
   add_com ("reverse-stepi", class_run, reverse_stepi, _("\
 Step backward exactly one instruction.\n\
 Argument N means do this N times (or till program stops for another reason).")
 	   );
-  add_com_alias ("rsi", "reverse-stepi", class_alias, 0);
+  add_com_alias ("rsi", "reverse-stepi", class_run, 0);
 
   add_com ("reverse-nexti", class_run, reverse_nexti, _("\
 Step backward one instruction, but proceed through called subroutines.\n\
 Argument N means do this N times (or till program stops for another reason).")
 	   );
-  add_com_alias ("rni", "reverse-nexti", class_alias, 0);
+  add_com_alias ("rni", "reverse-nexti", class_run, 0);
 
   add_com ("reverse-continue", class_run, reverse_continue, _("\
 Continue program being debugged but run it in reverse.\n\
 If proceeding from breakpoint, a number N may be used as an argument,\n\
 which means to set the ignore count of that breakpoint to N - 1 (so that\n\
 the breakpoint won't break until the Nth time it is reached)."));
-  add_com_alias ("rc", "reverse-continue", class_alias, 0);
+  add_com_alias ("rc", "reverse-continue", class_run, 0);
 
   add_com ("reverse-finish", class_run, reverse_finish, _("\
 Execute backward until just before selected stack frame is called."));
diff --git a/gdb/stack.c b/gdb/stack.c
index e8a9a924d4..11f6412b9d 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -3485,7 +3485,7 @@ With a negative COUNT, print outermost -COUNT frames."),
 
   add_com_alias ("bt", "backtrace", class_stack, 0);
 
-  add_com_alias ("where", "backtrace", class_alias, 0);
+  add_com_alias ("where", "backtrace", class_stack, 0);
   add_info ("stack", backtrace_command,
 	    _("Backtrace of the stack, or innermost COUNT frames."));
   add_info_alias ("s", "stack", 1);
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 7c862d5513..946443f07a 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3906,8 +3906,8 @@ on its own."), &cmdlist);
 			_("Commands for debugging overlays."), &overlaylist,
 			"overlay ", 0, &cmdlist);
 
-  add_com_alias ("ovly", "overlay", class_alias, 1);
-  add_com_alias ("ov", "overlay", class_alias, 1);
+  add_com_alias ("ovly", "overlay", class_support, 1);
+  add_com_alias ("ov", "overlay", class_support, 1);
 
   add_cmd ("map-overlay", class_support, map_overlay_command,
 	   _("Assert that an overlay section is mapped."), &overlaylist);
diff --git a/gdb/testsuite/gdb.base/alias.exp b/gdb/testsuite/gdb.base/alias.exp
index 69cfde67c1..6993d42648 100644
--- a/gdb/testsuite/gdb.base/alias.exp
+++ b/gdb/testsuite/gdb.base/alias.exp
@@ -122,3 +122,6 @@ gdb_test_no_output "alias abcd  = backtrace"
 gdb_test_no_output "alias abcde = backtrace"
 gdb_test_no_output "alias fghij = backtrace"
 gdb_test_no_output "alias fghi  = backtrace"
+
+# Verify help aliases shows the user defined aliases
+gdb_test "help aliases" ".*abcd --.*.*abcde --.*"
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index aa6bea4a8f..e7c7e50c0e 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -4098,8 +4098,8 @@ one or more \"collect\" commands, to specify what to collect\n\
 while single-stepping.\n\n\
 Note: this command can only be used in a tracepoint \"actions\" list."));
 
-  add_com_alias ("ws", "while-stepping", class_alias, 0);
-  add_com_alias ("stepping", "while-stepping", class_alias, 0);
+  add_com_alias ("ws", "while-stepping", class_trace, 0);
+  add_com_alias ("stepping", "while-stepping", class_trace, 0);
 
   add_com ("collect", class_trace, collect_pseudocommand, _("\
 Specify one or more data items to be collected at a tracepoint.\n\
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFA 10/10] Update NEWS and documentation for help and apropos changes.
  2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
                   ` (8 preceding siblings ...)
  2020-05-10 20:55 ` [RFA 09/10] Ensure class_alias is only used for user-defined aliases Philippe Waroquiers
@ 2020-05-10 20:55 ` Philippe Waroquiers
  2020-05-11 14:34   ` Eli Zaretskii
  9 siblings, 1 reply; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-10 20:55 UTC (permalink / raw)
  To: gdb-patches

gdb/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* NEWS: Mention changes to help and apropos.

gdb/doc/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.texinfo (Help): Document the help and apropos changes.
	(Aliases): Document new meaning of -a abbreviation flag.
---
 gdb/NEWS            | 10 ++++++++++
 gdb/doc/gdb.texinfo | 22 ++++++++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 5b9eabe746..5b416006cc 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,16 @@
 
 *** Changes since GDB 9
 
+* help and apropos commands will now show only once the help
+  for a command and all its aliases, instead of showing several
+  times the same help information.  When a command has one
+  or more aliases, the help documentation shows the command name
+  followed by all its aliases separated by commas, such as
+  'backtrace, where, bt'.
+
+* 'help aliases' now only shows the user defined aliases.  GDB predefined
+  aliases are shown together with their aliased command.
+
 * GDB now supports debuginfod, an HTTP server for distributing ELF/DWARF
   debugging information as well as source code.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d5bf59349e..4458144789 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2041,8 +2041,10 @@ Command name abbreviations are allowed if unambiguous.
 
 @item help @var{class}
 Using one of the general help classes as an argument, you can get a
-list of the individual commands in that class.  For example, here is the
-help display for the class @code{status}:
+list of the individual commands in that class.  If a command has
+aliases, the aliases are given after the command name, separated by
+commas. For example, here is the help display for the class
+@code{status}:
 
 @smallexample
 (@value{GDBP}) help status
@@ -2052,9 +2054,11 @@ List of commands:
 
 @c Line break in "show" line falsifies real output, but needed
 @c to fit in smallbook page size.
-info -- Generic command for showing things
+info, inf, i -- Generic command for showing things
         about the program being debugged
-show -- Generic command for showing things
+info address -- Describe where symbol SYM is stored.
+...
+show, info set -- Generic command for showing things
         about the debugger
 
 Type "help" followed by command name for full
@@ -2065,7 +2069,9 @@ Command name abbreviations are allowed if unambiguous.
 
 @item help @var{command}
 With a command name as @code{help} argument, @value{GDBN} displays a
-short paragraph on how to use that command.
+short paragraph on how to use that command.  If that command has
+one or more aliases, @value{GDBN} will display a first line with
+the command name and all its aliases separated by commas.
 
 @kindex apropos
 @item apropos [-v] @var{regexp}
@@ -2087,9 +2093,6 @@ results in:
 @group
 alias -- Define a new command that is an alias of an existing command
 aliases -- Aliases of other commands
-d -- Delete some breakpoints or auto-display expressions
-del -- Delete some breakpoints or auto-display expressions
-delete -- Delete some breakpoints or auto-display expressions
 @end group
 @end smallexample
 
@@ -27512,8 +27515,7 @@ underscores.
 that is being aliased.
 
 The @samp{-a} option specifies that the new alias is an abbreviation
-of the command.  Abbreviations are not shown in command
-lists displayed by the @samp{help} command.
+of the command.  Abbreviations are not used in command completion.
 
 The @samp{--} option specifies the end of options,
 and is useful when @var{ALIAS} begins with a dash.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 10/10] Update NEWS and documentation for help and apropos changes.
  2020-05-10 20:55 ` [RFA 10/10] Update NEWS and documentation for help and apropos changes Philippe Waroquiers
@ 2020-05-11 14:34   ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2020-05-11 14:34 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

> Date: Sun, 10 May 2020 22:55:30 +0200
> From: Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org>
> 
> gdb/ChangeLog
> 
> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* NEWS: Mention changes to help and apropos.
> 
> gdb/doc/ChangeLog
> 
> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.texinfo (Help): Document the help and apropos changes.
> 	(Aliases): Document new meaning of -a abbreviation flag.

Thanks, a couple of comments below.

> +* help and apropos commands will now show only once the help
> +  for a command and all its aliases, instead of showing several
> +  times the same help information.  When a command has one
> +  or more aliases, the help documentation shows the command name
> +  followed by all its aliases separated by commas, such as
> +  'backtrace, where, bt'.

Took me a few readings to understand what "show only once" in the 1st
sentence wants to say.  I suggest to rephrase:

  Help and apropos commands will now show the documentation of a
  command only once, even if that command has one or more aliases.
  These commands now show the command name, then all of its aliases,
  and finally the description of the command.

> +* 'help aliases' now only shows the user defined aliases.  GDB predefined

"only shows" => "shows only".  The former could be interpreted to mean
it shows the aliases and nothing else.

The documentation parts are OK with those changes.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 01/10] Add a selftest that detects a 'corrupted' command tree structure in GDB.
  2020-05-10 20:55 ` [RFA 01/10] Add a selftest that detects a 'corrupted' command tree structure in GDB Philippe Waroquiers
@ 2020-05-14 15:51   ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 15:51 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Philippe> This commit introduces a selftest that detects (at least some cases of) errors
Philippe> leading to 'next' and '*prefixlist' not giving a tree structure.

Thank you.

Philippe> +/* Verify that a list of commands is present in the tree only once.  */
Philippe> +
Philippe> +static void
Philippe> +command_structure_invariants_tests ()
Philippe> +{
Philippe> +
Philippe> +  traverse_command_structure (&cmdlist, "");

Spurious blank line after the "{".

Philippe> +
Philippe> +  /* Release memory, be ready to be re-run.  */
Philippe> +  lists.clear ();
Philippe> +
Philippe> +  SELF_CHECK (nr_duplicates == 0);

This should also reset nr_duplicates.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 02/10] Fix the only incorrect case found by command_structure_invariants selftest.
  2020-05-10 20:55 ` [RFA 02/10] Fix the only incorrect case found by command_structure_invariants selftest Philippe Waroquiers
@ 2020-05-14 15:52   ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 15:52 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Philippe> The command 'info set' was defined as a specific prefix command,
Philippe> but re-using the command list already used for the 'show' command.
Philippe> This leads to the command tree 'next/*prefixlist' to not be a tree.

Philippe> This change defines 'info set ' as an alias, thereby fixing the selftest.

I didn't even know about "info set".

This is ok, thanks.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 04/10] command-def-selftests.c: detect missing or wrong prefix cmd in subcommands.
  2020-05-10 20:55 ` [RFA 04/10] command-def-selftests.c: detect missing or wrong prefix cmd in subcommands Philippe Waroquiers
@ 2020-05-14 15:54   ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 15:54 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Thank you for the patch.

Philippe> This test reveals a number of problems fixed in the next commit.

It's best (IME) to develop in this order, but then to either combine the
test and the fix, or to put the fix before the test when submitting.
The reason for this is that it helps avoid failures when bisecting.

Philippe> +  SELF_CHECK (nr_invalid_prefixcmd == 0);

This code should reset nr_invalid_prefixcmd as well.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 03/10] Fix problem that alias can be defined or not depending on the order.
  2020-05-10 20:55 ` [RFA 03/10] Fix problem that alias can be defined or not depending on the order Philippe Waroquiers
@ 2020-05-14 16:00   ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 16:00 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Philippe> gdb/ChangeLog
Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* cli/cli-cmds.c (alias_command): Check for an existing alias
Philippe> 	using lookup_cmd_composition, as valid_command_p is too strict
Philippe> 	and forbids aliases that are the prefix of an existing alias
Philippe> 	or command.
Philippe> 	* cli/cli-decode.c (lookup_cmd_composition): Ensure a prefix
Philippe> 	command is properly recognised as a valid command.

Philippe> gdb/testsuite/ChangeLog
Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* gdb.base/alias.exp: Test aliases starting with a prefix of
Philippe> 	another alias.

Thank you for the patch.  This looks good to me.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 05/10] Fix the problems reported by prefix check of command-def-selftests.c
  2020-05-10 20:55 ` [RFA 05/10] Fix the problems reported by prefix check of command-def-selftests.c Philippe Waroquiers
@ 2020-05-14 16:07   ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 16:07 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Philippe> The command structure selftest detects a series of problems
Philippe> that are fixed by this commit.

Thank you.

Philippe> Caused by initialize_remote_fileio that was taking the address of
Philippe> its arguments remote_set_cmdlist and remote_show_cmdlist instead
Philippe> of receiving the correct values to use as list.

Amazing this lasted so long.

Philippe> 	* cli/cli-decode.c (lookup_cmd_for_prefix): Return the aliased command
Philippe> 	as prefix, not one of its aliases.
Philippe> 	(set_cmd_prefix): Remove.
Philippe> 	(do_add_cmd): Centralize the setting of the prefix of a command, when
Philippe> 	command is defined after its full chain of prefix commands.
Philippe> 	(add_alias_cmd): Remove call to set_cmd_prefix, as do_add_cmd does it.
Philippe> 	(add_setshow_cmd_full): Likewise.
Philippe> 	(update_prefix_field_of_prefixed_commands): New function.
Philippe> 	(add_prefix_cmd): Replace non working call to set_cmd_prefix by
Philippe> 	update_prefix_field_of_prefixed_commands.
Philippe> 	* gdb/remote-fileio.c (initialize_remote_fileio): Use the real
Philippe> 	addresses of remote_set_cmdlist and remote_show_cmdlist given
Philippe> 	as argument, not the address of an argument.
Philippe> 	* gdb/remote-fileio.h (initialize_remote_fileio): Likewise.
Philippe> 	* gdb/remote.c (_initialize_remote): Likewise.

Ok.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 06/10] Fix inconsistent output of prefix and bugs in 'show' command
  2020-05-10 20:55 ` [RFA 06/10] Fix inconsistent output of prefix and bugs in 'show' command Philippe Waroquiers
@ 2020-05-14 16:16   ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 16:16 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* cli/cli-setshow.h (cmd_show_list): Remove prefix argument.
Philippe> 	* cli/cli-decode.c (do_show_prefix_cmd): Likewise.
Philippe> 	* command.h (cmd_show_list): Likewise.
Philippe> 	* dwarf2/index-cache.c (show_index_cache_command): Likewise.
Philippe> 	* cli/cli-setshow.c (cmd_show_list): Use the prefix to produce the output.  Skip aliases.

Looks good.  Thank you.

Philippe> -      if (list->prefixlist && !list->abbrev_flag)
Philippe> +      if (list->prefixlist && list->cmd_pointer == nullptr)

It wasn't immediately apparent to me why checking cmd_pointer is the
correct thing to do here, but that's just because I forgot what it was
for.  The comment in cli-decode.h made it clear.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 07/10] Fix/improve 'help CLASS' output
  2020-05-10 20:55 ` [RFA 07/10] Fix/improve 'help CLASS' output Philippe Waroquiers
@ 2020-05-14 16:26   ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 16:26 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Philippe> The output layout chosen is to have the command first, followed by all its
Philippe> aliases separated by a comma.  Note that the command and alias names are
Philippe> title-styled.  For sure, other layouts could be discussed, but this one is IMO
Philippe> readable and compact.

I think it's good this way.

Philippe> +static void help_cmd_list (struct cmd_list_element *list, enum command_class theclass,

This line looks to be too long.

Philippe> -  help_cmd_list (list, theclass, cmdtype, (int) theclass >= 0, stream);
Philippe> +  help_cmd_list (list, theclass, (int) theclass >= 0, stream);

I think you might as well remove the "(int)" cast while you're touching
this line.  I don't believe it's needed.

Otherwise this looks good to me.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 08/10] Fix/improve 'apropos' output
  2020-05-10 20:55 ` [RFA 08/10] Fix/improve 'apropos' output Philippe Waroquiers
@ 2020-05-14 16:30   ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 16:30 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* cli/cli-decode.c (apropos_cmd): Produce output for aliases
Philippe> 	when their aliased command is traversed.
Philippe> 	(help_cmd): Add fput_command_names_styled call to
Philippe> 	output command name and aliases when command has an alias.

Philippe> gdb/testsuite/ChangeLog

Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* gdb.base/help.exp: Test apropos and help for commands
Philippe> 	having aliases.

Thanks.

Philippe> +	  /*  Command aliases/abbreviations are skipped to ensure we print the
Philippe> +	      doc of a command only once, when encountering the aliased
Philippe> +	      command.  */

The leading indent in the comment looks weird.

Philippe> +# test apropos for commands having aliases.
Philippe> +gdb_test "apropos Print backtrace of all stack frames, or innermost COUNT frames\." \
Philippe> +    "backtrace, where, bt -- Print backtrace of all stack frames, or innermost COUNT frames\."
Philippe> +
Philippe> +# test help for commands having aliases.
Philippe> +gdb_test "help bt" "backtrace, where, bt\[\r\n\]+Print backtrace of all stack frames, or innermost COUNT frames\..*"

The new comments here should start with an upper-case letter.

Ok with those nits fixed.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 09/10] Ensure class_alias is only used for user-defined aliases.
  2020-05-10 20:55 ` [RFA 09/10] Ensure class_alias is only used for user-defined aliases Philippe Waroquiers
@ 2020-05-14 17:33   ` Tom Tromey
  2020-05-14 21:12     ` Philippe Waroquiers
  0 siblings, 1 reply; 22+ messages in thread
From: Tom Tromey @ 2020-05-14 17:33 UTC (permalink / raw)
  To: Philippe Waroquiers via Gdb-patches

>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:

Philippe> This commit finally does the (small) change that started this patch
Philippe> series.

Isn't that always the way.

Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* command.h (enum command_class): Improve comments, document
Philippe> 	that class_alias is for user-defined aliases, give the class
Philippe> 	name for each class, remove unused class_xdb.
Philippe> 	* cli/cli-decode.c (add_com_alias): Document THECLASS intended usage.
Philippe> 	* breakpoint.c (_initialize_breakpoint): Replace class_alias
Philippe> 	by a precise class.
Philippe> 	* infcmd.c (_initialize_infcmd): Likewise.
Philippe> 	* reverse.c (_initialize_reverse): Likewise.
Philippe> 	* stack.c (_initialize_stack): Likewise.
Philippe> 	* symfile.c (_initialize_symfile): Likewise.
Philippe> 	* tracepoint.c (_initialize_tracepoint): Likewise.

Looks good.

Philippe> +  add_com_alias ("tp", "trace", class_breakpoint, 0);
Philippe> +  add_com_alias ("tr", "trace", class_breakpoint, 1);
Philippe> +  add_com_alias ("tra", "trace", class_breakpoint, 1);
Philippe> +  add_com_alias ("trac", "trace", class_breakpoint, 1);

I wonder if there's ever a case where we want an alias to have a
different class from the thing it aliases.  If not, maybe add_com_alias
could just do this and we could remove the parameter.

Philippe>  enum command_class
Philippe>  {

Thanks for this.

This is ok.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFA 09/10] Ensure class_alias is only used for user-defined aliases.
  2020-05-14 17:33   ` Tom Tromey
@ 2020-05-14 21:12     ` Philippe Waroquiers
  0 siblings, 0 replies; 22+ messages in thread
From: Philippe Waroquiers @ 2020-05-14 21:12 UTC (permalink / raw)
  To: Tom Tromey, Philippe Waroquiers via Gdb-patches

On Thu, 2020-05-14 at 11:33 -0600, Tom Tromey wrote:
> Philippe> +  add_com_alias ("tp", "trace", class_breakpoint, 0);
> Philippe> +  add_com_alias ("tr", "trace", class_breakpoint, 1);
> Philippe> +  add_com_alias ("tra", "trace", class_breakpoint, 1);
> Philippe> +  add_com_alias ("trac", "trace", class_breakpoint, 1);
> 
> I wonder if there's ever a case where we want an alias to have a
> different class from the thing it aliases.  If not, maybe add_com_alias
> could just do this and we could remove the parameter.
I also wondered but decided to not do this, at least for the moment.

The advantage of allowing another class for an alias and for its aliased
command is that this can be used to 'market' the same GDB functionality
via different classes.
 
Now that the help shows the command and all its aliases, that might
help some users to find a certain command via different classes,
in case such a command could reasonably be related to more than
one class.

Thanks for the review, I have handled all your comments and the comments
of Eli about the documentation, and will send an RFAv2 soon.

Thanks

Philippe



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-05-14 21:12 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-10 20:55 [RFA 00/10] fix/improve cmd structure, class_alias, help, apropos Philippe Waroquiers
2020-05-10 20:55 ` [RFA 01/10] Add a selftest that detects a 'corrupted' command tree structure in GDB Philippe Waroquiers
2020-05-14 15:51   ` Tom Tromey
2020-05-10 20:55 ` [RFA 02/10] Fix the only incorrect case found by command_structure_invariants selftest Philippe Waroquiers
2020-05-14 15:52   ` Tom Tromey
2020-05-10 20:55 ` [RFA 03/10] Fix problem that alias can be defined or not depending on the order Philippe Waroquiers
2020-05-14 16:00   ` Tom Tromey
2020-05-10 20:55 ` [RFA 04/10] command-def-selftests.c: detect missing or wrong prefix cmd in subcommands Philippe Waroquiers
2020-05-14 15:54   ` Tom Tromey
2020-05-10 20:55 ` [RFA 05/10] Fix the problems reported by prefix check of command-def-selftests.c Philippe Waroquiers
2020-05-14 16:07   ` Tom Tromey
2020-05-10 20:55 ` [RFA 06/10] Fix inconsistent output of prefix and bugs in 'show' command Philippe Waroquiers
2020-05-14 16:16   ` Tom Tromey
2020-05-10 20:55 ` [RFA 07/10] Fix/improve 'help CLASS' output Philippe Waroquiers
2020-05-14 16:26   ` Tom Tromey
2020-05-10 20:55 ` [RFA 08/10] Fix/improve 'apropos' output Philippe Waroquiers
2020-05-14 16:30   ` Tom Tromey
2020-05-10 20:55 ` [RFA 09/10] Ensure class_alias is only used for user-defined aliases Philippe Waroquiers
2020-05-14 17:33   ` Tom Tromey
2020-05-14 21:12     ` Philippe Waroquiers
2020-05-10 20:55 ` [RFA 10/10] Update NEWS and documentation for help and apropos changes Philippe Waroquiers
2020-05-11 14:34   ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).