Re: [RFA, doc RFA] Add gdb.add_command_alias

[Doug Evans <dje@google.com>]

[-- Attachment #1: Type: text/plain, Size: 7802 bytes --]

On Fri, Sep 9, 2011 at 2:13 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri,  9 Sep 2011 11:36:36 -0700 (PDT)
>> From: dje@google.com (Doug Evans)
>>
>> Per discussion on IRC, here is a patch to add support for adding
>> command aliases.
>
> I wish people would discuss such issues here, not on IRC.  These
> discussions should be recorded, for one thing.

Whatever additional discussion one wants to have we can have here.
No worries.

> Me, I don't understand the need for this feature, especially not why
> it should be a Python-only feature.  Moreover, why do we need to have
> this, when one can easily write a command that just calls an existing
> one, to have the same effect.

Conversely, why does gdb have cli-decode.c:add_com_alias?
IOW, it's more than just writing a command that just calls an existing one.
Python is our extension language and we want to, for example,
allow users to add new functionality to gdb that is on par with gdb's
own internal functionality.  In the case of gdb commands, take for example
the "run" and "print" commands.  There are alias commands of these ("r" and "p")
for good reasons.  This patch extends this capability to commands
created in python.

[For completeness sake,
There is still a capability that internal gdb commands have that python commands
don't: one can add an alias for more than just simple commands.
E.g.

valprint.c:2177:  add_alias_cmd ("p", "print", no_class, 1, &setlist);
valprint.c:2184:  add_alias_cmd ("p", "print", no_class, 1, &showlist);

We've been extending python incrementally, and I'm doing that here
as well.]

btw,

I've been debating whether to implement command aliases with a python
object (akin to how gdb commands are implemented with gdb.Command).
It's not clear an object-oriented approach for command aliases really
brings any functional utility to the table.  It would be consistent
with how commands are treated, at the expense of extra complexity.
I'm open to thoughts on which way to go.

I've also been debating whether to use the name define_command_alias
instead of add_command_alias.
I don't have a strong enough preference to not go with what gdb uses internally.

>> valid_cmd_name_p is more restrictive than it could be.
>> E.g. gdb allows a user-defined command named "42", but
>> "it's easier to relax restrictions than it is to impose them after the fact",
>> so I'm going with this.
>
> Sorry, I don't understand what this is about.  I guess that was again
> discussed "on IRC" or wherever.

Actually it's just a restriction I've added on my own.
I'm not entirely comfortable with gdb's allowing one to define
user-commands like that, and I don't want to allow this
in python if I don't have to.  At least not in the first pass.
If someone comes up with a compelling argument for
relaxing what valid_cmd_name_p accepts we can cross
that bridge at that time.

>> +   NOTE: TUI has a few special commands, +, <, >.
>> +   We don't watch for those here.  */
>
> Why not?  And what does that mean in terms of user expectations?

This is another case of not wanting to have valid_cmd_name_p
accept as legitimate names I'm not comfortable with.
At least not in the first pass.
As far as user expectation goes, I don't see a problem.
What are valid command names is documented, and the current
set of valid names is completely within reason IMO.
[I was thinking of disallowing "-foo", but didn't have a strong
enough opinion so I left it.]
Note that we *can* relax things later if a compelling
argument presents itself, I don't mind taking these particular
kinds of things one step at a time (and often prefer it).

IWBN to allow "!" be an alias for shell, for example,
but gdb currently doesn't allow it.
What special characters are valid and what are not, and why not?
That discussion needn't block this.

[For completeness sake,
Having gdb.Command validate the command name is left for another patch.
E.g., I can define a command named "!" and it will appear in "help" output,
but I can't execute it.
If/when we fix that we can relax valid_cmd_name_p: again,
I don't mind taking these particular kinds of things one step at a time.

bash$ cat foo.py
import gdb

class PlayCommand (gdb.Command):
  """Command to play with."""

  def __init__(self):
    super(PlayCommand, self).__init__("!", gdb.COMMAND_OBSCURE)

  def invoke(self, arg, from_tty):
    """GDB calls this to perform the command."""
    gdb.execute("shell %s" % arg)

PlayCommand()

class Play2Command (gdb.Command):
  """Command to play with."""

  def __init__(self):
    super(Play2Command, self).__init__("bang", gdb.COMMAND_OBSCURE)

  def invoke(self, arg, from_tty):
    """GDB calls this to perform the command."""
    gdb.execute("shell %s" % arg)

Play2Command()
bash$ gdb
(gdb) source foo.py
(gdb) help obscure
Obscure features.

List of commands:

! -- Command to play with
bang -- Command to play with
checkpoint -- Fork a duplicate process (experimental)
compare-sections -- Compare section data on target to the exec file
complete -- List the completions for the rest of the line as a command
monitor -- Send a command to the remote monitor (remote targets only)
python -- Evaluate a Python command
record -- Abbreviated form of "target record" command
record delete -- Delete the rest of execution log and start recording it anew
record goto -- Restore the program to its state at instruction number N
record restore -- Restore the execution log from a file
record save -- Save the execution log to a file
record stop -- Stop the record/replay target
restart -- Restart <n>: restore program context from a checkpoint
stop -- There is no `stop' command

Type "help" followed by command name for full documentation.
Type "apropos word" to search for commands related to "word".
Command name abbreviations are allowed if unambiguous.
(gdb) ! pwd
Undefined command: "".  Try "help".
(gdb) bang pwd
/home/dje/src/play
(gdb)

>> +This is useful, for example, when you want to be able to type a command
>> +with a long name using fewer characters, and the contraction is otherwise
>> +ambiguous.  It can also we used when you want to give a command an alternate
>                           ^^
> "be"

Righto, thanks.

>> +@var{name} is the name of the new command.
>> [..]
>> +Command names must begin with a letter, dash or underscore,
>> +and must consist of letters, numbers, dashes and underscores.
>
>  @var{name} is the name of the new command; it must begin with a
>  letter, dash or underscore, and must consist of letters, numbers,
>  dashes and underscores.

Ok.

>> +@var{command_class} should be one of the @samp{COMMAND_} constants.
>
> Please add here a cross-reference to where these constants are
> documented.

I was going to do that but since the values are documented
preceding this item on the same page,
and the existing text doesn't use a cross-reference
(and since I forgot how to label the text so that I could
cross-reference it :-))
I punted.

Here's a revised patch.

2011-09-13  Doug Evans  <dje@google.com>

        Add support for adding command aliases from python.
        * NEWS: Mention gdb.add_command_alias.
        * command.h (valid_cmd_name_p): Declare.
        * cli/cli-decode.c (valid_cmd_name_p): New function.
        * python/py-cmd.c (gdbpy_add_com_alias): New function.
        * python/python-internal.h (gdbpy_add_com_alias): Declare.
        * python/python.c (GdbMethods): Add add_command_alias.

        doc/
        * gdb.texinfo (Commands In Python): Document add_command_alias.

        testsuite/
        * gdb.python/py-cmd.exp: Add tests for gdb.add_command_alias.

[-- Attachment #2: gdb-110913-add-command-alias-2.patch.txt --]
[-- Type: text/plain, Size: 8548 bytes --]

2011-09-09  Doug Evans  <dje@google.com>

	Add support for adding command aliases from python.
	* NEWS: Mention gdb.add_command_alias.
	* command.h (valid_cmd_name_p): Declare.
	* cli/cli-decode.c (valid_cmd_name_p): New function.
	* python/py-cmd.c (gdbpy_add_com_alias): New function.
	* python/python-internal.h (gdbpy_add_com_alias): Declare.
	* python/python.c (GdbMethods): Add add_command_alias.

	doc/
	* gdb.texinfo (Commands In Python): Document add_command_alias.

	testsuite/
	* gdb.python/py-cmd.exp: Add tests for gdb.add_command_alias.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.452
diff -u -p -r1.452 NEWS
--- NEWS	4 Sep 2011 17:48:51 -0000	1.452
+++ NEWS	9 Sep 2011 18:07:50 -0000
@@ -38,6 +38,8 @@
 
   ** Symbols now provide the "type" attribute, the type of the symbol.
 
+  ** Command aliases can now be defined with the add_command_alias function.
+
 * libthread-db-search-path now supports two special values: $sdir and $pdir.
   $sdir specifies the default system locations of shared libraries.
   $pdir specifies the directory where the libpthread used by the application
Index: command.h
===================================================================
RCS file: /cvs/src/src/gdb/command.h,v
retrieving revision 1.74
diff -u -p -r1.74 command.h
--- command.h	14 Feb 2011 23:41:33 -0000	1.74
+++ command.h	9 Sep 2011 18:07:50 -0000
@@ -106,6 +106,8 @@ struct cmd_list_element;
 
 /* Forward-declarations of the entry-points of cli/cli-decode.c.  */
 
+extern int valid_cmd_name_p (const char *name);
+
 extern struct cmd_list_element *add_cmd (char *, enum command_class,
 					 void (*fun) (char *, int), char *,
 					 struct cmd_list_element **);
Index: cli/cli-decode.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-decode.c,v
retrieving revision 1.97
diff -u -p -r1.97 cli-decode.c
--- cli/cli-decode.c	8 Sep 2011 17:20:43 -0000	1.97
+++ cli/cli-decode.c	9 Sep 2011 18:07:50 -0000
@@ -126,6 +126,38 @@ set_cmd_completer (struct cmd_list_eleme
   cmd->completer = completer; /* Ok.  */
 }
 
+/* Return TRUE if NAME is a valid command name.
+
+   NOTE: TUI has a few special commands, +, <, >.
+   We don't watch for those here.  */
+
+int
+valid_cmd_name_p (const char *name)
+{
+  const char *p;
+
+  /* First character must be a letter, -, or _.  */
+  if (*name == '\0')
+    return FALSE;
+  if (isalpha (*name)
+      || *name == '-'
+      || *name == '_')
+    ; /* Ok.  */
+  else
+    return FALSE;
+
+  for (p = name + 1; *p != '\0'; ++p)
+    {
+      if (isalnum (*p)
+	  || *p == '-'
+	  || *p == '_')
+	; /* Ok.  */
+      else
+	return FALSE;
+    }
+
+  return TRUE;
+}
 
 /* Add element named NAME.
    Space for NAME and DOC must be allocated by the caller.
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.858
diff -u -p -r1.858 gdb.texinfo
--- doc/gdb.texinfo	4 Sep 2011 17:08:56 -0000	1.858
+++ doc/gdb.texinfo	9 Sep 2011 18:07:51 -0000
@@ -22668,6 +22668,35 @@ registration of the command with @value{
 Python code is read into @value{GDBN}, you may need to import the
 @code{gdb} module explicitly.
 
+@findex gdb.add_command_alias
+@defun add_command_alias name aliased_name command_class abbrev_flag
+Command aliases can be defined with the @code{gdb.add_command_alias} function.
+This is useful, for example, when you want to be able to type a command
+with a long name using fewer characters, and the contraction is otherwise
+ambiguous.  It can also we used when you want to give a command an alternate
+spelling.
+
+@var{name} is the name of the new command.
+@var{aliased_name} is the name of the command that is being aliased.
+Command names must begin with a letter, dash or underscore,
+and must consist of letters, numbers, dashes and underscores.
+
+@var{command_class} should be one of the @samp{COMMAND_} constants.
+
+@var{abbrev_flag} is a boolean flag specifying whether to treat the alias
+as an abbreviation of the original command or not.
+If the alias is an abbreviation it will not appear in @code{help}
+command list output.
+
+Here is an example where we make @code{e} an alternate spelling of the
+@code{x} command.
+
+@smallexample
+(gdb) python gdb.add_command_alias ("e", "x", gdb.COMMAND_DATA, 0)
+(gdb) e/10i main
+@end smallexample
+@end defun
+
 @node Parameters In Python
 @subsubsection Parameters In Python
 
Index: python/py-cmd.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-cmd.c,v
retrieving revision 1.16
diff -u -p -r1.16 py-cmd.c
--- python/py-cmd.c	8 Sep 2011 19:51:27 -0000	1.16
+++ python/py-cmd.c	9 Sep 2011 18:07:51 -0000
@@ -698,3 +698,34 @@ gdbpy_string_to_argv (PyObject *self, Py
 
   return py_argv;
 }
+
+\f
+
+/* Wrapper around add_com_alias.  */
+
+PyObject *
+gdbpy_add_com_alias (PyObject *self, PyObject *args, PyObject *kw)
+{
+  static char *keywords[] = {
+    "name", "aliased_name", "command_class", "abbrev_flag", NULL
+  };
+  char *name, *aliased_name;
+  int command_class, abbrev_flag;
+
+  if (! PyArg_ParseTupleAndKeywords (args, kw, "ssii", keywords,
+				     &name, &aliased_name,
+				     &command_class, &abbrev_flag))
+    return NULL;
+
+  if (! valid_cmd_name_p (name))
+    return PyErr_Format (PyExc_RuntimeError,
+			 _("Invalid command name `%s'."), name);
+  if (! valid_cmd_name_p (aliased_name))
+    return PyErr_Format (PyExc_RuntimeError,
+			 _("Invalid aliased command name `%s'."), aliased_name);
+
+  /* add_cmd requires *we* allocate space for name, hence the xstrdup.  */
+  add_com_alias (xstrdup (name), aliased_name, command_class, abbrev_flag);
+
+  Py_RETURN_NONE;
+}
Index: python/python-internal.h
===================================================================
RCS file: /cvs/src/src/gdb/python/python-internal.h,v
retrieving revision 1.47
diff -u -p -r1.47 python-internal.h
--- python/python-internal.h	8 Sep 2011 19:51:27 -0000	1.47
+++ python/python-internal.h	9 Sep 2011 18:07:52 -0000
@@ -152,6 +152,7 @@ PyObject *gdbpy_create_lazy_string_objec
 PyObject *gdbpy_inferiors (PyObject *unused, PyObject *unused2);
 PyObject *gdbpy_selected_thread (PyObject *self, PyObject *args);
 PyObject *gdbpy_string_to_argv (PyObject *self, PyObject *args);
+PyObject *gdbpy_add_com_alias (PyObject *self, PyObject *args, PyObject *kw);
 PyObject *gdbpy_parameter (PyObject *self, PyObject *args);
 PyObject *gdbpy_parameter_value (enum var_types type, void *var);
 char *gdbpy_parse_command_name (const char *name,
Index: python/python.c
===================================================================
RCS file: /cvs/src/src/gdb/python/python.c,v
retrieving revision 1.71
diff -u -p -r1.71 python.c
--- python/python.c	6 Sep 2011 14:49:00 -0000	1.71
+++ python/python.c	9 Sep 2011 18:07:52 -0000
@@ -1426,6 +1426,10 @@ Return the selected thread object." },
   { "inferiors", gdbpy_inferiors, METH_NOARGS,
     "inferiors () -> (gdb.Inferior, ...).\n\
 Return a tuple containing all inferiors." },
+  { "add_command_alias", (PyCFunction)gdbpy_add_com_alias,
+    METH_VARARGS | METH_KEYWORDS,
+    "Add an alias for an existing command." },
+
   {NULL, NULL, 0, NULL}
 };
 
Index: testsuite/gdb.python/py-cmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.python/py-cmd.exp,v
retrieving revision 1.6
diff -u -p -r1.6 py-cmd.exp
--- testsuite/gdb.python/py-cmd.exp	10 Jan 2011 11:00:23 -0000	1.6
+++ testsuite/gdb.python/py-cmd.exp	9 Sep 2011 18:07:52 -0000
@@ -45,6 +45,20 @@ gdb_py_test_multiple "input simple comma
 
 gdb_test "test_cmd ugh" "test_cmd output, arg = ugh" "call simple command"
 
+# Test an alias command.
+
+gdb_test_no_output "python gdb.add_command_alias (\"test_alias_cmd\", \"test_cmd\", gdb.COMMAND_OBSCURE, 0)" "add_command_alias"
+
+gdb_test "test_alias_cmd ugh" "test_cmd output, arg = ugh" "call alias command"
+
+gdb_test "python gdb.add_command_alias (\"(bad name)\", \"test_cmd\", gdb.COMMAND_OBSCURE, 0)" \
+  "RuntimeError: Invalid command name.*" \
+  "bad alias command name"
+
+gdb_test "python gdb.add_command_alias (\"test_alias_cmd\", \"(bad name)\", gdb.COMMAND_OBSCURE, 0)" \
+  "RuntimeError: Invalid aliased command name.*" \
+  "bad aliased command name"
+
 # Test a prefix command, and a subcommand within it.
 
 gdb_py_test_multiple "input prefix command" \