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

[Pedro Alves <palves@redhat.com>]

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