[v2,gdb/tui] Fix tui compact-source

Message ID 20230509181206.28857-1-tdevries@suse.de
State Committed
Headers
Series [v2,gdb/tui] Fix tui compact-source |

Commit Message

Tom de Vries May 9, 2023, 6:12 p.m. UTC
  Consider a hello.c, with less than 10 lines:
...
$ wc -l hello.c
8 hello.c
...
and compiled with -g into an a.out.

With compact-source off:
...
$ gdb -q a.out \
    -ex "set tui border-kind ascii" \
    -ex "maint set tui-left-margin-verbose on" \
    -ex "set tui compact-source off" \
    -ex "tui enable"
...
we get:
...
+-./data/hello.c-----------------------+
|___000005_{                           |
|___000006_  printf ("hello\n");       |
|___000007_  return 0;                 |
|___000008_}                           |
|___000009_                            |
|___000010_                            |
|___000011_                            |
...
but with compact-source on:
...
+-./data/hello.c-----------------------+
|___5{                                 |
|___6  printf ("hello\n");             |
|___7  return 0;                       |
|___8}                                 |
|___9                                  |
|___1                                  |
|___1                                  |
...

There are a couple of problems with compact-source.

First of all the documentation mentions:
...
The default display uses more space for line numbers and starts the
source text at the next tab stop; the compact display uses only as
much space as is needed for the line numbers in the current file, and
only a single space to separate the line numbers from the source.
...

The bit about the default display and the next tab stop looks incorrect.  The
source doesn't start at a tab stop, instead it uses a single space to separate
the line numbers from the source.

Then the documentation mentions that there's single space in the compact
display, but evidently that's missing.

Then there's the fact that the line numbers "10" and "11" are both abbreviated
to "1" in the compact case.

The abbreviation is due to allocating space for <lines in source>, which is
8 for this example, and takes a single digit.  The line numbers though
continue past the end of the file, so fix this by allocating space for
max (<lines in source>, <last line in window>), which in this example takes 2
digits.

The missing space is due to some confusion about what the "1" here in
tui_source_window::set_contents represent:
...
      double l = log10 ((double) offsets->size ());
      m_digits = 1 + (int) l;
...

It could be the trailing space that's mentioned in tui-source.h:
...
  /* How many digits to use when formatting the line number.  This
     includes the trailing space.  */
  int m_digits;
...

Then again, it could be part of the calculation for the number of digits
needed for printing.  With this minimal example:
...
int main () {
  for (int i = 8; i <= 11; ++i) {
    double l = log10 ((double) i);
    printf ("%d %d\n", i, (int)l);
  }
  return 0;
}
...
we get:
...
$ ./a.out
8 0
9 0
10 1
11 1
...
which shows that the number of digits needed for printing i is
"1 + (int)log10 ((double) i)".

Fix this by introducing named variables needed_digits and trailing_space, each
adding 1.

With the fixes, we get for compact-source on:
...
+-./data/hello.c-----------------------+
|___05_{                               |
|___06_  printf ("hello\n");           |
|___07_  return 0;                     |
|___08_}                               |
|___09_                                |
|___10_                                |
|___11_                                |
|...

Also fix the documentation and help text to actually match effect of
compact-source.

Tested on x86_64-linux.
---
 gdb/doc/gdb.texinfo                      |  7 ++-
 gdb/testsuite/gdb.tui/compact-source.exp | 61 ++++++++++++++++++++++++
 gdb/tui/tui-source.c                     |  8 +++-
 gdb/tui/tui-win.c                        |  3 +-
 4 files changed, 71 insertions(+), 8 deletions(-)
 create mode 100644 gdb/testsuite/gdb.tui/compact-source.exp


base-commit: d9cc4b060dd23724e1acf974aed3d1b72c8459e5
  

Comments

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

Tom> Consider a hello.c, with less than 10 lines:
...

Tom> Fix this by introducing named variables needed_digits and trailing_space, each
Tom> adding 1.

Thank you.  This looks good to me.

Tom
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8c4177c1901..4ae84ec5a63 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -30311,10 +30311,9 @@  assembly windows.
 @item set tui compact-source @r{[}on@r{|}off@r{]}
 @kindex set tui compact-source
 Set whether the TUI source window is displayed in ``compact'' form.
-The default display uses more space for line numbers and starts the
-source text at the next tab stop; the compact display uses only as
-much space as is needed for the line numbers in the current file, and
-only a single space to separate the line numbers from the source.
+The default display uses more space for line numbers; the compact
+display uses only as much space as is needed for the line numbers in
+the current file.
 
 @kindex set debug tui
 @item set debug tui @r{[}on|off@r{]}
diff --git a/gdb/testsuite/gdb.tui/compact-source.exp b/gdb/testsuite/gdb.tui/compact-source.exp
new file mode 100644
index 00000000000..71e6b7b0b0a
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/compact-source.exp
@@ -0,0 +1,61 @@ 
+# Copyright 2023 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/>.
+
+# Check that "set tui compact-source on" has the intended effect.
+
+require allow_tui_tests
+
+tuiterm_env
+
+standard_testfile
+
+# Let's generate the source file.  We want a short file, with less than 10
+# lines, and the copyright notice by itself is already more that that.
+set src_txt \
+    [join \
+	 [list \
+	      "int" \
+	      "main (void)" \
+	      "{" \
+	      "  return 0;" \
+	      "}"] "\n"]
+set srcfile [standard_output_file $srcfile]
+set fd [open $srcfile w]
+puts $fd $src_txt
+close $fd
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+    return -1
+}
+
+Term::clean_restart 17 80 $binfile
+
+gdb_test_no_output "maint set tui-left-margin-verbose on"
+gdb_test_no_output "set tui compact-source on"
+
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+set re_border "\\|"
+Term::check_contents "compact source format" \
+    "${re_border}___04_  return 0; *$re_border"
+
+with_test_prefix window-resize=1 {
+    Term::command "wh src -1"
+    Term::check_contents "compact source" \
+	"${re_border}___4_  return 0; *$re_border"
+}
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index 3d08ea8b7cd..1233e945cab 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -79,8 +79,12 @@  tui_source_window::set_contents (struct gdbarch *arch,
     {
       /* Solaris 11+gcc 5.5 has ambiguous overloads of log10, so we
 	 cast to double to get the right one.  */
-      double l = log10 ((double) offsets->size ());
-      m_digits = 1 + (int) l;
+      int lines_in_file = offsets->size ();
+      int last_line_nr_in_window = line_no + nlines - 1;
+      int max_line_nr = std::max (lines_in_file, last_line_nr_in_window);
+      int digits_needed = 1 + (int)log10 ((double) max_line_nr);
+      int trailing_space = 1;
+      m_digits = digits_needed + trailing_space;
     }
 
   m_max_length = -1;
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 7eac03f47a1..6710b3e17e5 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -1274,8 +1274,7 @@  When enabled GDB will print a message when the terminal is resized."),
 Set whether the TUI source window is compact."), _("\
 Show whether the TUI source window is compact."), _("\
 This variable controls whether the TUI source window is shown\n\
-in a compact form.  The compact form puts the source closer to\n\
-the line numbers and uses less horizontal space."),
+in a compact form.  The compact form uses less horizontal space."),
 			   tui_set_compact_source, tui_show_compact_source,
 			   &tui_setlist, &tui_showlist);