[PATCH 1/2] gdb/testsuite: New skip_tui_tests predicate.

[PATCH 1/2] gdb/testsuite: New skip_tui_tests predicate.

[Andrew Burgess]

Add a new predicate procedure to the gdb.exp library 'skip_tui_tests',
which returns true if the tui is not compiled into gdb.

I've made use of this predicate in the gdb.base/tui-layout.exp test as
an example.

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (skip_tui_tests): New proc.
	* gdb.base/tui-layout.exp: Check skip_tui_tests.
---
 gdb/testsuite/ChangeLog               |  5 +++++
 gdb/testsuite/gdb.base/tui-layout.exp |  7 +++++++
 gdb/testsuite/lib/gdb.exp             | 15 +++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e501b11..6abb62f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+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.
+
 2015-03-25  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* gdb.btrace/next.exp: Merged into step.exp.
diff --git a/gdb/testsuite/gdb.base/tui-layout.exp b/gdb/testsuite/gdb.base/tui-layout.exp
index 0dcf1ca..cac2bc9 100644
--- a/gdb/testsuite/gdb.base/tui-layout.exp
+++ b/gdb/testsuite/gdb.base/tui-layout.exp
@@ -19,4 +19,11 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile] } {
     return -1
 }
 
+if {[skip_tui_tests]} {
+    # TUI support is disabled.  Check for error message.
+    gdb_test "layout asm" "Undefined command: \"layout\".  Try \"help\"."
+    return
+}
+
+# Just check the command does not cause gdb to crash.
 gdb_test "layout asm"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f274b64..5a59a48 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1688,6 +1688,21 @@ proc skip_shlib_tests {} {
     return 1
 }
 
+# Return 1 if we should skip tui related tests.
+
+proc skip_tui_tests {} {
+    global gdb_prompt
+
+    gdb_test_multiple "help layout" "verify tui support" {
+	-re "Undefined command: \"layout\".*$gdb_prompt $" {
+	    return 1
+	}
+	-re "$gdb_prompt $"	{}
+    }
+
+    return 0
+}
+
 # Test files shall make sure all the test result lines in gdb.sum are
 # unique in a test run, so that comparing the gdb.sum files of two
 # test runs gives correct results.  Test files that exercise
-- 
2.2.2

[PATCH 0/2] Command completion for 'layout' command.

[Andrew Burgess]

These patches are an attempt to add command completion to the "layout"
command, completing the possible layout names.

This is made slightly harder as some of the layout names include the
'$' character, which usually counts as a word break character.

The first patch just adds some extra infrastructure to the testsuite
library to make testing the second patch easier.

The second patch adds the command completion.  The '$' issue makes the
patch messier that I'd hoped, I'd be happy to take suggestions on
possible improvements.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb/testsuite: New skip_tui_tests predicate.
  gdb: Add completer for layout command.

 gdb/ChangeLog                         | 13 +++++++++
 gdb/completer.c                       | 18 +++++++++---
 gdb/completer.h                       |  4 +++
 gdb/testsuite/ChangeLog               | 10 +++++++
 gdb/testsuite/gdb.base/completion.exp | 18 ++++++++++++
 gdb/testsuite/gdb.base/tui-layout.exp |  7 +++++
 gdb/testsuite/lib/gdb.exp             | 15 ++++++++++
 gdb/tui/tui-layout.c                  | 53 +++++++++++++++++++++++++++++++++--
 8 files changed, 132 insertions(+), 6 deletions(-)

-- 
2.2.2

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

[Andrew Burgess]

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

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, _("\
-- 
2.2.2

Re: [PATCH 1/2] gdb/testsuite: New skip_tui_tests predicate.

[Pedro Alves]

On 04/07/2015 09:08 AM, Andrew Burgess wrote:
> Add a new predicate procedure to the gdb.exp library 'skip_tui_tests',
> which returns true if the tui is not compiled into gdb.
> 
> I've made use of this predicate in the gdb.base/tui-layout.exp test as
> an example.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* lib/gdb.exp (skip_tui_tests): New proc.
> 	* gdb.base/tui-layout.exp: Check skip_tui_tests.
> ---
>  gdb/testsuite/ChangeLog               |  5 +++++
>  gdb/testsuite/gdb.base/tui-layout.exp |  7 +++++++
>  gdb/testsuite/lib/gdb.exp             | 15 +++++++++++++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index e501b11..6abb62f 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +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.
> +
>  2015-03-25  Markus Metzger  <markus.t.metzger@intel.com>
>  
>  	* gdb.btrace/next.exp: Merged into step.exp.
> diff --git a/gdb/testsuite/gdb.base/tui-layout.exp b/gdb/testsuite/gdb.base/tui-layout.exp
> index 0dcf1ca..cac2bc9 100644
> --- a/gdb/testsuite/gdb.base/tui-layout.exp
> +++ b/gdb/testsuite/gdb.base/tui-layout.exp
> @@ -19,4 +19,11 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile] } {
>      return -1
>  }
>  
> +if {[skip_tui_tests]} {
> +    # TUI support is disabled.  Check for error message.
> +    gdb_test "layout asm" "Undefined command: \"layout\".  Try \"help\"."
> +    return
> +}
> +
> +# Just check the command does not cause gdb to crash.
>  gdb_test "layout asm"
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index f274b64..5a59a48 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1688,6 +1688,21 @@ proc skip_shlib_tests {} {
>      return 1
>  }
>  
> +# Return 1 if we should skip tui related tests.
> +
> +proc skip_tui_tests {} {
> +    global gdb_prompt
> +
> +    gdb_test_multiple "help layout" "verify tui support" {
> +	-re "Undefined command: \"layout\".*$gdb_prompt $" {
> +	    return 1
> +	}
> +	-re "$gdb_prompt $"	{}

Space instead of tab before {.  Please put the closing } on its
own line:

	-re "$gdb_prompt $" {
	}

OK.

Thanks,
Pedro Alves

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

[Pedro Alves]

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

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

[Andrew Burgess]

* 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

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

[Keith Seitz]

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