[RFA] completion in command definition

Message ID 20170111160518.GH27546@adacore.com
State Superseded
Headers show

Commit Message

Jerome Guitton Jan. 11, 2017, 4:05 p.m.
Simon Marchi (simon.marchi@polymtl.ca):

> I can comment on styling issues (some of them nits), but not so much

> on the actual problem, since I'm not very familiar with it.  But in

> general it cleans up the code nicely I think.


Thank you for your comments! All comments taken into account in the
new patch in attachment. About this one:


> >+  struct cmd_list_element *last_line = 0;

> 

> This should be either "= NULL" or "= nullptr", but actually I don't

> think it needs to be initialized.


lookup_cmd_1 does not set this parameter when it is initialized
to NULL. However, the name of the variable was a thinko. I changed it
to "result_list".

Comments

Simon Marchi Jan. 11, 2017, 4:14 p.m. | #1
On 2017-01-11 11:05, Jerome Guitton wrote:
>> >+  struct cmd_list_element *last_line = 0;

>> 

>> This should be either "= NULL" or "= nullptr", but actually I don't

>> think it needs to be initialized.

> 

> lookup_cmd_1 does not set this parameter when it is initialized

> to NULL. However, the name of the variable was a thinko. I changed it

> to "result_list".


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);

Thanks,

Simon

Patch

commit a7783be3175ca5cceab9f3ae0cfd6f4308e57f7b
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..e9a1d78 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,9 @@  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 *result_list = NULL;
+  struct cmd_list_element *cmd;
 
   /* Not sure what to do here.  */
   if (p == NULL)
@@ -938,9 +965,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, &result_list, 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 +987,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 +1002,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 +1028,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.
 #