[RFA] completion in command definition
Commit Message
Simon Marchi (simon.marchi@polymtl.ca):
> Actually, I think you are confusing pointer and pointee.
> lookup_cmd_1 checks if result_list is NULL, not if *result_list is
> NULL. So with your version, since result_list is not NULL
> (&last_line is not NULL, it's the address of the variable)
> lookup_cmd_1 will set the last_line variable. There's no harm, but
> since we don't care about that value it's not useful either. I
> think what you want is:
>
> cmd = lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1);
You are absolutely right; I got confused by lookup_cmd_1's doc somehow.
New patch in attachment. Thanks again!
Comments
Hi Jerome,
On 01/12/2017 10:05 AM, Jerome Guitton wrote:
> commit 05bd4353619a49e8506bc6f8cd428b6a0961b28f
> Author: Jerome Guitton <guitton@adacore.com>
> Date: Tue Jan 10 15:15:53 2017 +0100
>
> [RFA] completion in command definition
>
> When defining a new macro, "command" is not recognized as an aliased for
> "commands":
>
> (gdb) define breakmain
> Type commands for definition of "breakmain".
> End with a line saying just "end".
> >break main
> >command
> >echo "IN MAIN\n"
> >end
> (gdb)
>
> There is a special case for while-stepping, where 'ws' and 'stepping' are
> recognized explicitely. We could add "command" to the list as well. I'd
> rather use cli-decode though. Patch in attachment, with a test, tested
> in x86-linux. OK to apply?
OK (but please remember to edit out the question and "attachment"
from the commit log).
>
> gdb/ChangeLog:
> * cli/cli-decode.c (find_command_name_length): Make it global.
s/global/extern/
> * cli/cli-decode.h (find_command_name_length): Declare.
> * cli/cli-script.c (command_name_equals, line_first_arg):
> New functions.
> (process_next_line): Use cli-decode to parse command names
Missing period.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/define.exp: Add test for completion in command definition.
>
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index dbd874e..b71f6f6 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -1255,7 +1255,7 @@ find_cmd (const char *command, int len, struct cmd_list_element *clist,
> return found;
> }
>
> -static int
> +int
> find_command_name_length (const char *text)
> {
> const char *p = text;
> diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
> index e5ab839..66159fd 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -253,4 +253,6 @@ extern const char * const auto_boolean_enums[];
>
> extern int cli_user_command_p (struct cmd_list_element *);
>
> +extern int find_command_name_length (const char *);
> +
Now that this is a public function, it'd be nice to
document its interface.
> #endif /* !defined (CLI_DECODE_H) */
> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index 6f44d52..f19df9e 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -143,7 +143,7 @@ multi_line_command_p (enum command_control_type type)
> control commands (if/while). */
>
> static struct command_line *
> -build_command_line (enum command_control_type type, char *args)
> +build_command_line (enum command_control_type type, const char *args)
Mention this in the ChangeLog.
> {
> struct command_line *cmd;
>
> @@ -904,6 +904,30 @@ read_next_line (void)
> return command_line_input (prompt_ptr, from_tty, "commands");
> }
>
> +/* Return non-zero if CMD's name is NAME. */
> +
> +static bool
> +command_name_equals (struct cmd_list_element *cmd, const char *name)
> +{
> + return cmd != NULL
> + && cmd != CMD_LIST_AMBIGUOUS
> + && strcmp (cmd->name, name) == 0;
Multi-line conditionals should be wrapped in ()s, and then
the && naturally aligns under "cmd":
return (cmd != NULL
&& cmd != CMD_LIST_AMBIGUOUS
&& strcmp (cmd->name, name) == 0);
(and suitably tabified).
> +}
> +
> +/* Given an input line P, skip the command and return a pointer to the
> + first argument. */
> +
> +static const char *
> +line_first_arg (const char *p)
> +{
> + const char *first_arg = p + find_command_name_length (p);
> +
> + while (first_arg != '\0' && isspace (*first_arg))
> + first_arg++;
return skip_spaces_const (first_arg);
?
> +
> + return first_arg;
> +}
> +
> /* Process one input line. If the command is an "end", return such an
> indication to the caller. If PARSE_COMMANDS is true, strip leading
> whitespace (trailing whitespace is always stripped) in the line,
> @@ -919,6 +943,8 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
> char *p_end;
> char *p_start;
> int not_handled = 0;
> + const char *cmd_name = p;
> + struct cmd_list_element *cmd;
Move these to the scope that needs them?
>
> /* Not sure what to do here. */
> if (p == NULL)
> @@ -938,9 +964,11 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
> We also permit whitespace before end and after. */
> if (p_end - p_start == 3 && startswith (p_start, "end"))
> return end_command;
> -
> +
> if (parse_commands)
> {
> + cmd = lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1);
It'd be good to add a comment about why we do this instead of
just doing a strcmp. Mention something about aliases and
unique command abbreviations. Please tweak the subject
line / commit log too; I initially thought this was going
to be about tab completion. :-)
> +
> /* If commands are parsed, we skip initial spaces. Otherwise,
> which is the case for Python commands and documentation
> (see the 'document' command), spaces are preserved. */
> @@ -958,9 +986,7 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
>
> /* Check for while, if, break, continue, etc and build a new
> command line structure for them. */
> - if ((p_end - p >= 14 && startswith (p, "while-stepping"))
> - || (p_end - p >= 8 && startswith (p, "stepping"))
> - || (p_end - p >= 2 && startswith (p, "ws")))
> + if (command_name_equals (cmd, "while-stepping"))
> {
> /* Because validate_actionline and encode_action lookup
> command's line as command, we need the line to
> @@ -975,40 +1001,25 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
> not. */
> *command = build_command_line (while_stepping_control, p);
> }
> - else if (p_end - p > 5 && startswith (p, "while"))
> + else if (command_name_equals (cmd, "while"))
> {
> - char *first_arg;
> -
> - first_arg = p + 5;
> - while (first_arg < p_end && isspace (*first_arg))
> - first_arg++;
> - *command = build_command_line (while_control, first_arg);
> + *command = build_command_line (while_control, line_first_arg (p));
> }
> - else if (p_end - p > 2 && startswith (p, "if"))
> + else if (command_name_equals (cmd, "if"))
> {
> - char *first_arg;
> -
> - first_arg = p + 2;
> - while (first_arg < p_end && isspace (*first_arg))
> - first_arg++;
> - *command = build_command_line (if_control, first_arg);
> + *command = build_command_line (if_control, line_first_arg (p));
> }
> - else if (p_end - p >= 8 && startswith (p, "commands"))
> + else if (command_name_equals (cmd, "commands"))
> {
> - char *first_arg;
> -
> - first_arg = p + 8;
> - while (first_arg < p_end && isspace (*first_arg))
> - first_arg++;
> - *command = build_command_line (commands_control, first_arg);
> + *command = build_command_line (commands_control, line_first_arg (p));
> }
> - else if (p_end - p == 6 && startswith (p, "python"))
> + else if (command_name_equals (cmd, "python"))
> {
> /* Note that we ignore the inline "python command" form
> here. */
> *command = build_command_line (python_control, "");
> }
> - else if (p_end - p == 6 && startswith (p, "compile"))
> + else if (command_name_equals (cmd, "compile"))
> {
> /* Note that we ignore the inline "compile command" form
> here. */
> @@ -1016,7 +1027,7 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
> (*command)->control_u.compile.scope = COMPILE_I_INVALID_SCOPE;
> }
>
> - else if (p_end - p == 5 && startswith (p, "guile"))
> + else if (command_name_equals (cmd, "guile"))
> {
> /* Note that we ignore the inline "guile command" form here. */
> *command = build_command_line (guile_control, "");
> diff --git a/gdb/testsuite/gdb.base/define.exp b/gdb/testsuite/gdb.base/define.exp
> index 59203ec..b57b068 100644
> --- a/gdb/testsuite/gdb.base/define.exp
> +++ b/gdb/testsuite/gdb.base/define.exp
> @@ -147,6 +147,30 @@ gdb_test_multiple "define ifnospace" "define user command: ifnospace" \
>
> gdb_test "ifnospace" ".*hi there.*" "test ifnospace is parsed correctly"
>
> +# Verify that the command parser properly handle completion.
Here too.
> +set test "define use command: breakmain"
typo: "user" ?
> +gdb_test_multiple "define breakmain" "$test" \
> +{
> + -re "Type commands for definition of \"breakmain\".\r\nEnd with a line saying just \"end\".\r\n>$" \
> + {
> + pass "$test"
> +
> + set test "send body of breakmain"
> + gdb_test_multiple "break main\ncommand\necho\nend\nend" "$test" \
> + {
> + -re "$gdb_prompt $"\
> + {pass "$test"}
Does this fail with an unfixed GDB? I ask since this is just matching the
prompt, which a "bad" GDB also outputs?
> + }
> + }
> +}
> +
> +gdb_test "breakmain" ".*Breakpoint 2.*" "test command completion in define"
Odd message for this test. I suggest wrapping these three
tests in
with_test_prefix "command abbreviations in define" {
set test "define user command: breakmain"
gdb_test_multiple "define breakmain" "$test" .....
gdb_test "breakmain" ".*Breakpoint 2.*" "run user command"
gdb_test "info break 2" \
.... \
"info break shows echo command"
}
> +
> +gdb_test "info break 2" \
> + "Num Type\[ \]+Disp Enb Address\[ \]+What.*
> +\[0-9\]+\[\t \]+breakpoint keep y.* in main at .*
> +\[\t \]+echo.*"
> +
It's better to avoid hardcoding breakpoint numbers, since
it'll break if someone adds some test before this one
that creates some other breakpoint.
You can use "$bpnum" instead to avoid that.
> # Verify that the command parser doesn't require a space after an 'while'
> # command in a user defined function.
> #
>
Thanks,
Pedro Alves
commit 05bd4353619a49e8506bc6f8cd428b6a0961b28f
Author: Jerome Guitton <guitton@adacore.com>
Date: Tue Jan 10 15:15:53 2017 +0100
[RFA] completion in command definition
When defining a new macro, "command" is not recognized as an aliased for
"commands":
(gdb) define breakmain
Type commands for definition of "breakmain".
End with a line saying just "end".
>break main
>command
>echo "IN MAIN\n"
>end
(gdb)
There is a special case for while-stepping, where 'ws' and 'stepping' are
recognized explicitely. We could add "command" to the list as well. I'd
rather use cli-decode though. Patch in attachment, with a test, tested
in x86-linux. OK to apply?
gdb/ChangeLog:
* cli/cli-decode.c (find_command_name_length): Make it global.
* cli/cli-decode.h (find_command_name_length): Declare.
* cli/cli-script.c (command_name_equals, line_first_arg):
New functions.
(process_next_line): Use cli-decode to parse command names
gdb/testsuite/ChangeLog:
* gdb.base/define.exp: Add test for completion in command definition.
@@ -1255,7 +1255,7 @@ find_cmd (const char *command, int len, struct cmd_list_element *clist,
return found;
}
-static int
+int
find_command_name_length (const char *text)
{
const char *p = text;
@@ -253,4 +253,6 @@ extern const char * const auto_boolean_enums[];
extern int cli_user_command_p (struct cmd_list_element *);
+extern int find_command_name_length (const char *);
+
#endif /* !defined (CLI_DECODE_H) */
@@ -143,7 +143,7 @@ multi_line_command_p (enum command_control_type type)
control commands (if/while). */
static struct command_line *
-build_command_line (enum command_control_type type, char *args)
+build_command_line (enum command_control_type type, const char *args)
{
struct command_line *cmd;
@@ -904,6 +904,30 @@ read_next_line (void)
return command_line_input (prompt_ptr, from_tty, "commands");
}
+/* Return non-zero if CMD's name is NAME. */
+
+static bool
+command_name_equals (struct cmd_list_element *cmd, const char *name)
+{
+ return cmd != NULL
+ && cmd != CMD_LIST_AMBIGUOUS
+ && strcmp (cmd->name, name) == 0;
+}
+
+/* Given an input line P, skip the command and return a pointer to the
+ first argument. */
+
+static const char *
+line_first_arg (const char *p)
+{
+ const char *first_arg = p + find_command_name_length (p);
+
+ while (first_arg != '\0' && isspace (*first_arg))
+ first_arg++;
+
+ return first_arg;
+}
+
/* Process one input line. If the command is an "end", return such an
indication to the caller. If PARSE_COMMANDS is true, strip leading
whitespace (trailing whitespace is always stripped) in the line,
@@ -919,6 +943,8 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
char *p_end;
char *p_start;
int not_handled = 0;
+ const char *cmd_name = p;
+ struct cmd_list_element *cmd;
/* Not sure what to do here. */
if (p == NULL)
@@ -938,9 +964,11 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
We also permit whitespace before end and after. */
if (p_end - p_start == 3 && startswith (p_start, "end"))
return end_command;
-
+
if (parse_commands)
{
+ cmd = lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1);
+
/* If commands are parsed, we skip initial spaces. Otherwise,
which is the case for Python commands and documentation
(see the 'document' command), spaces are preserved. */
@@ -958,9 +986,7 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
/* Check for while, if, break, continue, etc and build a new
command line structure for them. */
- if ((p_end - p >= 14 && startswith (p, "while-stepping"))
- || (p_end - p >= 8 && startswith (p, "stepping"))
- || (p_end - p >= 2 && startswith (p, "ws")))
+ if (command_name_equals (cmd, "while-stepping"))
{
/* Because validate_actionline and encode_action lookup
command's line as command, we need the line to
@@ -975,40 +1001,25 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
not. */
*command = build_command_line (while_stepping_control, p);
}
- else if (p_end - p > 5 && startswith (p, "while"))
+ else if (command_name_equals (cmd, "while"))
{
- char *first_arg;
-
- first_arg = p + 5;
- while (first_arg < p_end && isspace (*first_arg))
- first_arg++;
- *command = build_command_line (while_control, first_arg);
+ *command = build_command_line (while_control, line_first_arg (p));
}
- else if (p_end - p > 2 && startswith (p, "if"))
+ else if (command_name_equals (cmd, "if"))
{
- char *first_arg;
-
- first_arg = p + 2;
- while (first_arg < p_end && isspace (*first_arg))
- first_arg++;
- *command = build_command_line (if_control, first_arg);
+ *command = build_command_line (if_control, line_first_arg (p));
}
- else if (p_end - p >= 8 && startswith (p, "commands"))
+ else if (command_name_equals (cmd, "commands"))
{
- char *first_arg;
-
- first_arg = p + 8;
- while (first_arg < p_end && isspace (*first_arg))
- first_arg++;
- *command = build_command_line (commands_control, first_arg);
+ *command = build_command_line (commands_control, line_first_arg (p));
}
- else if (p_end - p == 6 && startswith (p, "python"))
+ else if (command_name_equals (cmd, "python"))
{
/* Note that we ignore the inline "python command" form
here. */
*command = build_command_line (python_control, "");
}
- else if (p_end - p == 6 && startswith (p, "compile"))
+ else if (command_name_equals (cmd, "compile"))
{
/* Note that we ignore the inline "compile command" form
here. */
@@ -1016,7 +1027,7 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
(*command)->control_u.compile.scope = COMPILE_I_INVALID_SCOPE;
}
- else if (p_end - p == 5 && startswith (p, "guile"))
+ else if (command_name_equals (cmd, "guile"))
{
/* Note that we ignore the inline "guile command" form here. */
*command = build_command_line (guile_control, "");
@@ -147,6 +147,30 @@ gdb_test_multiple "define ifnospace" "define user command: ifnospace" \
gdb_test "ifnospace" ".*hi there.*" "test ifnospace is parsed correctly"
+# Verify that the command parser properly handle completion.
+set test "define use command: breakmain"
+gdb_test_multiple "define breakmain" "$test" \
+{
+ -re "Type commands for definition of \"breakmain\".\r\nEnd with a line saying just \"end\".\r\n>$" \
+ {
+ pass "$test"
+
+ set test "send body of breakmain"
+ gdb_test_multiple "break main\ncommand\necho\nend\nend" "$test" \
+ {
+ -re "$gdb_prompt $"\
+ {pass "$test"}
+ }
+ }
+}
+
+gdb_test "breakmain" ".*Breakpoint 2.*" "test command completion in define"
+
+gdb_test "info break 2" \
+ "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+breakpoint keep y.* in main at .*
+\[\t \]+echo.*"
+
# Verify that the command parser doesn't require a space after an 'while'
# command in a user defined function.
#