Re: [PATCH] Don't let TUI focus on locator

[Simon Marchi <simark@simark.ca>]

On 2020-09-23 2:59 p.m., Tom Tromey wrote:
> PR tui/26638 notes that the C-x o binding can put the focus on the
> locator window.  However, this is not useful and did not happen
> historically.  This patch changes the TUI to skip this window when
> switching focus.
>
> gdb/ChangeLog
> 2020-09-23  Tom Tromey  <tromey@adacore.com>
>
> 	PR tui/26638:
> 	* tui/tui-data.c (tui_next_win): Exclude the locator.
> 	(tui_prev_win): Rewrite.
> ---
>  gdb/ChangeLog      |  6 ++++++
>  gdb/tui/tui-data.c | 35 +++++++++++++++++++++++++++--------
>  2 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
> index 8f7d257e945..0d2b2219439 100644
> --- a/gdb/tui/tui-data.c
> +++ b/gdb/tui/tui-data.c
> @@ -23,6 +23,7 @@
>  #include "symtab.h"
>  #include "tui/tui.h"
>  #include "tui/tui-data.h"
> +#include "tui/tui-stack.h"
>  #include "tui/tui-win.h"
>  #include "tui/tui-wingeneral.h"
>  #include "tui/tui-winsource.h"
> @@ -113,9 +114,18 @@ tui_next_win (struct tui_win_info *cur_win)
>    auto iter = std::find (tui_windows.begin (), tui_windows.end (), cur_win);
>    gdb_assert (iter != tui_windows.end ());
>
> -  ++iter;
> -  if (iter == tui_windows.end ())
> -    return tui_windows[0];
> +  gdb_assert (cur_win != tui_locator_win_info_ptr ());
> +  /* This won't loop forever since we can't have just a locator
> +     window.  */
> +  while (true)
> +    {
> +      ++iter;
> +      if (iter == tui_windows.end ())
> +	iter = tui_windows.begin ();
> +      if (*iter != tui_locator_win_info_ptr ())
> +	break;
> +    }
> +
>    return *iter;
>  }
>
> @@ -125,12 +135,21 @@ tui_next_win (struct tui_win_info *cur_win)
>  struct tui_win_info *
>  tui_prev_win (struct tui_win_info *cur_win)
>  {
> -  auto iter = std::find (tui_windows.begin (), tui_windows.end (), cur_win);
> -  gdb_assert (iter != tui_windows.end ());
> +  auto iter = std::find (tui_windows.rbegin (), tui_windows.rend (), cur_win);
> +  gdb_assert (iter != tui_windows.rend ());
> +
> +  gdb_assert (cur_win != tui_locator_win_info_ptr ());
> +  /* This won't loop forever since we can't have just a locator
> +     window.  */
> +  while (true)
> +    {
> +      ++iter;
> +      if (iter == tui_windows.rend ())
> +	iter = tui_windows.rbegin ();
> +      if (*iter != tui_locator_win_info_ptr ())
> +	break;
> +    }
>
> -  if (iter == tui_windows.begin ())
> -    return tui_windows.back ();
> -  --iter;
>    return *iter;
>  }

Instead of hard-coding here which windows can receive focus, how about
having a tui_win_info virtual method "is_focusable", where the default
implementation returns true, and tui_locator_window's implementation
returns false?  The code in tui_next_win / tui_prev_win would return the
next / prev focusable window.

Simon