[RFA] completion in command definition

Message ID 20170112100506.GK27546@adacore.com
State New
Headers show

Commit Message

Jerome Guitton Jan. 12, 2017, 10:05 a.m.
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

Pedro Alves Jan. 19, 2017, 2:55 p.m. | #1
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
Jerome Guitton Jan. 31, 2017, 2:29 p.m. | #2
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.
 #

Pedro Alves Jan. 31, 2017, 3:09 p.m. | #3
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
Jerome Guitton Jan. 31, 2017, 3:29 p.m. | #4
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.
 #

Pedro Alves Jan. 31, 2017, 3:53 p.m. | #5
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
Jerome Guitton Jan. 31, 2017, 4:04 p.m. | #6
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.
 #

Pedro Alves Jan. 31, 2017, 4:05 p.m. | #7
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
Jerome Guitton Feb. 8, 2017, 5:59 p.m. | #8
Pedro Alves (palves@redhat.com):

> LGTM.


Thank you! Patch committed.

Patch hide | download patch | download mbox

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.
 #