diff mbox

[2/2] gdb: Add completer for layout command.

Message ID 18d55b2f3d6b65fd7e1fef8374fbadf429e95095.1428011044.git.andrew.burgess@embecosm.com
State Dropped
Headers show

Commit Message

Andrew Burgess April 7, 2015, 8:08 a.m. UTC 
Add layout name completion for the layout command.  Some extra effort is
required as some layout names use '$' which is by default a word
breaking character.

gdb/ChangeLog:

	* completer.c (set_gdb_completion_word_break_characters_string):
	New function.
	(set_gdb_completion_word_break_characters): Use new function.
	* completer.h (set_gdb_completion_word_break_characters_string):
	Declare.
	* tui/tui-layout.c: Add 'completer.h' include.
	(layout_completer): New function.
	(layout_brkchars): New function.
	(_initialize_tui_layout): Set completer and brkchars on layout
	command.

gdb/testsuite/ChangeLog:

	* gdb.base/completion.exp: Add test for completion of layout
	names.
---
 gdb/ChangeLog                         | 13 +++++++++
 gdb/completer.c                       | 18 +++++++++---
 gdb/completer.h                       |  4 +++
 gdb/testsuite/ChangeLog               |  5 ++++
 gdb/testsuite/gdb.base/completion.exp | 18 ++++++++++++
 gdb/tui/tui-layout.c                  | 53 +++++++++++++++++++++++++++++++++--
 6 files changed, 105 insertions(+), 6 deletions(-)

Comments

Pedro Alves May 19, 2015, 1:38 p.m. UTC | #1 
Hi Andrew,

Sorry for the delay.

( Adding Keith, who's recently posted a series that touches
all completers.  You guys sort it out in what order the
patches should go in.  :-) )

On 04/07/2015 09:08 AM, Andrew Burgess wrote:
> Add layout name completion for the layout command.  Some extra effort is
> required as some layout names use '$' which is by default a word
> breaking character.
> 
>  }
> +
> +gdb_test_no_output "set max-completions unlimited"
> +
> +if {![skip_tui_tests]} {
> +    set test "test completion of layout names"
> +    send_gdb "layout\t\t\t"
> +    gdb_test_multiple "" "$test" {
> +	-re "\\\$fregs  \\\$gregs  \\\$regs   \\\$sregs  asm     next    prev    regs    split   src *\r\n$gdb_prompt layout $" {
> +	    pass "$test"
> +	}
> +    }
> +    send_gdb "\003"
> +    gdb_test_multiple "" "$test" {
> +	-re "$gdb_prompt $" {
> +	    pass "quit command input after testing layout completion"
> +	}
> +    }

I think this should be:

    set test "quit command input after testing layout completion"
    gdb_test_multiple "" "$test" {
	-re "$gdb_prompt $" {
	    pass $test
	}
    }

so that timeout or internal error caught inside gdb_test_multiple
use that test's message, instead of the previous'.

> +
> +static void
> +layout_brkchars (struct cmd_list_element *command,
> +		 const char *text, const char *word)
> +{
> +  /* This set of characters should be the same set returned by
> +     DEFAULT_WORD_BREAK_CHARACTERS but with '$' removed.  */
> +  set_gdb_completion_word_break_characters_string
> +    (" \t\n!@#%^&*()+=|~`}{[]\"';:?/>.<,");

How about instead doing it like f_word_break_characters does?
That is, do what the comment says, at run time.

Thanks,
Pedro Alves
Andrew Burgess May 20, 2015, 11:22 p.m. UTC | #2 
* Pedro Alves <palves@redhat.com> [2015-05-19 14:38:21 +0100]:

> Sorry for the delay.

No problem.  Thanks for the review.

>
> I think this should be:
>
>     set test "quit command input after testing layout completion"
>     gdb_test_multiple "" "$test" {
> 	-re "$gdb_prompt $" {
> 	    pass $test
> 	}
>     }
>
> so that timeout or internal error caught inside gdb_test_multiple
> use that test's message, instead of the previous'.

I have now posted a new patch series that replaces this patch.  I've
addressed your review comments about the testing in my new patch.  As
a bonus the new completion part of the new series is simpler and so
should not clash with Keith's work.

Thanks,
Andrew
Keith Seitz May 21, 2015, 12:19 a.m. UTC | #3 
On 05/20/2015 04:22 PM, Andrew Burgess wrote:
> As a bonus the new completion part of the new series is simpler and
> so should not clash with Keith's work.

I wouldn't worry too much about collisions anyway. My patch is so large,
it will likely take considerable time to be reviewed.

Even so, the change to any completer is fairly trivial. Add a parameter
and use add_completion, returning if it is a certain value. Your layout
completer would be trivial to convert.

Thank you for keeping an eye out, though. I've been through a lot of
rebasing nightmares in the last few months as I try to keep two large
patchsets in shape.

Keith
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index febc377..3507097 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@ 
+2015-03-26  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* completer.c (set_gdb_completion_word_break_characters_string):
+	New function.
+	(set_gdb_completion_word_break_characters): Use new function.
+	* completer.h (set_gdb_completion_word_break_characters_string):
+	Declare.
+	* tui/tui-layout.c: Add 'completer.h' include.
+	(layout_completer): New function.
+	(layout_brkchars): New function.
+	(_initialize_tui_layout): Set completer and brkchars on layout
+	command.
+
 2015-03-25  Pedro Alves  <palves@redhat.com>
 
 	* target.h <to_async>: Replace 'callback' and 'context' parameters
diff --git a/gdb/completer.c b/gdb/completer.c
index c8c0e4c..8b1fa41 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -457,16 +457,26 @@  expression_completer (struct cmd_list_element *ignore,
 /* See definition in completer.h.  */
 
 void
+set_gdb_completion_word_break_characters_string (char *c)
+{
+  rl_completer_word_break_characters = c;
+}
+
+/* See definition in completer.h.  */
+
+void
 set_gdb_completion_word_break_characters (completer_ftype *fn)
 {
   /* So far we are only interested in differentiating filename
      completers from everything else.  */
+  char *c;
+
   if (fn == filename_completer)
-    rl_completer_word_break_characters
-      = gdb_completer_file_name_break_characters;
+    c = gdb_completer_file_name_break_characters;
   else
-    rl_completer_word_break_characters
-      = gdb_completer_command_word_break_characters;
+    c = gdb_completer_command_word_break_characters;
+
+  set_gdb_completion_word_break_characters_string (c);
 }
 
 /* Here are some useful test cases for completion.  FIXME: These
diff --git a/gdb/completer.h b/gdb/completer.h
index 56e1a2b..5e62828 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -107,6 +107,10 @@  extern char *gdb_completion_word_break_characters (void);
 
 extern void set_gdb_completion_word_break_characters (completer_ftype *fn);
 
+/* Set the word break characters array to the set STR.  */
+
+extern void set_gdb_completion_word_break_characters_string (char *STR);
+
 /* Exported to linespec.c */
 
 extern const char *skip_quoted_chars (const char *, const char *,
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6abb62f..ee9fc77 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@ 
 2015-03-27  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.base/completion.exp: Add test for completion of layout
+	names.
+
+2015-03-27  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* lib/gdb.exp (skip_tui_tests): New proc.
 	* gdb.base/tui-layout.exp: Check skip_tui_tests.
 
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index f77bfe2..43a4e1e 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -859,3 +859,21 @@  gdb_test_multiple "" "$test" {
 	pass "$test"
     }
 }
+
+gdb_test_no_output "set max-completions unlimited"
+
+if {![skip_tui_tests]} {
+    set test "test completion of layout names"
+    send_gdb "layout\t\t\t"
+    gdb_test_multiple "" "$test" {
+	-re "\\\$fregs  \\\$gregs  \\\$regs   \\\$sregs  asm     next    prev    regs    split   src *\r\n$gdb_prompt layout $" {
+	    pass "$test"
+	}
+    }
+    send_gdb "\003"
+    gdb_test_multiple "" "$test" {
+	-re "$gdb_prompt $" {
+	    pass "quit command input after testing layout completion"
+	}
+    }
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 6547404..e05418f 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -25,6 +25,7 @@ 
 #include "symtab.h"
 #include "frame.h"
 #include "source.h"
+#include "completer.h"
 #include <ctype.h>
 
 #include "tui/tui.h"
@@ -373,6 +374,50 @@  tui_default_win_viewport_height (enum tui_win_type type,
   return h;
 }
 
+/* Complete possible layout names.  TEXT is the complete text entered so
+   far, WORD is the word currently being completed.  */
+
+static VEC (char_ptr) *
+layout_completer (struct cmd_list_element *ignore,
+		  const char *text, const char *word)
+{
+  VEC (char_ptr) *return_val = NULL;
+  const char **tmp, *p;
+  size_t len;
+
+  static const char *layout_names [] =
+    { "src", "asm", "split", "regs", "next", "prev",
+      TUI_FLOAT_REGS_NAME_LOWER,
+      TUI_GENERAL_REGS_NAME_LOWER,
+      TUI_SPECIAL_REGS_NAME_LOWER,
+      TUI_GENERAL_SPECIAL_REGS_NAME_LOWER,
+      NULL };
+
+  gdb_assert (text != NULL);
+  p = strchr (text, ' ');
+  len = (p == NULL) ? strlen (text) : p - text;
+
+  for (tmp = layout_names; *tmp != NULL; ++tmp)
+    {
+      if (strncmp (text, *tmp, len) == 0)
+	{
+	  char *str = xstrdup (*tmp);
+	  VEC_safe_push (char_ptr, return_val, str);
+	}
+    }
+
+  return return_val;
+}
+
+static void
+layout_brkchars (struct cmd_list_element *command,
+		 const char *text, const char *word)
+{
+  /* This set of characters should be the same set returned by
+     DEFAULT_WORD_BREAK_CHARACTERS but with '$' removed.  */
+  set_gdb_completion_word_break_characters_string
+    (" \t\n!@#%^&*()+=|~`}{[]\"';:?/>.<,");
+}
 
 /* Function to initialize gdb commands, for tui window layout
    manipulation.  */
@@ -383,8 +428,10 @@  extern initialize_file_ftype _initialize_tui_layout;
 void
 _initialize_tui_layout (void)
 {
-  add_com ("layout", class_tui, tui_layout_command, _("\
-Change the layout of windows.\n\
+  struct cmd_list_element *cmd;
+
+  cmd = add_com ("layout", class_tui, tui_layout_command, _("\
+Change the layout of windows.\n				     \
 Usage: layout prev | next | <layout_name> \n\
 Layout names are:\n\
    src   : Displays source and command windows.\n\
@@ -396,6 +443,8 @@  Layout names are:\n\
            source/assembly/command (split) is displayed, \n\
            the register window is displayed with \n\
            the window that has current logical focus.\n"));
+  set_cmd_completer (cmd, layout_completer);
+  set_cmd_completer_handle_brkchars (cmd, layout_brkchars);
   if (xdb_commands)
     {
       add_com ("td", class_tui, tui_toggle_layout_command, _("\