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

Simon Marchi
2020-09-23T19:10:09Z

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