Re: [PATCH] Add tab completion for TUI's "focus" command

[Patrick Palka <patrick at parcs dot ath dot cx>]

On Tue, Jun 23, 2015 at 10:01 AM, Doug Evans <dje@google.com> wrote:
> On Sun, Jun 21, 2015 at 10:30 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> The implementation is pretty straightforward, though there's the
>> question of what the completion list should show when the TUI has not
>> yet been initalized.
>
> Hi.
> What does "not yet been initialized" mean here?

I mean that the user never switched to the TUI during the session, so
the windows and stuff have not yet been created.

>
>> At that point no TUI window is considered visible
>> thus the completion list will be composed of only the "next" and "prev"
>> keywords (which are always added to the completion list).  But
>> technically the commands "focus cmd" and "focus src" will work even when
>> the TUI is not yet initialized because the initialization done in the
>> meantime will make these windows visible by default.  So should the
>> entries "cmd" and "src" be added to the completion list as special
>> cases when the TUI is not yet initialized?
>>
>> gdb/ChangeLog:
>>
>>         * tui/tui-win.c (focus_completer): New static function.
>>         (_initialize_tui_win): Set the completion function of the
>>         "focus" command to focus_completer.
>>
>> gdb/testsuite/ChangeLog:
>>
>>         * gdb.base/completion.exp: Test the completion of the "focus"
>>         command.
>> ---
>>  gdb/testsuite/gdb.base/completion.exp | 18 +++++++++++
>>  gdb/tui/tui-win.c                     | 59 ++++++++++++++++++++++++++++++++++-
>>  2 files changed, 76 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
>> index 4c31bfc..5c20f26 100644
>> --- a/gdb/testsuite/gdb.base/completion.exp
>> +++ b/gdb/testsuite/gdb.base/completion.exp
>> @@ -878,3 +878,21 @@ if {![skip_tui_tests]} {
>>         }
>>      }
>>  }
>> +if {![skip_tui_tests]} {
>> +    with_test_prefix "focus command" {
>> +       set test "test completion"
>> +       send_gdb "focus\t\t\t"
>
> I think I understand the purpose of three tabs here,
> but I'm wondering what happens if we add a "focus-foo" command.
> Seems like it'd be more robust to replace the first tab with a space.

Good point.  Then we'll need to emit only 2 tabs I think.

>
>> +       gdb_test_multiple "" "$test" {
>> +           -re "next *prev *\r\n$gdb_prompt focus $" {
>> +               pass "$test"
>> +           }
>> +       }
>> +       send_gdb "\003"
>> +       set test "quit command input after testing completion"
>> +       gdb_test_multiple "" "$test" {
>> +           -re "$gdb_prompt $" {
>> +               pass "$test"
>> +           }
>> +       }
>> +    }
>> +}
>> diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
>> index 8c0685b..8860ca7 100644
>> --- a/gdb/tui/tui-win.c
>> +++ b/gdb/tui/tui-win.c
>> @@ -354,6 +354,61 @@ 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.  */
>> +
>> +static VEC (char_ptr) *
>> +focus_completer (struct cmd_list_element *ignore,
>> +                 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++)
>> +    {
>> +      const char *completion_name = NULL;
>> +
>> +      /* We can't focus on an invisible window.  */
>> +      if (tui_win_list[win_type] == NULL
>> +         || !tui_win_list[win_type]->generic.is_visible)
>> +       continue;
>> +
>> +      switch (win_type)
>> +       {
>> +       case SRC_WIN:
>> +         completion_name = "src";
>> +         break;
>> +       case DISASSEM_WIN:
>> +         completion_name = "asm";
>> +         break;
>> +       case DATA_WIN:
>> +         completion_name = "regs";
>> +         break;
>> +       case CMD_WIN:
>> +         completion_name = "cmd";
>> +         break;
>> +       default:
>> +         break;
>> +       }
>> +
>> +      if (completion_name != NULL)
>> +       VEC_safe_push (const_char_ptr, completion_name_vec, completion_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);
>> +
>> +  matches_vec
>> +    = complete_on_enum (VEC_address (const_char_ptr, completion_name_vec),
>> +                       text, word);
>> +
>> +  VEC_free (const_char_ptr, completion_name_vec);
>> +
>> +  return matches_vec;
>> +}
>> +
>>  /* Function to initialize gdb commands, for tui window
>>     manipulation.  */
>>
>> @@ -365,6 +420,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;
>>
>>    /* Define the classes of commands.
>>       They will appear in the help list in the reverse of this order.  */
>> @@ -393,7 +449,7 @@ regs : the register display\n"));
>>    add_com_alias ("wh", "winheight", class_tui, 0);
>>    add_info ("win", tui_all_windows_info,
>>             _("List of all displayed windows.\n"));
>> -  add_com ("focus", class_tui, tui_set_focus_command, _("\
>> +  focus_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\
>> @@ -402,6 +458,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);
>>    add_com ("+", class_tui, tui_scroll_forward_command, _("\
>>  Scroll window forward.\n\
>>  Usage: + [win] [n]\n"));
>> --
>> 2.4.4.410.g43ed522.dirty
>>