* [PATCH] Don't let TUI focus on locator
@ 2020-09-23 18:59 Tom Tromey
2020-09-23 19:10 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2020-09-23 18:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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;
}
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Don't let TUI focus on locator
2020-09-23 18:59 [PATCH] Don't let TUI focus on locator Tom Tromey
@ 2020-09-23 19:10 ` Simon Marchi
2020-09-23 19:50 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2020-09-23 19:10 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Don't let TUI focus on locator
2020-09-23 19:10 ` Simon Marchi
@ 2020-09-23 19:50 ` Tom Tromey
2020-09-23 19:57 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2020-09-23 19:50 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
Simon> Instead of hard-coding here which windows can receive focus, how about
Simon> having a tui_win_info virtual method "is_focusable", where the default
Simon> implementation returns true, and tui_locator_window's implementation
Simon> returns false? The code in tui_next_win / tui_prev_win would return the
Simon> next / prev focusable window.
Let me know what you think of this.
Tom
commit adac0583ca49653e98630757599eee20faca4b33
Author: Tom Tromey <tromey@adacore.com>
Date: Wed Sep 23 12:57:19 2020 -0600
Don't let TUI focus on locator
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-stack.h (struct tui_locator_window) <can_focus>: New
method.
* tui/tui-data.h (struct tui_win_info) <can_focus>: New method.
* tui/tui-data.c (tui_next_win): Exclude non-focusable windows.
(tui_prev_win): Rewrite.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 241f3e70271..96fddbcd6e7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-09-23 Tom Tromey <tromey@adacore.com>
+
+ PR tui/26638:
+ * tui/tui-stack.h (struct tui_locator_window) <can_focus>: New
+ method.
+ * tui/tui-data.h (struct tui_win_info) <can_focus>: New method.
+ * tui/tui-data.c (tui_next_win): Exclude non-focusable windows.
+ (tui_prev_win): Rewrite.
+
2020-09-23 Tom Tromey <tom@tromey.com>
PR symtab/25470:
diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index 8f7d257e945..d475d031065 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -113,9 +113,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->can_focus ());
+ /* This won't loop forever since we can't have just an un-focusable
+ window. */
+ while (true)
+ {
+ ++iter;
+ if (iter == tui_windows.end ())
+ iter = tui_windows.begin ();
+ if ((*iter)->can_focus ())
+ break;
+ }
+
return *iter;
}
@@ -125,12 +134,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->can_focus ());
+ /* This won't loop forever since we can't have just an un-focusable
+ window. */
+ while (true)
+ {
+ ++iter;
+ if (iter == tui_windows.rend ())
+ iter = tui_windows.rbegin ();
+ if ((*iter)->can_focus ())
+ break;
+ }
- if (iter == tui_windows.begin ())
- return tui_windows.back ();
- --iter;
return *iter;
}
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 5e7a12293c9..d61bfc7dff8 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -99,6 +99,12 @@ struct tui_win_info
return handle != nullptr;
}
+ /* Return true if this window can accept the focus. */
+ virtual bool can_focus () const
+ {
+ return true;
+ }
+
/* Disable output until the next call to doupdate. */
void no_refresh ()
{
diff --git a/gdb/tui/tui-stack.h b/gdb/tui/tui-stack.h
index 9ff57b1ba73..0e5916f0ce3 100644
--- a/gdb/tui/tui-stack.h
+++ b/gdb/tui/tui-stack.h
@@ -52,6 +52,11 @@ struct tui_locator_window : public tui_win_info
return false;
}
+ bool can_focus () const override
+ {
+ return false;
+ }
+
void rerender () override;
/* Update the locator, with the provided arguments.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Don't let TUI focus on locator
2020-09-23 19:50 ` Tom Tromey
@ 2020-09-23 19:57 ` Simon Marchi
2020-09-24 18:27 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2020-09-23 19:57 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2020-09-23 3:50 p.m., Tom Tromey wrote:
> Simon> Instead of hard-coding here which windows can receive focus, how about
> Simon> having a tui_win_info virtual method "is_focusable", where the default
> Simon> implementation returns true, and tui_locator_window's implementation
> Simon> returns false? The code in tui_next_win / tui_prev_win would return the
> Simon> next / prev focusable window.
>
> Let me know what you think of this.
That looks good to me, thanks!
I've never looked at the TUI tests but... how hard would it be to write
a test for this?
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Don't let TUI focus on locator
2020-09-23 19:57 ` Simon Marchi
@ 2020-09-24 18:27 ` Tom Tromey
2020-09-24 18:33 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2020-09-24 18:27 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>> Let me know what you think of this.
Simon> That looks good to me, thanks!
Simon> I've never looked at the TUI tests but... how hard would it be to write
Simon> a test for this?
I skipped it since I thought it would be hard, but I forgot that "focus"
tells us what it is doing, and so it turned out to be super easy :}
Tom
commit b551a89f51504735e9979ac885a5784e21cfecef
Author: Tom Tromey <tromey@adacore.com>
Date: Wed Sep 23 12:57:19 2020 -0600
Don't let TUI focus on locator
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-24 Tom Tromey <tromey@adacore.com>
PR tui/26638:
* tui/tui-stack.h (struct tui_locator_window) <can_focus>: New
method.
* tui/tui-data.h (struct tui_win_info) <can_focus>: New method.
* tui/tui-data.c (tui_next_win): Exclude non-focusable windows.
(tui_prev_win): Rewrite.
gdb/testsuite/ChangeLog
2020-09-24 Tom Tromey <tromey@adacore.com>
PR tui/26638:
* gdb.tui/list.exp: Check output of "focus next".
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3b851dba41e..3872a8d9cbe 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-09-24 Tom Tromey <tromey@adacore.com>
+
+ PR tui/26638:
+ * tui/tui-stack.h (struct tui_locator_window) <can_focus>: New
+ method.
+ * tui/tui-data.h (struct tui_win_info) <can_focus>: New method.
+ * tui/tui-data.c (tui_next_win): Exclude non-focusable windows.
+ (tui_prev_win): Rewrite.
+
2020-09-23 Hannes Domani <ssbssa@yahoo.de>
* nat/windows-nat.c (handle_exception): Handle 64bit breakpoints
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c85f3ed9427..41849756f97 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-09-24 Tom Tromey <tromey@adacore.com>
+
+ PR tui/26638:
+ * gdb.tui/list.exp: Check output of "focus next".
+
2020-09-23 Tom Tromey <tom@tromey.com>
* gdb.dwarf2/intbits.exp: New file.
diff --git a/gdb/testsuite/gdb.tui/list.exp b/gdb/testsuite/gdb.tui/list.exp
index 77dbd69efd8..576b33fa869 100644
--- a/gdb/testsuite/gdb.tui/list.exp
+++ b/gdb/testsuite/gdb.tui/list.exp
@@ -39,3 +39,4 @@ Term::check_contents "list main" "21 *return 0"
# The following 'focus next' must be immediately after 'list main' to
# ensure that GDB has a valid idea of what is currently focused.
Term::command "focus next"
+Term::check_contents "focus next" "Focus set to cmd window"
diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index 8f7d257e945..d475d031065 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -113,9 +113,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->can_focus ());
+ /* This won't loop forever since we can't have just an un-focusable
+ window. */
+ while (true)
+ {
+ ++iter;
+ if (iter == tui_windows.end ())
+ iter = tui_windows.begin ();
+ if ((*iter)->can_focus ())
+ break;
+ }
+
return *iter;
}
@@ -125,12 +134,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->can_focus ());
+ /* This won't loop forever since we can't have just an un-focusable
+ window. */
+ while (true)
+ {
+ ++iter;
+ if (iter == tui_windows.rend ())
+ iter = tui_windows.rbegin ();
+ if ((*iter)->can_focus ())
+ break;
+ }
- if (iter == tui_windows.begin ())
- return tui_windows.back ();
- --iter;
return *iter;
}
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 5e7a12293c9..d61bfc7dff8 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -99,6 +99,12 @@ struct tui_win_info
return handle != nullptr;
}
+ /* Return true if this window can accept the focus. */
+ virtual bool can_focus () const
+ {
+ return true;
+ }
+
/* Disable output until the next call to doupdate. */
void no_refresh ()
{
diff --git a/gdb/tui/tui-stack.h b/gdb/tui/tui-stack.h
index 9ff57b1ba73..0e5916f0ce3 100644
--- a/gdb/tui/tui-stack.h
+++ b/gdb/tui/tui-stack.h
@@ -52,6 +52,11 @@ struct tui_locator_window : public tui_win_info
return false;
}
+ bool can_focus () const override
+ {
+ return false;
+ }
+
void rerender () override;
/* Update the locator, with the provided arguments.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Don't let TUI focus on locator
2020-09-24 18:27 ` Tom Tromey
@ 2020-09-24 18:33 ` Simon Marchi
2020-09-24 19:02 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2020-09-24 18:33 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2020-09-24 2:27 p.m., Tom Tromey wrote:
>>> Let me know what you think of this.
>
> Simon> That looks good to me, thanks!
>
> Simon> I've never looked at the TUI tests but... how hard would it be to write
> Simon> a test for this?
>
> I skipped it since I thought it would be hard, but I forgot that "focus"
> tells us what it is doing, and so it turned out to be super easy :}
>
> Tom
Awesome, thanks!
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Don't let TUI focus on locator
2020-09-24 18:33 ` Simon Marchi
@ 2020-09-24 19:02 ` Tom Tromey
0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2020-09-24 19:02 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
Simon> Awesome, thanks!
Thanks. I'm going to check this in to trunk and gdb-10.
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-24 19:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23 18:59 [PATCH] Don't let TUI focus on locator Tom Tromey
2020-09-23 19:10 ` Simon Marchi
2020-09-23 19:50 ` Tom Tromey
2020-09-23 19:57 ` Simon Marchi
2020-09-24 18:27 ` Tom Tromey
2020-09-24 18:33 ` Simon Marchi
2020-09-24 19:02 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).