[RFA] completion in command definition
Commit Message
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.
---
gdb/cli/cli-decode.c | 2 +-
gdb/cli/cli-decode.h | 1 +
gdb/cli/cli-script.c | 66 +++++++++++++++++++++----------------
gdb/testsuite/gdb.base/define.exp | 21 ++++++++++++
4 files changed, 61 insertions(+), 29 deletions(-)
Comments
Hi Jerome,
I can comment on styling issues (some of them nits), but not so much on
the actual problem, since I'm not very familiar with it. But in general
it cleans up the code nicely I think.
> diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
> index e5ab839..83bc34a 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -253,4 +253,5 @@ 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 *);
Add new line here?
> #endif /* !defined (CLI_DECODE_H) */
> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index 6f44d52..338d726 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -904,6 +904,28 @@ read_next_line (void)
> return command_line_input (prompt_ptr, from_tty, "commands");
> }
>
> +/* Return non-zero if CMD's name is NAME. */
> +
> +static int
> +command_name_equals (struct cmd_list_element *cmd, char *name)
This can return a bool, and the command can say true instead of
non-zero.
name can be "const char *".
> +{
> + return cmd
For null pointer checks, use "cmd != NULL" or "cmd != nullptr".
> + && 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 char *
> +line_first_arg (char *p)
The argument type and return type can be const. It will require you to
constify a few other things which the compiler will tell you.
> +{
> + char *first_arg = p + find_command_name_length (p);
Add an empty line here...
> + while (first_arg != '\0' && isspace (*first_arg))
> + first_arg++;
... and here.
> + 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 +941,9 @@ process_next_line (char *p, struct command_line
> **command, int parse_commands,
> char *p_end;
> char *p_start;
> int not_handled = 0;
> + char *cmd_name = p;
This can be constified.
> + struct cmd_list_element *last_line = 0;
This should be either "= NULL" or "= nullptr", but actually I don't
think it needs to be initialized.
> + struct cmd_list_element *cmd;
>
> /* Not sure what to do here. */
> if (p == NULL)
> @@ -938,9 +963,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 ((const char **) &cmd_name, cmdlist,
> &last_line, 1);
If cmd_name is constified, the const char cast can be removed.
> diff --git a/gdb/testsuite/gdb.base/define.exp
> b/gdb/testsuite/gdb.base/define.exp
> index 59203ec..355a7bc 100644
> --- a/gdb/testsuite/gdb.base/define.exp
> +++ b/gdb/testsuite/gdb.base/define.exp
> @@ -147,6 +147,27 @@ 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.
> +
> +gdb_test_multiple "define breakmain" "define user command: breakmain"
> \
> +{
> + -re "Type commands for definition of \"breakmain\".\r\nEnd with a
> line saying just \"end\".\r\n>$" \
> + {
> + gdb_test_multiple "break main\ncommand\necho\nend\nend" "send
> body of breakmain" \
> + {
> + -re "$gdb_prompt $"\
> + {pass "define user command: breakmain"}
> + }
> + }
> +}
I think it's preferable to use the same test name in the
gdb_test_multiple and in the pass. Usually, we define a variable with
the test name and use it for both. Also, if there are two
gdb_test_multiple, I guess we would need two "pass"? Like:
set test "define user command: breakmain"
gdb_test_multiple ... $test {
-re ... {
pass $test
set test "send body of breakmain"
gdb_test_multiple ... $test {
-re ... {
pass $test
}
}
}
}
I'm not sure about that...
Thanks!
Simon
@@ -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,5 @@ 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) */
@@ -904,6 +904,28 @@ read_next_line (void)
return command_line_input (prompt_ptr, from_tty, "commands");
}
+/* Return non-zero if CMD's name is NAME. */
+
+static int
+command_name_equals (struct cmd_list_element *cmd, char *name)
+{
+ return cmd
+ && 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 char *
+line_first_arg (char *p)
+{
+ 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 +941,9 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
char *p_end;
char *p_start;
int not_handled = 0;
+ char *cmd_name = p;
+ struct cmd_list_element *last_line = 0;
+ struct cmd_list_element *cmd;
/* Not sure what to do here. */
if (p == NULL)
@@ -938,9 +963,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 ((const char **) &cmd_name, cmdlist, &last_line, 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 +985,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 +1000,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 +1026,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,27 @@ 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.
+
+gdb_test_multiple "define breakmain" "define user command: breakmain" \
+{
+ -re "Type commands for definition of \"breakmain\".\r\nEnd with a line saying just \"end\".\r\n>$" \
+ {
+ gdb_test_multiple "break main\ncommand\necho\nend\nend" "send body of breakmain" \
+ {
+ -re "$gdb_prompt $"\
+ {pass "define user command: breakmain"}
+ }
+ }
+}
+
+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.
#