diff mbox series

[gdb/tui] Use more box_width in tui-regs.c

Message ID 20231215113752.29375-1-tdevries@suse.de
State Committed
Headers
Series [gdb/tui] Use more box_width in tui-regs.c |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom de Vries Dec. 15, 2023, 11:37 a.m. UTC 
  While experimenting with can_box () == false by default, I noticed two places
in tui-regs.c where we can replace a hardcoded 1 with box_width ().

It also turned out to be necessary to set scrollok to false, otherwise writing
the last char of the last line with register info will cause a scroll.

Tested on x86_64-linux.
---
 gdb/tui/tui-regs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


base-commit: 8cb16b68584e14aade8de166c75e1d85e38507bd

Comments

Tom Tromey Dec. 15, 2023, 3:32 p.m. UTC | #1 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom>    /* Now create each data "sub" window, and write the display into
Tom>       it.  */
Tom> -  int cur_y = 1;
Tom> +  int cur_y = box_width ();

It is sort of weird to see the pairing of "y" and "width", but I suppose
there isn't a "box_height" method.

Anyway it seems fine.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
Tom de Vries Dec. 16, 2023, 9:27 a.m. UTC | #2 
On 12/15/23 16:32, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom>    /* Now create each data "sub" window, and write the display into
> Tom>       it.  */
> Tom> -  int cur_y = 1;
> Tom> +  int cur_y = box_width ();
> 
> It is sort of weird to see the pairing of "y" and "width", but I suppose
> there isn't a "box_height" method.
> 

Yeah, I may have confused width and thickness when choosing that name.

Anyway, looking at one of the PRs you just filed, we may go to a 
hborder/vborder (horizontal/vertical) approach [ or even a 
tborder/bborder/lborder/rborder (top/bottom/left/right) approach ] and I 
suppose that could be a good moment to rectify this misnomer.

> Anyway it seems fine.
> Approved-By: Tom Tromey <tom@tromey.com>

Thanks for the review.

Committed.

Thanks,
- Tom
Tom Tromey Dec. 18, 2023, 2:16 p.m. UTC | #3 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Anyway, looking at one of the PRs you just filed, we may go to a
Tom> hborder/vborder (horizontal/vertical) approach [ or even a
Tom> tborder/bborder/lborder/rborder (top/bottom/left/right) approach ] and
Tom> I suppose that could be a good moment to rectify this misnomer.

If anyone ever implements it, yeah.
I wonder if windows should just have their "inner" size and position and
then the TUI itself draw the boxes; instead of the current model where
the box is part of the window.

Tom
diff mbox series

Patch

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 4c6ea8aff0d..ba801ddc050 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -288,7 +288,7 @@  tui_data_window::display_registers_from (int start_element_no)
 
   /* Now create each data "sub" window, and write the display into
      it.  */
-  int cur_y = 1;
+  int cur_y = box_width ();
   while (i < m_regs_content.size () && cur_y <= height - box_size ())
     {
       for (int j = 0;
@@ -296,7 +296,7 @@  tui_data_window::display_registers_from (int start_element_no)
 	   j++)
 	{
 	  /* Create the window if necessary.  */
-	  m_regs_content[i].x = (m_item_width * j) + 1;
+	  m_regs_content[i].x = box_width () + (m_item_width * j);
 	  m_regs_content[i].y = cur_y;
 	  m_regs_content[i].visible = true;
 	  m_regs_content[i].rerender (handle.get (), m_item_width);
@@ -487,6 +487,10 @@  tui_data_window::check_register_values (frame_info_ptr frame)
 void
 tui_data_item_window::rerender (WINDOW *handle, int field_width)
 {
+  /* In case the regs window is not boxed, we'll write the last char in the
+     last line here, causing a scroll, so prevent that.  */
+  scrollok (handle, FALSE);
+
   if (highlight)
     /* We ignore the return value, casting it to void in order to avoid
        a compiler warning.  The warning itself was introduced by a patch