public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCHv2 11/15] gdb/tui: allow cmd window to change size in tui_layout_split::apply
Date: Sun,  6 Feb 2022 14:12:49 +0000	[thread overview]
Message-ID: <32a4b0150ef950aba5e5763dd37d831bbdcd5565.1644156219.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1644156219.git.aburgess@redhat.com>

When we switch layouts we call the tui_layout_split::apply member
function to reapply the layout, and recalculate all the window sizes.

One special case is the cmd window, which we try to keep at its
existing size.

However, in some cases it is not appropriate to keep the cmd window at
its existing size.  I will describe two such cases here, in one, we
want the cmd window to reduce in size, and in the other, we want the
cmd window to grow in size.

Try these steps in a 80 columns, by 24 lines terminal:

  (gdb) tui enable
  (gdb) layout src
  (gdb) winheight cmd 20
  (gdb) layout split

You should see that the status window is missing from the new layout,
and that the cmd window has been placed over the border of the asm
window.  The 'info win' output is:

  (gdb) info win
  Name       Lines Columns Focus
  src            3      80 (has focus)
  asm            3      80
  status         1      80
  cmd           20      80

Notice that gdb has assigned 27 lines of screen space, even with the
border overlap between the src and asm windows, this is still 2 lines
too many.

The problem here is that after switching layouts, gdb has forced the
cmd window to retain its 20 line height.  Really, we want the cmd
window to reduce in height so that the src and asm windows can occupy
their minimum required space.

This commit allows this (details on how are below).  After this
commit, in the above situation, we now see the status window displayed
correctly, and the 'info win' output is:

  (gdb) info win
  Name       Lines Columns Focus
  src            3      80 (has focus)
  asm            3      80
  status         1      80
  cmd           18      80

The cmd window has been reduced in size by 2 lines so that everything
can fit on the screen.

The second example is one which was discussed in a recent commit,
consider this case (still in the 80 column, 24 line terminal):

  (gdb) tui enable
  (gdb) tui new-layout conly cmd 1
  (gdb) layout conly
  (gdb) info win
  Name       Lines Columns Focus
  cmd            8      80 (has focus)
  (gdb)

This layout only contains a cmd window, which we would expect to
occupy the entire terminal.  But instead, the cmd window only occupies
the first 8 lines, and the rest of the terminal is unused!

The reason is, again, that the cmd window is keeping its previous
size (8 lines).

After this commit things are slightly different, the 'info win' output
is now:

  (gdb) info win
  Name       Lines Columns Focus
  cmd           20      80 (has focus)

Which is a little better, but why only 20 lines?  Turns out there's
yet another bug hitting this case.  That bug will be addressed in a
later commit, so, for now, we're accepting the 20 lines.

What this commit does is modify the phase of tui_layout_split::apply
that handles any left over space.  Usually, in "Step 2", each
sub-layout has a size calculated.  As the size is an integer, then,
when all sizes are calculated we may have some space left over.

This extra space is then distributed between all the windows fairly
until all the space is used up.

When we consider windows minimum size, or fixed size windows, then it
is possible that we might try to use more space than is available,
this was our first example above.  The same code that added extra
space to the windows, can also be used to reclaim space (in the over
allocation case) to allow all windows to fit.

The problem then is the cmd window, which we often force to a fixed
size.  Inside the loop that handles the allocation of excess space, if
we find that we have tried every window, and still have space either
left to give, or we need to claim back more space, then, if the cmd
window was changed to a fixed size, we can change the cmd window back
to a non-fixed-size window, and proceed to either give, or take space
from the cmd window as needed.
---
 gdb/testsuite/gdb.tui/new-layout.exp |  9 +++
 gdb/testsuite/gdb.tui/winheight.exp  | 46 ++++++++++++++
 gdb/tui/tui-layout.c                 | 95 +++++++++++++++++++++++++---
 3 files changed, 142 insertions(+), 8 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/new-layout.exp b/gdb/testsuite/gdb.tui/new-layout.exp
index 66048e65238..0d49d033be5 100644
--- a/gdb/testsuite/gdb.tui/new-layout.exp
+++ b/gdb/testsuite/gdb.tui/new-layout.exp
@@ -124,5 +124,14 @@ foreach_with_prefix layout $layouts {
 	Term::command "winheight src + 5"
 	Term::check_box "left window box after grow" 0 0 40 15
 	Term::check_box "right window box after grow" 39 0 41 15
+    } elseif {$name == "cmd_only"} {
+	Term::check_region_contents "bottom of cmd window is blank" \
+	    0 14 80 10 "^\\s+$"
+	Term::command "info win"
+	Term::check_region_contents "info win output" \
+	    0 0 80 24 [multi_line "info win\\s+" \
+			   "Name\\s+Lines\\s+Columns\\s+Focus\\s+" \
+			   "cmd\\s+20\\s+80\\s+\\(has focus\\)\\s+" \
+			   "$gdb_prompt\\s+"]
     }
 }
diff --git a/gdb/testsuite/gdb.tui/winheight.exp b/gdb/testsuite/gdb.tui/winheight.exp
index 0d0c886304d..b541c21b825 100644
--- a/gdb/testsuite/gdb.tui/winheight.exp
+++ b/gdb/testsuite/gdb.tui/winheight.exp
@@ -50,3 +50,49 @@ Term::check_box "larger source box again" 0 0 80 15
 Term::command "layout asm"
 Term::check_box "check for asm window" 0 0 80 15
 
+
+# Check what happens when we switch from src layout to split layout.
+# The interesting thing here is that the src layout has one flexible
+# window (the src), the status window, which is of fixed size, and the
+# cmd window, which tries to retain its size when a layout changes.
+#
+# In contrast, the split layout has both a src and asm window, plus
+# the same status and cmd windows.
+#
+# Of particular interest here is the first test, where we maximise the
+# cmd window before switching to split.  This requires gdb to realise
+# that it has to shrink the cmd window, even though this is something
+# gdb usually avoids doing.
+#
+# Each test here is a size for the src window in the 'src' layout.
+# The test then switches to the 'split' layout, and calculates the
+# expected window sizes.
+foreach_with_prefix cmd_size {20 12 5} {
+    set src_size_before [expr 24 - ${cmd_size} - 1]
+    set split_size [expr (24 - ${cmd_size}) / 2]
+
+    if { $split_size < 3 } {
+	# The minimum window size is 3, so force that.
+	set src_size_after 3
+	set asm_size_after 3
+    } elseif { [expr $split_size % 2] == 0 } {
+	# The remaining space can be divided evenly between the two
+	# windows.
+	set src_size_after ${split_size}
+	set asm_size_after ${split_size}
+    } else {
+	# The space can't be divided evenly, the asm window will get
+	# the extra line.
+	set src_size_after ${split_size}
+	set asm_size_after [expr ${split_size} + 1]
+    }
+
+    Term::command "layout src"
+    Term::command "winheight cmd ${cmd_size}"
+    Term::check_box "check for src window" 0 0 80 ${src_size_before}
+
+    # Both windows should be of equal size, which will be their minimum.
+    Term::command "layout split"
+    Term::check_box "check for src window in split" 0 0 80 ${src_size_after}
+    Term::check_box "check for asm window in split" 0 [expr ${src_size_after} - 1] 80 ${asm_size_after}
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 8ba54a68fab..87097cb7166 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -765,6 +765,30 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_)
   width = width_;
   height = height_;
 
+  /* In some situations we fix the size of the cmd window.  However,
+     occasionally this turns out to be a mistake.  This struct is used to
+     hold the original information about the cmd window, so we can restore
+     it if needed.  */
+  struct old_size_info
+  {
+    /* Constructor.  */
+    old_size_info (int index_, int min_size_, int max_size_)
+      : index (index_),
+	min_size (min_size_),
+	max_size (max_size_)
+    { /* Nothing.  */ }
+
+    /* The index in m_splits where the cmd window was found.  */
+    int index;
+
+    /* The previous min/max size.  */
+    int min_size;
+    int max_size;
+  };
+
+  /* This is given a value only if we fix the size of the cmd window.  */
+  gdb::optional<old_size_info> old_cmd_info;
+
   std::vector<size_info> info (m_splits.size ());
 
   tui_debug_printf ("weights are: %s",
@@ -792,6 +816,10 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_)
 	  && m_splits[i].layout->get_name () != nullptr
 	  && strcmp (m_splits[i].layout->get_name (), "cmd") == 0)
 	{
+	  /* Save the old cmd window information, in case we need to
+	     restore it later.  */
+          old_cmd_info.emplace (i, info[i].min_size, info[i].max_size);
+
 	  /* If this layout has never been applied, then it means the
 	     user just changed the layout.  In this situation, it's
 	     desirable to keep the size of the command window the
@@ -867,22 +895,66 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_)
       tui_debug_print_size_info (info);
     }
 
+  /* If we didn't find any sub-layouts that were of a non-fixed size, but
+     we did find the cmd window, then we can consider that a sort-of
+     non-fixed size sub-layout.
+
+     The cmd window might, initially, be of a fixed size (see above), but,
+     we are willing to relax this constraint if required to correctly apply
+     this layout (see below).  */
+  if (last_index == -1 && old_cmd_info.has_value ())
+    last_index = old_cmd_info->index;
+
   /* Allocate any leftover size.  */
-  if (available_size > used_size && last_index != -1)
+  if (available_size != used_size && last_index != -1)
     {
-      /* Loop over all windows until all available space is used up.  */
+      /* Loop over all windows until the amount of used space is equal to
+	 the amount of available space.  There's an escape hatch within
+	 the loop in case we can't find any sub-layouts to resize.  */
       bool found_window_that_can_grow_p = true;
       for (int idx = last_index;
-	   available_size > used_size;
+	   available_size != used_size;
 	   idx = (idx + 1) % m_splits.size ())
 	{
-	  /* Once we have visited all of the windows, check that we did
-	     manage to allocate some more space.  This prevents us getting
-	     stuck in the loop forever if we can't allocate anything
-	     more.  */
+	  /* Every time we get back to last_index, which is where the loop
+	     started, we check to make sure that we did assign some space
+	     to a window, bringing used_size closer to available_size.
+
+	     If we didn't, but the cmd window is of a fixed size, then we
+	     can make the console window non-fixed-size, and continue
+	     around the loop, hopefully, this will allow the layout to be
+	     applied correctly.
+
+	     If we still make it around the loop without moving used_size
+	     closer to available_size, then there's nothing more we can do,
+	     and we break out of the loop.  */
 	  if (idx == last_index)
 	    {
-	      if (!found_window_that_can_grow_p)
+	      /* If the used_size is greater than the available_size then
+		 this indicates that the fixed-sized sub-layouts claimed
+		 more space than is available.  This layout is not going to
+		 work.  Our only hope at this point is to make the cmd
+		 window non-fixed-size (if possible), and hope we can
+		 shrink this enough to fit the rest of the sub-layouts in.
+
+	         Alternatively, we've made it around the loop without
+	         adjusting any window's size.  This likely means all
+	         windows have hit their min or max size.  Again, our only
+	         hope is to make the cmd window non-fixed-size, and hope
+	         this fixes all our problems.  */
+	      if (old_cmd_info.has_value ()
+		  && ((available_size < used_size)
+		      || !found_window_that_can_grow_p))
+		{
+		  info[old_cmd_info->index].min_size = old_cmd_info->min_size;
+		  info[old_cmd_info->index].max_size = old_cmd_info->max_size;
+		  tui_debug_printf
+		    ("restoring index %d (cmd) size limits, min = %d, max = %d",
+		     old_cmd_info->index, old_cmd_info->min_size,
+		     old_cmd_info->max_size);
+		  old_cmd_info.reset ();
+		}
+	      else if (!found_window_that_can_grow_p)
 		break;
 	      found_window_that_can_grow_p = false;
 	    }
@@ -894,6 +966,13 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_)
 	      info[idx].size += 1;
 	      used_size += 1;
 	    }
+	  else if (available_size < used_size
+		   && info[idx].size > info[idx].min_size)
+	    {
+	      found_window_that_can_grow_p = true;
+	      info[idx].size -= 1;
+	      used_size -= 1;
+	    }
 	}
 
       if (debug_tui)
-- 
2.25.4


  parent reply	other threads:[~2022-02-06 14:13 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-28 15:55 [PATCH 0/7] TUI command changes, including new winwidth command Andrew Burgess
2022-01-28 15:55 ` [PATCH 1/7] gdb/tui: add window width information to 'info win' output Andrew Burgess
2022-01-28 17:00   ` Eli Zaretskii
2022-02-06 13:43   ` Andrew Burgess
2022-01-28 15:55 ` [PATCH 2/7] gdb/doc: update docs for 'info win' and 'winheight' commands Andrew Burgess
2022-01-28 17:03   ` Eli Zaretskii
2022-02-06 13:43     ` Andrew Burgess
2022-01-28 15:55 ` [PATCH 3/7] gdb: move some commands into the tui namespace Andrew Burgess
2022-01-28 17:04   ` Eli Zaretskii
2022-01-28 15:55 ` [PATCH 4/7] gdb/tui: rename tui_layout_base::adjust_size to ::set_height Andrew Burgess
2022-01-28 15:55 ` [PATCH 5/7] gdb/tui: rename tui_layout_split:set_weights_from_heights Andrew Burgess
2022-01-28 15:55 ` [PATCH 6/7] gdb/testing/tui: add new functionality to tuiterm.exp Andrew Burgess
2022-01-28 15:55 ` [PATCH 7/7] gdb/tui: add new 'tui window width' command and 'winwidth' alias Andrew Burgess
2022-01-28 17:05   ` Eli Zaretskii
2022-02-06 14:12 ` [PATCHv2 00/15] TUI changes, new winwidth command and resizing changes Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 01/15] gdb: move some commands into the tui namespace Andrew Burgess
2022-02-06 15:50     ` Eli Zaretskii
2022-02-06 14:12   ` [PATCHv2 02/15] gdb/tui: rename tui_layout_base::adjust_size to ::set_height Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 03/15] gdb/tui: rename tui_layout_split:set_weights_from_heights Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 04/15] gdb/testing/tui: add new functionality to tuiterm.exp Andrew Burgess
2022-03-04 16:29     ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 05/15] gdb/tui: add new 'tui window width' command and 'winwidth' alias Andrew Burgess
2022-02-06 15:52     ` Eli Zaretskii
2022-02-09 15:33       ` Andrew Burgess
2022-02-09 17:03         ` Eli Zaretskii
2022-03-03 18:52     ` Pedro Alves
2022-02-06 14:12   ` [PATCHv2 06/15] gdb/tui: add a tui debugging flag Andrew Burgess
2022-02-06 15:53     ` Eli Zaretskii
2022-03-04 16:35     ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 07/15] gdb/tui: add left_boxed_p and right_boxed_p member functions Andrew Burgess
2022-03-04 16:37     ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 08/15] gdb/tui/testsuite: refactor new-layout.exp test Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 09/15] gdb/tui: avoid fp exception when applying layouts Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 10/15] gdb/tui: fairer distribution of excess space during apply Andrew Burgess
2022-03-04 16:42     ` Tom Tromey
2022-02-06 14:12   ` Andrew Burgess [this message]
2022-02-06 14:12   ` [PATCHv2 12/15] gdb/tui: support placing the cmd window into a horizontal layout Andrew Burgess
2022-03-04 17:17     ` Tom Tromey
2022-03-07 20:05       ` Andrew Burgess
2022-03-07 21:24         ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 13/15] gdb/testsuite: some additional tests in gdb.tui/scroll.exp Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 14/15] gdb/tui: relax restrictions on window max height and width Andrew Burgess
2022-03-04 17:20     ` Tom Tromey
2022-03-07 20:08       ` Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 15/15] gdb/tui: fair split of delta after a resize Andrew Burgess
2022-03-04 17:22     ` Tom Tromey
2022-03-07 22:07       ` Andrew Burgess
2022-03-07 23:42         ` Tom Tromey
2022-02-21 17:29   ` [PATCHv2 00/15] TUI changes, new winwidth command and resizing changes Andrew Burgess
2022-03-02 17:59     ` Andrew Burgess
2022-03-07 22:13   ` [PATCHv3 " Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 01/15] gdb: move some commands into the tui namespace Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 02/15] gdb/tui: rename tui_layout_base::adjust_size to ::set_height Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 03/15] gdb/tui: rename tui_layout_split:set_weights_from_heights Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 04/15] gdb/testing/tui: add new functionality to tuiterm.exp Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 05/15] gdb/tui: add new 'tui window width' command and 'winwidth' alias Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 06/15] gdb/tui: add a tui debugging flag Andrew Burgess
2022-03-08 12:16       ` Eli Zaretskii
2022-03-09 11:48         ` Andrew Burgess
2022-03-09 12:58           ` Eli Zaretskii
2022-03-09 17:53           ` Tom Tromey
2022-03-07 22:13     ` [PATCHv3 07/15] gdb/tui: add left_boxed_p and right_boxed_p member functions Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 08/15] gdb/tui/testsuite: refactor new-layout.exp test Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 09/15] gdb/tui: avoid fp exception when applying layouts Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 10/15] gdb/tui: fairer distribution of excess space during apply Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 11/15] gdb/tui: allow cmd window to change size in tui_layout_split::apply Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 12/15] gdb/tui: support placing the cmd window into a horizontal layout Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 13/15] gdb/testsuite: some additional tests in gdb.tui/scroll.exp Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 14/15] gdb/tui: relax restrictions on window max height and width Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 15/15] gdb/tui: fair split of delta after a resize Andrew Burgess
2022-03-21 17:52     ` [PATCHv3 00/15] TUI changes, new winwidth command and resizing changes Andrew Burgess
2022-03-30  9:13       ` Andrew Burgess
2022-04-03 14:43         ` Andrew Burgess
2022-03-04 17:23 ` [PATCH 0/7] TUI command changes, including new winwidth command Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32a4b0150ef950aba5e5763dd37d831bbdcd5565.1644156219.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).