| Message ID | 20170112100506.GK27546@adacore.com |
|---|---|
| State | New |
| Headers | show |
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
Hi Pedro, Pedro Alves (palves@redhat.com): > OK (but please remember to edit out the question and "attachment" > from the commit log). I've integrated your comments; I can push the patch in attachment next week, if no objection. About this question: > > +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? This won't fail; the following tests ("breakmain" and "info break") will fail though. Calling breakmain would make an unfixed GDB wait for an additional "end" command, causing a timeout. I've added an additional call to "end" after "breakmain, to avoid polluting the other tests in define.exp (otherwise they would time out one after the other). Thanks, Jeromecommit de170acae27af8a9899a01f32475b7095031e46a Author: Jerome Guitton <guitton@adacore.com> Date: Tue Jan 10 15:15:53 2017 +0100 Command abbreviation in define When defining a new macro, "command" is not recognized as an alias 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. Instead of adding more special cases, this change uses cli-decode. gdb/ChangeLog: * cli/cli-decode.c (find_command_name_length): Make it 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. (build_command_line): Make args a constant pointer. gdb/testsuite/ChangeLog: * gdb.base/define.exp: Add test for command abbreviations in define. diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d3be93c..9312177 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -1255,7 +1255,9 @@ find_cmd (const char *command, int len, struct cmd_list_element *clist, return found; } -static int +/* Return the length of command name in TEXT. */ + +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 *); + #endif /* !defined (CLI_DECODE_H) */ diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 6f44d52..9615037 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) { struct command_line *cmd; @@ -904,6 +904,27 @@ 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); + + return skip_spaces_const (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, @@ -938,9 +959,14 @@ 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) { + /* Resolve command abbreviations (e.g. 'ws' for 'while-stepping'). */ + const char *cmd_name = p; + struct cmd_list_element *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 +984,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 +999,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 +1025,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..91aadd7 100644 --- a/gdb/testsuite/gdb.base/define.exp +++ b/gdb/testsuite/gdb.base/define.exp @@ -147,6 +147,34 @@ 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 handles command abbreviations. +with_test_prefix "command abbreviations in define" { + set test "define user 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 .*" "run user command" + + # If GDB fails to interpret properly the abbrev "command", the last "end" + # will be missing. Issue it to avoid a desync that would break the other + # tests in this file. + gdb_test "end" ".*This command cannot be used at the top level.*" "additional end command" + + gdb_test "info break \$bpnum" \ + "Num Type\[ \]+Disp Enb Address\[ \]+What.* +\[0-9\]+\[\t \]+breakpoint keep y.* in main at .* +\[\t \]+echo.*" "info break shows echo command" +} + + # Verify that the command parser doesn't require a space after an 'while' # command in a user defined function. #
On 01/31/2017 02:29 PM, Jerome Guitton wrote: > Hi Pedro, > > Pedro Alves (palves@redhat.com): >> > OK (but please remember to edit out the question and "attachment" >> > from the commit log). > I've integrated your comments; I can push the patch in attachment > next week, if no objection. A few minor comments below. > > +/* Return non-zero if CMD's name is NAME. */ s/non-zero/true/. > + > +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); > + > + return skip_spaces_const (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, > @@ -938,9 +959,14 @@ 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) > { > + /* Resolve command abbreviations (e.g. 'ws' for 'while-stepping'). */ > + const char *cmd_name = p; > + struct cmd_list_element *cmd = > + lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1); "=" goes on the next line: struct cmd_list_element *cmd = lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1); > > +# Verify that the command parser properly handles command abbreviations. > +with_test_prefix "command abbreviations in define" { > + set test "define user 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 .*" "run user command" > + > + # If GDB fails to interpret properly the abbrev "command", the last "end" > + # will be missing. Issue it to avoid a desync that would break the other > + # tests in this file. Double space after period. > + gdb_test "end" ".*This command cannot be used at the top level.*" "additional end command" Too-long line. The initial ".*" in "This command" is not necessary; it's implicit. > + > + gdb_test "info break \$bpnum" \ > + "Num Type\[ \]+Disp Enb Address\[ \]+What.* > +\[0-9\]+\[\t \]+breakpoint keep y.* in main at .* > +\[\t \]+echo.*" I think we need to use use multi_line, to avoid problems with "\n" vs "\r" vs "\r\n". "info break shows echo command" > +} > + > + > # Verify that the command parser doesn't require a space after an 'while' > # command in a user defined function. > # > Thanks, Pedro Alves
Pedro Alves (palves@redhat.com): > A few minor comments below. Thanks again! Comments integrated. I'm just not sure about this one: > > + > > + gdb_test "info break \$bpnum" \ > > + "Num Type\[ \]+Disp Enb Address\[ \]+What.* > > +\[0-9\]+\[\t \]+breakpoint keep y.* in main at .* > > +\[\t \]+echo.*" > > I think we need to use use multi_line, to avoid problems with > "\n" vs "\r" vs "\r\n". I've used the same code pattern as in break.exp; is that OK?commit 1f892fe4947beee9879a4e04b93ce46acf591b8b Author: Jerome Guitton <guitton@adacore.com> Date: Tue Jan 10 15:15:53 2017 +0100 Command abbreviation in define When defining a new macro, "command" is not recognized as an alias 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. Instead of adding more special cases, this change uses cli-decode. gdb/ChangeLog: * cli/cli-decode.c (find_command_name_length): Make it 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. (build_command_line): Make args a constant pointer. gdb/testsuite/ChangeLog: * gdb.base/define.exp: Add test for command abbreviations in define. diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d3be93c..9312177 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -1255,7 +1255,9 @@ find_cmd (const char *command, int len, struct cmd_list_element *clist, return found; } -static int +/* Return the length of command name in TEXT. */ + +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 *); + #endif /* !defined (CLI_DECODE_H) */ diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 6f44d52..c3f1c65 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) { struct command_line *cmd; @@ -904,6 +904,27 @@ read_next_line (void) return command_line_input (prompt_ptr, from_tty, "commands"); } +/* Return true 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); + + return skip_spaces_const (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, @@ -938,9 +959,14 @@ 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) { + /* Resolve command abbreviations (e.g. 'ws' for 'while-stepping'). */ + const char *cmd_name = p; + struct cmd_list_element *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 +984,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 +999,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 +1025,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..97c47ff 100644 --- a/gdb/testsuite/gdb.base/define.exp +++ b/gdb/testsuite/gdb.base/define.exp @@ -147,6 +147,36 @@ 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 handles command abbreviations. +with_test_prefix "command abbreviations in define" { + set test "define user 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 .*" "run user command" + + # If GDB fails to interpret properly the abbrev "command", the last "end" + # will be missing. Issue it to avoid a desync that would break the other + # tests in this file. + gdb_test "end" \ + "This command cannot be used at the top level.*" \ + "additional end command" + + gdb_test "info break \$bpnum" \ + "Num Type\[ \]+Disp Enb Address\[ \]+What.* +\[0-9\]+\[\t \]+breakpoint keep y.* in main at .* +\[\t \]+echo.*" "info break shows echo command" +} + + # Verify that the command parser doesn't require a space after an 'while' # command in a user defined function. #
On 01/31/2017 03:29 PM, Jerome Guitton wrote: > Pedro Alves (palves@redhat.com): > >> A few minor comments below. > > Thanks again! Comments integrated. I'm just not sure about this one: > >>> + >>> + gdb_test "info break \$bpnum" \ >>> + "Num Type\[ \]+Disp Enb Address\[ \]+What.* >>> +\[0-9\]+\[\t \]+breakpoint keep y.* in main at .* >>> +\[\t \]+echo.*" >> >> I think we need to use use multi_line, to avoid problems with >> "\n" vs "\r" vs "\r\n". > > I've used the same code pattern as in break.exp; is that OK? > ISTR that on some hosts (maybe native Windows testing, but not Cygwin), we can end up with tcl/expect seeing different end lines from what we normally get on Unix, due to different levels of \n -> \r\n -> \n translation going on. Might have been on macOS, due \r being preferred there. I think tcl translates end lines to \n from files by default for input, and uses native eol for output. I think the way it's written above makes the end lines in the regexp's depend on how tcl reads eol out of the script file. So if gdb actually outputs "\r" only, the regexp won't match. Or something like that. It may be working as is due to each line ending with ".*". It's messy, and a might be a bit of cargo-culting is in place (not sure how relevant this all still is, we don't hear much about native Windows testing driven by native Windows expect, for instance), but as far as I remember, we've avoided writing multi-line regexps like that. Thanks, Pedro Alves
Pedro Alves (palves@redhat.com): > It's messy, and a might be a bit of cargo-culting is in place (not > sure how relevant this all still is, we don't hear much about > native Windows testing driven by native Windows expect, for > instance), but as far as I remember, we've avoided writing > multi-line regexps like that. Fair enough! I'm convinced. Patch updated.commit afab536850cf734a30f59c3769ddb7d4f5ca5ec1 Author: Jerome Guitton <guitton@adacore.com> Date: Tue Jan 10 15:15:53 2017 +0100 Command abbreviation in define When defining a new macro, "command" is not recognized as an alias 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. Instead of adding more special cases, this change uses cli-decode. gdb/ChangeLog: * cli/cli-decode.c (find_command_name_length): Make it 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. (build_command_line): Make args a constant pointer. gdb/testsuite/ChangeLog: * gdb.base/define.exp: Add test for command abbreviations in define. diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d3be93c..9312177 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -1255,7 +1255,9 @@ find_cmd (const char *command, int len, struct cmd_list_element *clist, return found; } -static int +/* Return the length of command name in TEXT. */ + +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 *); + #endif /* !defined (CLI_DECODE_H) */ diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 6f44d52..c3f1c65 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) { struct command_line *cmd; @@ -904,6 +904,27 @@ read_next_line (void) return command_line_input (prompt_ptr, from_tty, "commands"); } +/* Return true 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); + + return skip_spaces_const (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, @@ -938,9 +959,14 @@ 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) { + /* Resolve command abbreviations (e.g. 'ws' for 'while-stepping'). */ + const char *cmd_name = p; + struct cmd_list_element *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 +984,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 +999,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 +1025,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..711ed25 100644 --- a/gdb/testsuite/gdb.base/define.exp +++ b/gdb/testsuite/gdb.base/define.exp @@ -147,6 +147,38 @@ 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 handles command abbreviations. +with_test_prefix "command abbreviations in define" { + set test "define user 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 .*" "run user command" + + # If GDB fails to interpret properly the abbrev "command", the last "end" + # will be missing. Issue it to avoid a desync that would break the other + # tests in this file. + gdb_test "end" \ + "This command cannot be used at the top level.*" \ + "additional end command" + + gdb_test "info break \$bpnum" \ + [multi_line \ + "Num Type\[ \]+Disp Enb Address\[ \]+What.*" \ + "\[0-9\]+\[\t \]+breakpoint keep y.* in main at .*" \ + "\[\t \]+echo.*"] \ + "info break shows echo command" +} + + # Verify that the command parser doesn't require a space after an 'while' # command in a user defined function. #
On 01/31/2017 04:04 PM, Jerome Guitton wrote: > Pedro Alves (palves@redhat.com): > >> It's messy, and a might be a bit of cargo-culting is in place (not >> sure how relevant this all still is, we don't hear much about >> native Windows testing driven by native Windows expect, for >> instance), but as far as I remember, we've avoided writing >> multi-line regexps like that. > > Fair enough! I'm convinced. Patch updated. > LGTM. Thanks, Pedro Alves
Pedro Alves (palves@redhat.com):
> LGTM.
Thank you! Patch committed.
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.
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 *);
+
#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)
{
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, "");
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.
+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.
#