[RFA] completion in command definition

Message ID 20170131142910.GA22056@adacore.com
State New, archived
Headers

Commit Message

Jerome Guitton Jan. 31, 2017, 2:29 p.m. UTC
  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,
Jerome
  

Comments

Pedro Alves Jan. 31, 2017, 3:09 p.m. UTC | #1
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
  

Patch

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