[PATCH] [gdb/tui] Fix centering and highlighting of current line

[PATCH] [gdb/tui] Fix centering and highlighting of current line

[Tom de Vries]

After starting TUI like this with a hello world a.out:
...
$ gdb -q a.out -ex start -ex "tui enable"
...
we get:
...
┌─hello.c──────────────────────────────┐
│        5 {                           │
│        6   printf ("hello\n");       │
│        7                             │
│        8   return 0;                 │
│        9 }                           │
│                                      │
└──────────────────────────────────────┘
...

This is a regression since commit ee1e9bbb513 ("[gdb/tui] Fix displaying main
after resizing"), before which we had instead:
...
┌─hello.c──────────────────────────────┐
│        4 main (void)                 │
│        5 {                           │
│  >     6 ^[[7m  printf ("hello\n");^[[0m       │
│        7                             │
│        8   return 0;                 │
│        9 }                           │
└──────────────────────────────────────┘
...

In other words, the problems are:
- the active line (source line 6) is no longer highlighted, and
- the active line is not veritically centered (screen line 2 out 6 instead of
  screen line 3 out of 6).

Fix these problems respectively by:
- in tui_enable, instead of "tui_show_frame_info (0)" using
  'tui_show_frame_info (deprecated_safe_get_selected_frame ())", and
- in tui_source_window_base::rerender, adding centering functionality.

Tested on aarch64-linux.

Co-Authored-By: Tom Tromey <tromey@adacore.com>

PR tui/31522
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31522
---
 gdb/testsuite/gdb.tui/main-2.exp | 47 ++++++++++++++++++++++++++++++++
 gdb/tui/tui-winsource.c          | 10 +++++++
 gdb/tui/tui.c                    |  2 +-
 3 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.tui/main-2.exp

diff --git a/gdb/testsuite/gdb.tui/main-2.exp b/gdb/testsuite/gdb.tui/main-2.exp
new file mode 100644
index 00000000000..1dcb3cfc6d4
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/main-2.exp
@@ -0,0 +1,47 @@
+# Copyright 2024 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that enabling tui while having a current frame results in
+# centering and highlighting the associated line.
+
+require allow_tui_tests
+
+tuiterm_env
+
+standard_testfile tui-layout.c
+
+if { [build_executable "failed to prepare" $testfile $srcfile ] == -1} {
+    return -1
+}
+
+Term::clean_restart 24 80 $binfile
+
+if {![runto_main]} {
+    perror "test suppressed"
+    return
+}
+
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+set line "  return 0;"
+set nr [gdb_get_line_number $line]
+
+set screen_line [Term::get_line_with_attrs 6]
+verbose -log "screen line 6: '$screen_line'"
+gdb_assert { [regexp "$nr <reverse:1>$line<reverse:0>" $screen_line] } \
+    "highlighted line in middle of source window"
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 4dbbe922256..61c8e00589d 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -467,6 +467,16 @@ tui_source_window_base::rerender ()
       struct symtab *s = find_pc_line_symtab (get_frame_pc (frame));
       if (this != TUI_SRC_WIN)
 	find_line_pc (s, cursal.line, &cursal.pc);
+
+      /* This centering code is copied from tui_source_window::maybe_update.
+	 It would be nice to do centering more often, and do it in just one
+	 location.  But since this is a regression fix, handle this
+	 conservatively for now.  */
+      int start_line = (cursal.line - ((height - box_size ()) / 2)) + 1;
+      if (start_line <= 0)
+	start_line = 1;
+      cursal.line = start_line;
+
       update_source_window (gdbarch, cursal);
     }
   else
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 19f09609a2f..eaee85f82b4 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -466,7 +466,7 @@ tui_enable (void)
       tui_set_term_width_to (COLS);
       def_prog_mode ();
 
-      tui_show_frame_info (0);
+      tui_show_frame_info (deprecated_safe_get_selected_frame ());
       tui_set_initial_layout ();
       tui_set_win_focus_to (TUI_SRC_WIN);
       keypad (TUI_CMD_WIN->handle.get (), TRUE);

base-commit: 4ef6173d2dfeafd33deedf7ce0d384cfbcf1170d
-- 
2.35.3

Re: [PATCH] [gdb/tui] Fix centering and highlighting of current line

[Pedro Alves]

On 2024-03-28 14:33, Tom de Vries wrote:

> Tested on aarch64-linux.
> 
> Co-Authored-By: Tom Tromey <tromey@adacore.com>
> 
> PR tui/31522
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31522

Thank you.  As I said in the bug, this works for me.

Also ran the new test along all the other gdb.tui/ tests, and saw no regressions.

Tested-By: Pedro Alves <pedro@palves.net>

Pedro Alves

Re: [PATCH] [gdb/tui] Fix centering and highlighting of current line

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Fix these problems respectively by:
Tom> - in tui_enable, instead of "tui_show_frame_info (0)" using
Tom>   'tui_show_frame_info (deprecated_safe_get_selected_frame ())", and
Tom> - in tui_source_window_base::rerender, adding centering functionality.

Thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom> +      /* This centering code is copied from tui_source_window::maybe_update.
Tom> +	 It would be nice to do centering more often, and do it in just one
Tom> +	 location.  But since this is a regression fix, handle this
Tom> +	 conservatively for now.  */

Yeah, unfortunately some of this code remains spaghetti from the old
days.

Tom