PR cli/21688: Detect aliases when issuing python/compile/guile commands (and fix last commit)
Commit Message
My last commit fixed a regression that happened when using
inline/multi-line commands for Python/Compile/Guile, but introduced
another regression: it is now not possible to use aliases for the
commands mentioned above. The fix is to almost revert the change I've
made and go back to using the 'struct cmd_list_element *', but at the
same time make sure that we advance the 'cmd_name' variable past all
the whitespace characters after the command name. If, after skipping
the whitespace, we encounter a '\0', it means that the command is not
inline. Otherwise, it is.
This patch also expands the testcase in order to check for aliases and
for trailing whitespace after the command name.
gdb/ChangeLog:
2017-06-30 Sergio Durigan Junior <sergiodj@redhat.com>
Pedro Alves <palves@redhat.com>
PR cli/21688
* cli/cli-script.c (command_name_equals_not_inline): Remove function.
(process_next_line): New variables 'lookup_cmd' and 'inline'.
Adjust 'if' clauses for "python", "compile" and "guile" to use
'command_name_equals' and check for '!inline'.
gdb/testsuite/ChangeLog:
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.
---
gdb/ChangeLog | 9 +++++++++
gdb/cli/cli-script.c | 22 +++++-----------------
gdb/testsuite/ChangeLog | 6 ++++++
gdb/testsuite/gdb.python/py-cmd.exp | 15 ++++++++++++++-
4 files changed, 34 insertions(+), 18 deletions(-)
Comments
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
@@ -1,4 +1,13 @@
2017-06-30 Sergio Durigan Junior <sergiodj@redhat.com>
+ Pedro Alves <palves@redhat.com>
+
+ PR cli/21688
+ * cli/cli-script.c (command_name_equals_not_inline): Remove function.
+ (process_next_line): New variables 'lookup_cmd' and 'inline'.
+ Adjust 'if' clauses for "python", "compile" and "guile" to use
+ 'command_name_equals' and check for '!inline'.
+
+2017-06-30 Sergio Durigan Junior <sergiodj@redhat.com>
PR cli/21688
* cli/cli-script.c (command_name_equals_not_inline): New function.
@@ -900,20 +900,6 @@ command_name_equals (struct cmd_list_element *cmd, const char *name)
&& strcmp (cmd->name, name) == 0);
}
-/* Return true if NAME is the only command between COMMAND_START and
- COMMAND_END. This is useful when we want to know whether the
- command is inline (i.e., has arguments like 'python command1') or
- is the start of a multi-line command block. */
-
-static bool
-command_name_equals_not_inline (const char *command_start,
- const char *command_end,
- const char *name)
-{
- return (command_end - command_start == strlen (name)
- && startswith (command_start, name));
-}
-
/* Given an input line P, skip the command and return a pointer to the
first argument. */
@@ -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';
/* If commands are parsed, we skip initial spaces. Otherwise,
which is the case for Python commands and documentation
@@ -1011,20 +999,20 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
{
*command = build_command_line (commands_control, line_first_arg (p));
}
- else if (command_name_equals_not_inline (p_start, p_end, "python"))
+ else if (command_name_equals (cmd, "python") && !inline_cmd)
{
/* Note that we ignore the inline "python command" form
here. */
*command = build_command_line (python_control, "");
}
- else if (command_name_equals_not_inline (p_start, p_end, "compile"))
+ else if (command_name_equals (cmd, "compile") && !inline_cmd)
{
/* Note that we ignore the inline "compile command" form
here. */
*command = build_command_line (compile_control, "");
(*command)->control_u.compile.scope = COMPILE_I_INVALID_SCOPE;
}
- else if (command_name_equals_not_inline (p_start, p_end, "guile"))
+ else if (command_name_equals (cmd, "guile") && !inline_cmd)
{
/* Note that we ignore the inline "guile command" form here. */
*command = build_command_line (guile_control, "");
@@ -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.
@@ -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" } }
+
+ foreach t [list $define_cmd_not_inline $define_cmd_alias_not_inline $define_cmd_inline $define_cmd_alias_inline] {
foreach l $t {
lassign $l command regex testmsg
gdb_test_multiple "$command" "$testmsg" {