This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] completion in command definition
- From: Simon Marchi <simon dot marchi at polymtl dot ca>
- To: Jerome Guitton <guitton at adacore dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 10 Jan 2017 11:27:55 -0500
- Subject: Re: [RFA] completion in command definition
- Authentication-results: sourceware.org; auth=none
- References: <1484058481-6378-1-git-send-email-guitton@adacore.com>
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