Re: [RFAv2] Allow to document user-defined aliases.

[Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org>]

Ping ?

Note that the doc was reviewed by Eli (ok with a few nits to fix)
and a first code review was done by Tom (I handled the minor comments given
+ ensured that the alias definition is shown in the help of a user-documented alias.

Thanks
Philippe

On Mon, 2022-08-08 at 21:40 +0200, Philippe Waroquiers wrote:
> Compared to the previous version, this version fixes the comments reported by
> Tom Tromey and ensures that the 'help some-user-documented-alias'
> shows the alias definition to ensure the user understands this is an
> alias even if specifically documented.
> 
> When using 'help ALIASNAME', GDB shows the help of the aliased command.
> This is a good default behaviour.
> 
> However, GDB alias command allows to define aliases with arguments
> possibly changing or tuning significantly the behaviour of
> the aliased command.  In such a case, showing the help of the aliased
> command might not be ideal.
> 
> This is particularly true when defining an alias as a set of
> nested 'with' followed by a last command to launch, such as:
>   (gdb) alias pp10 = with print pretty -- with print elements 10 -- print
> Asking 'help pp10' shows the help of the 'with' command, which is
> not particularly useful:
>   (gdb) help pp10
>   with, pp10, w
>     alias pp10 = with print pretty -- with print elements 10 -- print
>   Temporarily set SETTING to VALUE, run COMMAND, and restore SETTING.
>   Usage: with SETTING [VALUE] [-- COMMAND]
>   ....
> 
> Such an alias can now be documented by the user:
>   (gdb) document pp10
>   >Pretty printing an expressiong, printing 10 elements.
>   >Usage: pp10 [PRINT-COMMAND-OPTIONS] EXP
>   >See 'help print' for more information.
>   >end
>   (gdb) help pp10
>     alias pp10 = with print pretty -- with print elements 10 -- print
>   Pretty printing an expressiong, printing 10 elements.
>   Usage: pp10 [PRINT-COMMAND-OPTIONS] EXP
>   See 'help print' for more information.
>   (gdb)
> 
> When a user-defined alias is documented specifically, help and apropos
> use the provided alias documentation instead of the documentation of
> the aliased command.
> 
> Such a documented alias is also not shown anymore in the help of the
> aliased command, and the alias is not listed anymore in the help
> of the aliased command.  In particular for cases such as pp10 example above,
> indicating that pp10 is an alias of the 'with' command is confusing.
> ---
>  gdb/NEWS                        | 10 ++++++
>  gdb/cli/cli-decode.c            | 59 ++++++++++++++++++++++++---------
>  gdb/cli/cli-script.c            | 41 ++++++++++++++---------
>  gdb/doc/gdb.texinfo             | 26 ++++++++++++++-
>  gdb/testsuite/gdb.base/help.exp | 44 ++++++++++++++++++++++--
>  5 files changed, 146 insertions(+), 34 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 8c837df76e5..1b74e0fc1bd 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -93,6 +93,16 @@ show style disassembler symbol
>  
> 
>  * Changed commands
>  
> 
> +document user-defined
> +  It is now possible to document user-defined aliases.
> +  When a user-defined alias is documented, the help and apropos commands
> +  use the provided documentation instead of the documentation of the
> +  aliased command.
> +  Documenting a user-defined alias is particularly useful when the alias
> +  is a set of nested 'with' commands to avoid showing the help of
> +  the with command for an alias that will in fact launch the
> +  last command given in the nested commands.
> +
>  maintenance info line-table
>    Add a PROLOGUE-END column to the output which indicates that an
>    entry corresponds to an address where a breakpoint should be placed
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index fde554c7e6c..7c98029f9f4 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -1349,6 +1349,18 @@ fput_command_name_styled (const cmd_list_element &c, struct ui_file *stream)
>  		  prefixname.c_str (), c.name);
>  }
>  
> 
> +/* True if ALIAS has a user-defined documentation.  */
> +
> +static bool
> +user_documented_alias (const cmd_list_element &alias)
> +{
> +  gdb_assert (alias.is_alias ());
> +  /* Alias is user documented if it has an allocated documentation
> +     that differs from the aliased command.  */
> +  return (alias.doc_allocated
> +	  && strcmp (alias.doc, alias.alias_target->doc) != 0);
> +}
> +
>  /* Print the definition of alias C using title style for alias
>     and aliased command.  */
>  
> 
> @@ -1364,20 +1376,22 @@ fput_alias_definition_styled (const cmd_list_element &c,
>    gdb_printf (stream, " %s\n", c.default_args.c_str ());
>  }
>  
> 
> -/* Print the definition of the aliases of CMD that have default args.  */
> +/* Print the definition of CMD aliases not deprecated and having default args
> +   and not specifically documented by the user.  */
>  
> 
>  static void
>  fput_aliases_definition_styled (const cmd_list_element &cmd,
>  				struct ui_file *stream)
>  {
>    for (const cmd_list_element &alias : cmd.aliases)
> -    if (!alias.cmd_deprecated && !alias.default_args.empty ())
> +    if (!alias.cmd_deprecated
> +	&& !user_documented_alias (alias)
> +	&& !alias.default_args.empty ())
>        fput_alias_definition_styled (alias, stream);
>  }
>  
> 
> -
> -/* If C has one or more aliases, style print the name of C and
> -   the name of its aliases, separated by commas.
> +/* If C has one or more aliases, style print the name of C and the name of its
> +   aliases not documented specifically by the user, 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.
>  */
> @@ -1389,11 +1403,11 @@ fput_command_names_styled (const cmd_list_element &c,
>  {
>    /* First, check if we are going to print something.  That is, either if
>       ALWAYS_FPUT_C_NAME is true or if there exists at least one non-deprecated
> -     alias.  */
> +     alias not documented specifically by the user.  */
>  
> 
>    auto print_alias = [] (const cmd_list_element &alias)
>      {
> -      return !alias.cmd_deprecated;
> +      return !alias.cmd_deprecated && !user_documented_alias (alias);
>      };
>  
> 
>    bool print_something = always_fput_c_name;
> @@ -1474,11 +1488,11 @@ apropos_cmd (struct ui_file *stream,
>    /* Walk through the commands.  */
>    for (c=commandlist;c;c=c->next)
>      {
> -      if (c->is_alias ())
> +      if (c->is_alias () && !user_documented_alias (*c))
>  	{
> -	  /* Command aliases/abbreviations are skipped to ensure we print the
> -	     doc of a command only once, when encountering the aliased
> -	     command.  */
> +	  /* Command aliases/abbreviations not specifically documented by the
> +	     user are skipped to ensure we print the doc of a command only once,
> +	     when encountering the aliased command.  */
>  	  continue;
>  	}
>  
> 
> @@ -1571,11 +1585,24 @@ 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);
> -  fput_aliases_definition_styled (*c, stream);
> -  gdb_puts (c->doc, stream);
> +  if (alias == nullptr || !user_documented_alias (*alias))
> +    {
> +      /* Case of a normal command, or an alias not explictly
> +	 documented by the user.  */
> +      /* 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);
> +      fput_aliases_definition_styled (*c, stream);
> +      gdb_puts (c->doc, stream);
> +    }
> +  else
> +    {
> +      /* Case of an alias explictly documented by the user.
> +	 Only output the alias definition and its explicit documentation.  */
> +      fput_alias_definition_styled (*alias, stream);
> +      fput_command_names_styled (*alias, false, "\n", stream);
> +      gdb_puts (alias->doc, stream);
> +    }
>    gdb_puts ("\n", stream);
>  
> 
>    if (!c->is_prefix () && !c->is_command_class_help ())
> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index 5f81db418bc..6c67b60e721 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -1500,39 +1500,49 @@ define_command (const char *comname, int from_tty)
>    do_define_command (comname, from_tty, nullptr);
>  }
>  
> 
> -/* Document a user-defined command.  If COMMANDS is NULL, then this is a
> -   top-level call and the document will be read using read_command_lines.
> -   Otherwise, it is a "document" command in an existing command and the
> -   commands are provided.  */
> +/* Document a user-defined command or user defined alias.  If COMMANDS is NULL,
> +   then this is a top-level call and the document will be read using
> +   read_command_lines.  Otherwise, it is a "document" command in an existing
> +   command and the commands are provided.  */
>  static void
>  do_document_command (const char *comname, int from_tty,
>                       const counted_command_line *commands)
>  {
> -  struct cmd_list_element *c, **list;
> -  const char *tem;
> +  struct cmd_list_element *alias, *prefix_cmd, *c;
>    const char *comfull;
>  
> 
>    comfull = comname;
> -  list = validate_comname (&comname);
> +  validate_comname (&comname);
>  
> 
> -  tem = comname;
> -  c = lookup_cmd (&tem, *list, "", NULL, 0, 1);
> +  lookup_cmd_composition (comfull, &alias, &prefix_cmd, &c);
>  
> 
> -  if (c->theclass != class_user)
> -    error (_("Command \"%s\" is built-in."), comfull);
> +  if (c->theclass != class_user
> +      && (alias == nullptr || alias->theclass != class_alias))
> +    {
> +      if (alias == nullptr)
> +	error (_("Command \"%s\" is built-in."), comfull);
> +      else
> +	error (_("Alias \"%s\" is built-in."), comfull);
> +    }
> +
> +  /* If we found an alias of class_alias, the user is documenting this
> +     user-defined alias.  */
> +  if (alias != nullptr)
> +    c = alias;
>  
> 
>    counted_command_line doclines;
>  
> 
>    if (commands == nullptr)
>      {
> -      std::string prompt 
> +      std::string prompt
>          = string_printf ("Type documentation for \"%s\".", comfull);
>        doclines = read_command_lines (prompt.c_str (), from_tty, 0, 0);
>      }
>    else
>      doclines = *commands;
>  
> 
> -  xfree ((char *) c->doc);
> +  if (c->doc_allocated)
> +    xfree ((char *) c->doc);
>  
> 
>    {
>      struct command_line *cl1;
> @@ -1553,6 +1563,7 @@ do_document_command (const char *comname, int from_tty,
>        }
>  
> 
>      c->doc = doc;
> +    c->doc_allocated = 1;
>    }
>  }
>  
> 
> @@ -1681,8 +1692,8 @@ _initialize_cli_script ()
>       its prefixes.  */
>    document_cmd_element = add_com ("document", class_support, document_command,
>                                    _("\
> -Document a user-defined command.\n\
> -Give command name as argument.  Give documentation on following lines.\n\
> +Document a user-defined command or user-defined alias.\n\
> +Give command or alias name as argument.  Give documentation on following lines.\n\
>  End with a line of just \"end\"."));
>    set_cmd_completer (document_cmd_element, command_completer);
>    define_cmd_element = add_com ("define", class_support, define_command, _("\
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 382df00ee7d..efa5d48a707 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -2265,11 +2265,20 @@ one or more aliases, @value{GDBN} will display a first line with
>  the command name and all its aliases separated by commas.
>  This first line will be followed by the full definition of all aliases
>  having default arguments.
> +When asking the help for an alias, the documentation for the aliased
> +command is shown.
> +
> +A user-defined alias can optionally be documented using the @code{document} command
> +(@pxref{Define, document}).
> +@value{GDBN} then considers this alias as different of the aliased
> +command: this alias is not listed in the aliased command help output and asking
> +help for this alias will show the user provided alias documentation
> +instead of the documentation of the aliased command.
>  
> 
>  @kindex apropos
>  @item apropos [-v] @var{regexp}
>  The @code{apropos} command searches through all of the @value{GDBN}
> -commands, and their documentation, for the regular expression specified in
> +commands and aliases, and their documentation, for the regular expression specified in
>  @var{args}.  It prints out all matches found.  The optional flag  @samp{-v},
>  which stands for @samp{verbose}, indicates to output the full documentation
>  of the matching commands and highlight the parts of the documentation
> @@ -27967,6 +27976,13 @@ You may use the @code{document} command again to change the
>  documentation of a command.  Redefining the command with @code{define}
>  does not change the documentation.
>  
> 
> +It is also possible to document user-defined aliases.  The alias documentation
> +will then be used by the @code{help} and @code{apropos} commands
> +instead of the documentation of the aliased command.
> +Documenting a user-defined alias is particularly useful when defining
> +an alias as a set of nested @code{with} commands
> +(@pxref{Command aliases default args}).
> +
>  @kindex define-prefix
>  @item define-prefix @var{commandname}
>  Define or mark the command @var{commandname} as a user-defined prefix
> @@ -28641,6 +28657,14 @@ by the user.
>  For more information about the @code{with} command usage,
>  see @ref{Command Settings}.
>  
> 
> +By default, asking the help for an alias shows the documentation of
> +the aliased command.  When the alias is a set of nested commands, @code{help}
> +of an alias shows the documentation of the first command.  This help
> +is not particularly useful for an alias such as @code{pp10}.
> +For such an alias, it is useful to give a specific documentation
> +using the @code{document} command (@pxref{Define, document}).
> +
> +
>  @c Python docs live in a separate file.
>  @include python.texi
>  
> 
> diff --git a/gdb/testsuite/gdb.base/help.exp b/gdb/testsuite/gdb.base/help.exp
> index 74da29e3d36..5ee8ce0726d 100644
> --- a/gdb/testsuite/gdb.base/help.exp
> +++ b/gdb/testsuite/gdb.base/help.exp
> @@ -128,8 +128,48 @@ gdb_test "apropos handle signal" "handle -- Specify how to handle signals\."
>  gdb_test "apropos apropos" "apropos -- Search for commands matching a REGEXP.*"
>  
> 
>  # Test apropos for commands having aliases.
> +gdb_test_no_output "alias mybt = backtrace" "define mybt alias"
> +gdb_test_no_output "alias mybt10 = backtrace 10" "define mybt10 alias"
> +
>  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\."
> +    "backtrace, mybt10, mybt, where, bt -- Print backtrace of all stack frames, or innermost COUNT frames\.\[\r\n\]+  alias mybt10 = backtrace 10"
>  
> 
>  # 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\..*"
> +gdb_test "help bt" "backtrace, mybt10, mybt, where, bt\[\r\n\]+  alias mybt10 = backtrace 10\[\r\n\]+Print backtrace of all stack frames, or innermost COUNT frames\..*"
> +
> +# Document the aliases.  The apropos and help commands should then consider them
> +# as "standalone" commands.
> +gdb_test_multiple "document mybt" "document alias: mybt" {
> +    -re "Type documentation for \"mybt\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +	gdb_test "An alias of command backtrace without any args.\nend" \
> +	    "" \
> +	    "document alias: mybt"
> +    }
> +}
> +gdb_test_multiple "document mybt10" "document alias: mybt10" {
> +    -re "Type documentation for \"mybt10\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +	gdb_test "An alias of command backtrace with arg 10.\nend" \
> +	    "" \
> +	    "document alias: mybt10"
> +    }
> +}
> +
> +# As the aliases are now documented, they do not appear in apropos/help backtrace output anymore.
> +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\." \
> +    "apropos after documenting aliases"
> +
> +gdb_test "help bt" "backtrace, where, bt\[\r\n\]+Print backtrace of all stack frames, or innermost COUNT frames\..*" \
> +    "help after documenting aliases"
> +
> +# Check apropos and help use the alias documentation.
> +gdb_test "apropos An alias of command backtrace with arg 10" \
> +    "mybt10 -- An alias of command backtrace with arg 10\." \
> +    "apropos after documenting aliases showing mybt10 doc"
> +
> +gdb_test "help mybt" " alias mybt = backtrace \[\r\n\]+An alias of command backtrace without any args\." \
> +    "help mybt after documenting aliases showing mybt doc"
> +
> +# Check pre-defined aliases cannot be documented.
> +gdb_test "document where" "Alias \"where\" is built-in.*" \
> +    "documenting builtin where alias disallowed"