gdb/tui: Add command completion to winheight command.

  * 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.

---
  

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
  

* 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
  

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