[PATCH] gdb/tui: Add command completion to winheight command.

[PATCH] gdb/tui: Add command completion to winheight command.

[Andrew Burgess]

Share the window name completion code from the focus command with the
winheight command, providing window name completion for the winheight
command.

gdb/ChangeLog:

	* tui/tui-win.c (window_name_completer): New function.
	(focus_completer): Call window_name_completer.  All old content
	moved into window_name_completer.
	(winheight_completer): New function.
	(_initialize_tui_win): Rename variable.  Add completer to
	winheight command.  Update doc string on winheight.
---
 gdb/ChangeLog     |  9 ++++++
 gdb/tui/tui-win.c | 91 ++++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 72 insertions(+), 28 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8ab1330..8f69009 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2015-07-10  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* tui/tui-win.c (window_name_completer): New function.
+	(focus_completer): Call window_name_completer.  All old content
+	moved into window_name_completer.
+	(winheight_completer): New function.
+	(_initialize_tui_win): Rename variable.  Add completer to
+	winheight command.  Update doc string on winheight.
+
+2015-07-10  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* tui/tui-win.c (tui_set_win_height): Use a cleanup to free the
 	string copy.
 	(parse_scrolling_args): Likewise.
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index cdf322b..aa22205 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -354,45 +354,53 @@ tui_set_var_cmd (char *null_args, int from_tty, struct cmd_list_element *c)
     tui_rehighlight_all ();
 }
 
-/* Complete possible window names to focus on.  TEXT is the complete text
-   entered so far, WORD is the word currently being completed.  */
+/* Generic window name completion function.  Complete window name pointed
+   too by TEXT and WORD.  If INCLUDE_NEXT_PREV_P is true then the special
+   window names 'next' and 'prev' are also included in the list of possible
+   completions (if appropriate).  */
 
 static VEC (char_ptr) *
-focus_completer (struct cmd_list_element *ignore,
-		  const char *text, const char *word)
+window_name_completer (int include_next_prev_p,
+		       const char *text, const char *word)
 {
   VEC (const_char_ptr) *completion_name_vec = NULL;
   VEC (char_ptr) *matches_vec;
-  int win_type;
 
-  for (win_type = SRC_WIN; win_type < MAX_MAJOR_WINDOWS; win_type++)
+  if (tui_active)
     {
-      const char *completion_name = NULL;
+      int win_type;
 
-      /* We can't focus on an invisible window.  */
-      if (tui_win_list[win_type] == NULL
-	  || !tui_win_list[win_type]->generic.is_visible)
-	continue;
+      for (win_type = SRC_WIN; win_type < MAX_MAJOR_WINDOWS; win_type++)
+	{
+	  const char *completion_name = NULL;
 
-      completion_name = tui_win_name (&tui_win_list [win_type]->generic);
-      gdb_assert (completion_name != NULL);
-      VEC_safe_push (const_char_ptr, completion_name_vec, completion_name);
-    }
+	  /* We can't focus on an invisible window.  */
+	  if (tui_win_list[win_type] == NULL
+	      || !tui_win_list[win_type]->generic.is_visible)
+	    continue;
 
-  /* If no windows are considered visible then the TUI has not yet been
-     initialized.  But still "focus src" and "focus cmd" will work because
-     invoking the focus command will entail initializing the TUI which sets the
-     default layout to SRC_COMMAND.  */
-  if (VEC_length (const_char_ptr, completion_name_vec) == 0)
+	  completion_name = tui_win_name (&tui_win_list [win_type]->generic);
+	  gdb_assert (completion_name != NULL);
+	  VEC_safe_push (const_char_ptr, completion_name_vec, completion_name);
+	}
+    }
+  else
     {
+      /* If the tui is not yet active then we should still offer up the two
+	 initial windows 'src' and 'cmd'.  All tui commands will
+	 auto-activate the tui, which means these windows are valid for
+	 use.  */
       VEC_safe_push (const_char_ptr, completion_name_vec, SRC_NAME);
       VEC_safe_push (const_char_ptr, completion_name_vec, CMD_NAME);
     }
 
-  VEC_safe_push (const_char_ptr, completion_name_vec, "next");
-  VEC_safe_push (const_char_ptr, completion_name_vec, "prev");
-  VEC_safe_push (const_char_ptr, completion_name_vec, NULL);
+  if (include_next_prev_p)
+    {
+      VEC_safe_push (const_char_ptr, completion_name_vec, "next");
+      VEC_safe_push (const_char_ptr, completion_name_vec, "prev");
+    }
 
+  VEC_safe_push (const_char_ptr, completion_name_vec, NULL);
   matches_vec
     = complete_on_enum (VEC_address (const_char_ptr, completion_name_vec),
 			text, word);
@@ -402,6 +410,32 @@ focus_completer (struct cmd_list_element *ignore,
   return matches_vec;
 }
 
+/* Complete possible window names to focus on.  TEXT is the complete text
+   entered so far, WORD is the word currently being completed.  */
+
+static VEC (char_ptr) *
+focus_completer (struct cmd_list_element *ignore,
+		  const char *text, const char *word)
+{
+  return window_name_completer (1, text, word);
+}
+
+/* Complete possible window names for winheight command.  TEXT is the
+   complete text entered so far, WORD is the word currently being
+   completed.  */
+
+static VEC (char_ptr) *
+winheight_completer (struct cmd_list_element *ignore,
+		     const char *text, const char *word)
+{
+  /* The first word is the window name.  That we can complete.  Subsequent
+     words can't be completed.  */
+  if (word != text)
+    return NULL;
+
+  return window_name_completer (0, text, word);
+}
+
 /* Function to initialize gdb commands, for tui window
    manipulation.  */
 
@@ -413,7 +447,7 @@ _initialize_tui_win (void)
 {
   static struct cmd_list_element *tui_setlist;
   static struct cmd_list_element *tui_showlist;
-  struct cmd_list_element *focus_cmd;
+  struct cmd_list_element *cmd;
 
   /* Define the classes of commands.
      They will appear in the help list in the reverse of this order.  */
@@ -431,8 +465,8 @@ _initialize_tui_win (void)
   add_com ("tabset", class_tui, tui_set_tab_width_command, _("\
 Set the width (in characters) of tab stops.\n\
 Usage: tabset <n>\n"));
-  add_com ("winheight", class_tui, tui_set_win_height_command, _("\
-Set the height of a specified window.\n\
+  cmd = add_com ("winheight", class_tui, tui_set_win_height_command, _("\
+Set or modify the height of a specified window.\n\
 Usage: winheight <win_name> [+ | -] <#lines>\n\
 Window names are:\n\
 src  : the source window\n\
@@ -440,9 +474,10 @@ cmd  : the command window\n\
 asm  : the disassembly window\n\
 regs : the register display\n"));
   add_com_alias ("wh", "winheight", class_tui, 0);
+  set_cmd_completer (cmd, winheight_completer);
   add_info ("win", tui_all_windows_info,
 	    _("List of all displayed windows.\n"));
-  focus_cmd = add_com ("focus", class_tui, tui_set_focus_command, _("\
+  cmd = add_com ("focus", class_tui, tui_set_focus_command, _("\
 Set focus to named window or next/prev window.\n\
 Usage: focus {<win> | next | prev}\n\
 Valid Window names are:\n\
@@ -451,7 +486,7 @@ asm  : the disassembly window\n\
 regs : the register display\n\
 cmd  : the command window\n"));
   add_com_alias ("fs", "focus", class_tui, 0);
-  set_cmd_completer (focus_cmd, focus_completer);
+  set_cmd_completer (cmd, focus_completer);
   add_com ("+", class_tui, tui_scroll_forward_command, _("\
 Scroll window forward.\n\
 Usage: + [win] [n]\n"));
-- 
2.4.0

Re: [PATCH] gdb/tui: Add command completion to winheight command.

[Pedro Alves]

On 07/10/2015 02:13 PM, Andrew Burgess wrote:
> Share the window name completion code from the focus command with the
> winheight command, providing window name completion for the winheight
> command.

Thanks.

har *null_args, int from_tty, struct cmd_list_element *c)
>      tui_rehighlight_all ();
>  }
>  
> -/* Complete possible window names to focus on.  TEXT is the complete text
> -   entered so far, WORD is the word currently being completed.  */
> +/* Generic window name completion function.  Complete window name pointed
> +   too by TEXT and WORD.  If INCLUDE_NEXT_PREV_P is true then the special

"pointed to" ?

> +   window names 'next' and 'prev' are also included in the list of possible
> +   completions (if appropriate).  */
>  
>  static VEC (char_ptr) *
> -focus_completer (struct cmd_list_element *ignore,
> -		  const char *text, const char *word)
> +window_name_completer (int include_next_prev_p,
> +		       const char *text, const char *word)
>  {
>    VEC (const_char_ptr) *completion_name_vec = NULL;
>    VEC (char_ptr) *matches_vec;
> -  int win_type;
>  
> -  for (win_type = SRC_WIN; win_type < MAX_MAJOR_WINDOWS; win_type++)
> +  if (tui_active)
>      {

...

> -  /* If no windows are considered visible then the TUI has not yet been
> -     initialized.  But still "focus src" and "focus cmd" will work because
> -     invoking the focus command will entail initializing the TUI which sets the
> -     default layout to SRC_COMMAND.  */
> -  if (VEC_length (const_char_ptr, completion_name_vec) == 0)
> +	  completion_name = tui_win_name (&tui_win_list [win_type]->generic);
> +	  gdb_assert (completion_name != NULL);
> +	  VEC_safe_push (const_char_ptr, completion_name_vec, completion_name);
> +	}
> +    }
> +  else
>      {
> +      /* If the tui is not yet active then we should still offer up the two
> +	 initial windows 'src' and 'cmd'.  All tui commands will
> +	 auto-activate the tui, which means these windows are valid for
> +	 use.  */

This changed the predicate used to get here, and it doesn't look right
to me.  It used to be "not initialized yet" (no window marked visible,
no matter whether the tui is active), now it's "tui active".  The former
meant that because tui had not been initialized, then the current layout
once the tui is initialized the default layout contains the src and cmd
windows.  But with the new predicate that is not always correct.
We should be able to enable the tui, do "layout asm", disable the tui,
and then do "focus <tab>".  This should offer the asm window, not
the src window.  That works today, but I think will not work after
your patch?

Thanks,
Pedro Alves

Re: [PATCH] gdb/tui: Add command completion to winheight command.

[Andrew Burgess]

* Pedro Alves <palves@redhat.com> [2015-07-10 17:23:58 +0100]:

> On 07/10/2015 02:13 PM, Andrew Burgess wrote:
> >
> > -/* Complete possible window names to focus on.  TEXT is the complete text
> > -   entered so far, WORD is the word currently being completed.  */
> > +/* Generic window name completion function.  Complete window name pointed
> > +   too by TEXT and WORD.  If INCLUDE_NEXT_PREV_P is true then the special
> 
> "pointed to" ?

Fixed.

> > +   window names 'next' and 'prev' are also included in the list of possible
> > +   completions (if appropriate).  */
> >  
> >  static VEC (char_ptr) *
> > -focus_completer (struct cmd_list_element *ignore,
> > -		  const char *text, const char *word)
> > +window_name_completer (int include_next_prev_p,
> > +		       const char *text, const char *word)
> >  {
> >    VEC (const_char_ptr) *completion_name_vec = NULL;
> >    VEC (char_ptr) *matches_vec;
> > -  int win_type;
> >  
> > -  for (win_type = SRC_WIN; win_type < MAX_MAJOR_WINDOWS; win_type++)
> > +  if (tui_active)
> >      {
> 
> ...
> 
> > -  /* If no windows are considered visible then the TUI has not yet been
> > -     initialized.  But still "focus src" and "focus cmd" will work because
> > -     invoking the focus command will entail initializing the TUI which sets the
> > -     default layout to SRC_COMMAND.  */
> > -  if (VEC_length (const_char_ptr, completion_name_vec) == 0)
> > +	  completion_name = tui_win_name (&tui_win_list [win_type]->generic);
> > +	  gdb_assert (completion_name != NULL);
> > +	  VEC_safe_push (const_char_ptr, completion_name_vec, completion_name);
> > +	}
> > +    }
> > +  else
> >      {
> > +      /* If the tui is not yet active then we should still offer up the two
> > +	 initial windows 'src' and 'cmd'.  All tui commands will
> > +	 auto-activate the tui, which means these windows are valid for
> > +	 use.  */
> 
> This changed the predicate used to get here, and it doesn't look right
> to me.  It used to be "not initialized yet" (no window marked visible,
> no matter whether the tui is active), now it's "tui active".  The former
> meant that because tui had not been initialized, then the current layout
> once the tui is initialized the default layout contains the src and cmd
> windows.  But with the new predicate that is not always correct.
> We should be able to enable the tui, do "layout asm", disable the tui,
> and then do "focus <tab>".  This should offer the asm window, not
> the src window.  That works today, but I think will not work after
> your patch?

Thanks, I had not appreciated this.  Fixed in the new patch.

Andrew

---

gdb/tui: Add command completion to winheight command.

Share the window name completion code from the focus command with the
winheight command, providing window name completion for the winheight
command.

gdb/ChangeLog:

	* tui/tui-win.c (window_name_completer): New function.
	(focus_completer): Call window_name_completer.  All old content
	moved into window_name_completer.
	(winheight_completer): New function.
	(_initialize_tui_win): Rename variable.  Add completer to
	winheight command.  Update doc string on winheight.

---

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8ab1330..8f69009 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2015-07-10  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* tui/tui-win.c (window_name_completer): New function.
+	(focus_completer): Call window_name_completer.  All old content
+	moved into window_name_completer.
+	(winheight_completer): New function.
+	(_initialize_tui_win): Rename variable.  Add completer to
+	winheight command.  Update doc string on winheight.
+
+2015-07-10  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* tui/tui-win.c (tui_set_win_height): Use a cleanup to free the
 	string copy.
 	(parse_scrolling_args): Likewise.
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index cdf322b..7248f8b 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -354,12 +354,14 @@ tui_set_var_cmd (char *null_args, int from_tty, struct cmd_list_element *c)
     tui_rehighlight_all ();
 }
 
-/* Complete possible window names to focus on.  TEXT is the complete text
-   entered so far, WORD is the word currently being completed.  */
+/* Generic window name completion function.  Complete window name pointed
+   to by TEXT and WORD.  If INCLUDE_NEXT_PREV_P is true then the special
+   window names 'next' and 'prev' are also included in the list of possible
+   completions (if appropriate).  */
 
 static VEC (char_ptr) *
-focus_completer (struct cmd_list_element *ignore,
-		  const char *text, const char *word)
+window_name_completer (int include_next_prev_p,
+		       const char *text, const char *word)
 {
   VEC (const_char_ptr) *completion_name_vec = NULL;
   VEC (char_ptr) *matches_vec;
@@ -389,10 +391,13 @@ focus_completer (struct cmd_list_element *ignore,
       VEC_safe_push (const_char_ptr, completion_name_vec, CMD_NAME);
     }
 
-  VEC_safe_push (const_char_ptr, completion_name_vec, "next");
-  VEC_safe_push (const_char_ptr, completion_name_vec, "prev");
-  VEC_safe_push (const_char_ptr, completion_name_vec, NULL);
+  if (include_next_prev_p)
+    {
+      VEC_safe_push (const_char_ptr, completion_name_vec, "next");
+      VEC_safe_push (const_char_ptr, completion_name_vec, "prev");
+    }
 
+  VEC_safe_push (const_char_ptr, completion_name_vec, NULL);
   matches_vec
     = complete_on_enum (VEC_address (const_char_ptr, completion_name_vec),
 			text, word);
@@ -402,6 +407,32 @@ focus_completer (struct cmd_list_element *ignore,
   return matches_vec;
 }
 
+/* Complete possible window names to focus on.  TEXT is the complete text
+   entered so far, WORD is the word currently being completed.  */
+
+static VEC (char_ptr) *
+focus_completer (struct cmd_list_element *ignore,
+		  const char *text, const char *word)
+{
+  return window_name_completer (1, text, word);
+}
+
+/* Complete possible window names for winheight command.  TEXT is the
+   complete text entered so far, WORD is the word currently being
+   completed.  */
+
+static VEC (char_ptr) *
+winheight_completer (struct cmd_list_element *ignore,
+		     const char *text, const char *word)
+{
+  /* The first word is the window name.  That we can complete.  Subsequent
+     words can't be completed.  */
+  if (word != text)
+    return NULL;
+
+  return window_name_completer (0, text, word);
+}
+
 /* Function to initialize gdb commands, for tui window
    manipulation.  */
 
@@ -413,7 +444,7 @@ _initialize_tui_win (void)
 {
   static struct cmd_list_element *tui_setlist;
   static struct cmd_list_element *tui_showlist;
-  struct cmd_list_element *focus_cmd;
+  struct cmd_list_element *cmd;
 
   /* Define the classes of commands.
      They will appear in the help list in the reverse of this order.  */
@@ -431,8 +462,8 @@ _initialize_tui_win (void)
   add_com ("tabset", class_tui, tui_set_tab_width_command, _("\
 Set the width (in characters) of tab stops.\n\
 Usage: tabset <n>\n"));
-  add_com ("winheight", class_tui, tui_set_win_height_command, _("\
-Set the height of a specified window.\n\
+  cmd = add_com ("winheight", class_tui, tui_set_win_height_command, _("\
+Set or modify the height of a specified window.\n\
 Usage: winheight <win_name> [+ | -] <#lines>\n\
 Window names are:\n\
 src  : the source window\n\
@@ -440,9 +471,10 @@ cmd  : the command window\n\
 asm  : the disassembly window\n\
 regs : the register display\n"));
   add_com_alias ("wh", "winheight", class_tui, 0);
+  set_cmd_completer (cmd, winheight_completer);
   add_info ("win", tui_all_windows_info,
 	    _("List of all displayed windows.\n"));
-  focus_cmd = add_com ("focus", class_tui, tui_set_focus_command, _("\
+  cmd = add_com ("focus", class_tui, tui_set_focus_command, _("\
 Set focus to named window or next/prev window.\n\
 Usage: focus {<win> | next | prev}\n\
 Valid Window names are:\n\
@@ -451,7 +483,7 @@ asm  : the disassembly window\n\
 regs : the register display\n\
 cmd  : the command window\n"));
   add_com_alias ("fs", "focus", class_tui, 0);
-  set_cmd_completer (focus_cmd, focus_completer);
+  set_cmd_completer (cmd, focus_completer);
   add_com ("+", class_tui, tui_scroll_forward_command, _("\
 Scroll window forward.\n\
 Usage: + [win] [n]\n"));

Re: [PATCH] gdb/tui: Add command completion to winheight command.

[Pedro Alves]

On 07/12/2015 09:51 AM, Andrew Burgess wrote:

> gdb/ChangeLog:
> 
> 	* tui/tui-win.c (window_name_completer): New function.
> 	(focus_completer): Call window_name_completer.  All old content
> 	moved into window_name_completer.
> 	(winheight_completer): New function.
> 	(_initialize_tui_win): Rename variable.  Add completer to
> 	winheight command.  Update doc string on winheight.
> 

This is OK.

> -/* Complete possible window names to focus on.  TEXT is the complete text
> -   entered so far, WORD is the word currently being completed.  */
> +/* Generic window name completion function.  Complete window name pointed
> +   to by TEXT and WORD.  If INCLUDE_NEXT_PREV_P is true then the special
> +   window names 'next' and 'prev' are also included in the list of possible
> +   completions (if appropriate).  */
>  

I don't really understand what "if appropriate" is referring to, though.

Thanks,
Pedro Alves

Re: [PATCH] gdb/tui: Add command completion to winheight command.

[Andrew Burgess]

* Pedro Alves <palves@redhat.com> [2015-07-12 15:58:59 +0100]:

> On 07/12/2015 09:51 AM, Andrew Burgess wrote:
> 
> > gdb/ChangeLog:
> > 
> > 	* tui/tui-win.c (window_name_completer): New function.
> > 	(focus_completer): Call window_name_completer.  All old content
> > 	moved into window_name_completer.
> > 	(winheight_completer): New function.
> > 	(_initialize_tui_win): Rename variable.  Add completer to
> > 	winheight command.  Update doc string on winheight.
> > 
> 
> This is OK.
> 
> > -/* Complete possible window names to focus on.  TEXT is the complete text
> > -   entered so far, WORD is the word currently being completed.  */
> > +/* Generic window name completion function.  Complete window name pointed
> > +   to by TEXT and WORD.  If INCLUDE_NEXT_PREV_P is true then the special
> > +   window names 'next' and 'prev' are also included in the list of possible
> > +   completions (if appropriate).  */
> >  
> 
> I don't really understand what "if appropriate" is referring to, though.

I originally wrote "... are also included in the list of possible
completions." however, this is not true, if a window name has been
partially typed then clearly 'prev' or 'next' might not be included in
the list of possible completions (if say the partial window name
started with a 'c').

How about this wording:

"If INCLUDE_NEXT_PREV_P is true then the special window names 'next'
 and 'prev' will also be considered as possible completions of the
 window name."

Or feel free to suggest something simpler.

Thanks,
Andrew

Re: [PATCH] gdb/tui: Add command completion to winheight command.

[Pedro Alves]

On 07/12/2015 10:34 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2015-07-12 15:58:59 +0100]:

>> I don't really understand what "if appropriate" is referring to, though.
> 
> I originally wrote "... are also included in the list of possible
> completions." however, this is not true, if a window name has been
> partially typed then clearly 'prev' or 'next' might not be included in
> the list of possible completions (if say the partial window name
> started with a 'c').
> 
> How about this wording:
> 
> "If INCLUDE_NEXT_PREV_P is true then the special window names 'next'
>  and 'prev' will also be considered as possible completions of the
>  window name."
> 

I like that.

> Or feel free to suggest something simpler.

Thanks,
Pedro Alves