Re: [PATCH] PR cli/21688: Detect aliases when issuing python/compile/guile commands (and fix last commit)

Pedro Alves
2017-06-30T13:02:05Z

On 06/30/2017 01:34 PM, Sergio Durigan Junior wrote:

> @@ -966,6 +952,8 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
> const char *cmd_name = p;
> struct cmd_list_element *cmd
> = lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1);
> + const char *lookup_cmd = skip_spaces_const (cmd_name);
> + bool inline_cmd = *lookup_cmd != '\0';

The "lookup_cmd" in my suggestion:

~~~~~
lookup_cmd = skip_spaces_const (cmd_name);
~~~~~

was a typo/pasto from "lookup_cmd_1"... I meant:

cmd_name = skip_spaces_const (cmd_name);

Fine with me to use a new variable like you had, but
it should have a name that actually means something
related to the task at hand. "cmd_arg" or something.

> /* Note that we ignore the inline "guile command" form here. */
> *command = build_command_line (guile_control, "");
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 06bf5a4..6160c4b 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,6 +1,12 @@
> 2017-06-30 Sergio Durigan Junior <sergiodj@redhat.com>
>
> PR cli/21688
> + * gdb.python/py-cmd.exp (test_python_inline_or_multiline): Add new
> + tests for alias commands and trailing whitespace.
> +
> +2017-06-30 Sergio Durigan Junior <sergiodj@redhat.com>
> +
> + PR cli/21688
> * gdb.python/py-cmd.exp (test_python_inline_or_multiline): New
> procedure. Call it.
>
> diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp
> index 39bb785..287ecda 100644
> --- a/gdb/testsuite/gdb.python/py-cmd.exp
> +++ b/gdb/testsuite/gdb.python/py-cmd.exp
> @@ -194,12 +194,25 @@ proc test_python_inline_or_multiline { } {
> { "end" " >$" "multi-line first end" }
> { "end" "hello\r\n" "multi-line last end" } }
>
> + # This also tests trailing whitespace on the command.
> + set define_cmd_alias_not_inline {
> + { "if 1" " >$" "multi-line if 1 alias" }
> + { "py " " >$" "multi-line python command alias" }
> + { "print ('hello')" " >$" "multi-line print alias" }
> + { "end" " >$" "multi-line first end alias" }
> + { "end" "hello\r\n" "multi-line last end alias" } }
> +
> set define_cmd_inline {
> { "if 1" " >$" "inline if 1" }
> { "python print ('hello')" " >$" "inline python command" }
> { "end" "hello\r\n" "inline end" } }
>
> - foreach t [list $define_cmd_not_inline $define_cmd_inline] {
> + set define_cmd_alias_inline {
> + { "if 1" " >$" "inline if 1 alias" }
> + { "py print ('hello')" " >$" "inline python command alias" }
> + { "end" "hello\r\n" "inline end alias" } }
> +

Any reason you didn't add a test for the "alias foo=python" case?
We want to be sure that aliases that are not abbreviations are
handled too. "py" is probably really implemented as a
disambiguating alias, due to "python-interactive", but it could be an
abbreviation too [py, pyt, pyth], etc. I think an explicit test for
a non-abbreviation alias would be good, to be sure the code isn't just
doing a "startswith"-like check. Otherwise, I'm going to ask for a
test that exercises abbreviations, like for example "compil", but you
don't want that. :-)

Thanks,
Pedro Alves