diff mbox series

[gdb/tui] Fix fingerprint for cmd-only layout

Message ID 20230528150559.21219-1-tdevries@suse.de
State Committed
Headers
Series [gdb/tui] Fix fingerprint for cmd-only layout |

Commit Message

Tom de Vries May 28, 2023, 3:05 p.m. UTC 
  I added a cmd-only layout:
...
(gdb) tui new-layout cmd cmd 1
...
and set it:
...
(gdb) layout cmd
...
which gave me the expect result: only the cmd window in the screen.

However, after going back to layout src:
...
(gdb) layout src
...
I got a source window with only one line in it, and the cmd window taking most
of the screen.

I traced this back to tui_set_layout, where for both the old and the new
layout the fingerprint of the cmd window in the layout is taken.  If the
fingerprint is the same, an effort will be done to preserve the command
window size.

The fingerprint is "VC" for both the old (cmd) and new (src) layouts, which
explains the behaviour.

I think this is essentially a bug in the finger print calculation, and it
should be "C" for the cmd layout.

Fix this by not adding a V or H in the fingerprint if the list size is one.

Tested on x86_64-linux.
---
 gdb/testsuite/gdb.tui/new-layout.exp | 9 +++++++++
 gdb/tui/tui-layout.c                 | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)


base-commit: 85f4cf41a852b5983ca436615a019315f6dc7301

Comments

Tom Tromey May 30, 2023, 4:16 p.m. UTC | #1 
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> The fingerprint is "VC" for both the old (cmd) and new (src) layouts, which
Tom> explains the behaviour.

Tom> I think this is essentially a bug in the finger print calculation, and it
Tom> should be "C" for the cmd layout.

Tom> Fix this by not adding a V or H in the fingerprint if the list size is one.

I think this makes sense, but the comment in tui-layout.h needs to be
updated, since it says:

     When called on a complete, top-level layout, the fingerprint will be a
     non-empty string made of 'V' and 'H' characters, followed by a single
     'C' character.  Each 'V' and 'H' represents a vertical or horizontal

... but now the prefix could be empty in this corner case.

Tom
Tom de Vries May 31, 2023, 5:41 a.m. UTC | #2 
On 5/30/23 18:16, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> The fingerprint is "VC" for both the old (cmd) and new (src) layouts, which
> Tom> explains the behaviour.
> 
> Tom> I think this is essentially a bug in the finger print calculation, and it
> Tom> should be "C" for the cmd layout.
> 
> Tom> Fix this by not adding a V or H in the fingerprint if the list size is one.
> 
> I think this makes sense, but the comment in tui-layout.h needs to be
> updated, since it says:
> 
>       When called on a complete, top-level layout, the fingerprint will be a
>       non-empty string made of 'V' and 'H' characters, followed by a single
>       'C' character.  Each 'V' and 'H' represents a vertical or horizontal
> 
> ... but now the prefix could be empty in this corner case.

Thanks for the review.

Updated the comment and committed as attached.

Thanks,
- Tom
diff mbox series

Patch

diff --git a/gdb/testsuite/gdb.tui/new-layout.exp b/gdb/testsuite/gdb.tui/new-layout.exp
index eaebac88ce1..3085cc36232 100644
--- a/gdb/testsuite/gdb.tui/new-layout.exp
+++ b/gdb/testsuite/gdb.tui/new-layout.exp
@@ -142,3 +142,12 @@  foreach_with_prefix layout $layouts {
 			   "$gdb_prompt\\s+"]
     }
 }
+
+Term::command "layout src"
+Term::command "winheight cmd 8"
+Term::check_box "before cmd_only: src box in src layout" 0 0 80 15
+
+Term::command "layout cmd_only"
+
+Term::command "layout src"
+Term::check_box "after cmd_only: src box in src layout" 0 0 80 15
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 50c568fb7d7..159445dc520 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -1101,7 +1101,7 @@  tui_layout_split::layout_fingerprint () const
   for (auto &item : m_splits)
     {
       std::string fp = item.layout->layout_fingerprint ();
-      if (!fp.empty ())
+      if (!fp.empty () && m_splits.size () != 1)
 	return std::string (m_vertical ? "V" : "H") + fp;
     }