* [PATCH] [gdb/tui] Fix tui compact-source a bit more @ 2023-05-10 13:21 Tom de Vries 2023-05-10 15:28 ` Tom Tromey 2023-05-11 14:55 ` Simon Marchi 0 siblings, 2 replies; 6+ messages in thread From: Tom de Vries @ 2023-05-10 13:21 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess Andrew pointed out that the behaviour as tested in gdb.tui/compact-source.exp is incorrect: ... 0 +-compact-source.c--------------------------------------------------------+ 1 |___3_{ | 2 |___4_ return 0; | 3 |___5_} | 4 |___6_ | 5 |___7_ | 6 |___8_ | 7 |___9_ | 8 +-------------------------------------------------------------------------+ ... The last line number in the source file is 5, and there are 7 lines to display source lines, so if we'd scroll all the way down, the first line number in the source window would be 5, and the last one would be 11. To represent 11 we'd need 2 digits, so we expect to see ___04_ here instead of ___4_, even though all line numbers currently in the src window (3-9) can be represented with only 1 digit. Fix this in tui_source_window::set_contents, by updating the computation of max_line_nr: ... - int max_line_nr = std::max (lines_in_file, last_line_nr_in_window); + int max_line_nr = lines_in_file + nlines - 1; ... Tested on x86_64-linux. Co-Authored-By: Andrew Burgess <aburgess@redhat.com> --- gdb/testsuite/gdb.tui/compact-source.exp | 43 +++++++++++++++--------- gdb/tui/tui-source.c | 3 +- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/gdb/testsuite/gdb.tui/compact-source.exp b/gdb/testsuite/gdb.tui/compact-source.exp index 71e6b7b0b0a..f972d961d72 100644 --- a/gdb/testsuite/gdb.tui/compact-source.exp +++ b/gdb/testsuite/gdb.tui/compact-source.exp @@ -23,14 +23,15 @@ 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 src_list \ + [list \ + "int" \ + "main (void)" \ + "{" \ + " return 0;" \ + "}"] +set re_line_four [string_to_regexp [lindex $src_list 3]] +set src_txt [join $src_list "\n"] set srcfile [standard_output_file $srcfile] set fd [open $srcfile w] puts $fd $src_txt @@ -40,7 +41,7 @@ if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} { return -1 } -Term::clean_restart 17 80 $binfile +Term::clean_restart 24 80 $binfile gdb_test_no_output "maint set tui-left-margin-verbose on" gdb_test_no_output "set tui compact-source on" @@ -51,11 +52,23 @@ if {![Term::enter_tui]} { } 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" +foreach_with_prefix src_window_size {7 8} { + set src_window_lines [expr $src_window_size - 2] + set max_line_nr_in_source_file [llength $src_list] + set max_line_nr_in_source_window \ + [expr $max_line_nr_in_source_file + $src_window_lines - 1] + + Term::command "wh src $src_window_size" + + if { $max_line_nr_in_source_window == 9 } { + set re_left_margin "___4_" + } elseif { $max_line_nr_in_source_window == 10 } { + set re_left_margin "___04_" + } else { + error "unhandled max_line_nr_in_source_window" + } + + Term::check_contents "compact source format" \ + "$re_border$re_left_margin$re_line_four *$re_border" } diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c index 1233e945cab..9d0376000de 100644 --- a/gdb/tui/tui-source.c +++ b/gdb/tui/tui-source.c @@ -80,8 +80,7 @@ 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. */ 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 max_line_nr = lines_in_file + nlines - 1; int digits_needed = 1 + (int)log10 ((double) max_line_nr); int trailing_space = 1; m_digits = digits_needed + trailing_space; base-commit: 8b7b3b2bf4357781439e5434c4a5942ea29e983d -- 2.35.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Fix tui compact-source a bit more 2023-05-10 13:21 [PATCH] [gdb/tui] Fix tui compact-source a bit more Tom de Vries @ 2023-05-10 15:28 ` Tom Tromey 2023-05-11 14:55 ` Simon Marchi 1 sibling, 0 replies; 6+ messages in thread From: Tom Tromey @ 2023-05-10 15:28 UTC (permalink / raw) To: Tom de Vries via Gdb-patches; +Cc: Tom de Vries, Andrew Burgess >>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes: Tom> To represent 11 we'd need 2 digits, so we expect to see ___04_ here instead of Tom> ___4_, even though all line numbers currently in the src window (3-9) can be Tom> represented with only 1 digit. Tom> Fix this in tui_source_window::set_contents, by updating the computation of Tom> max_line_nr: Tom> ... Tom> - int max_line_nr = std::max (lines_in_file, last_line_nr_in_window); Tom> + int max_line_nr = lines_in_file + nlines - 1; Tom> ... Makes sense to me. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Fix tui compact-source a bit more 2023-05-10 13:21 [PATCH] [gdb/tui] Fix tui compact-source a bit more Tom de Vries 2023-05-10 15:28 ` Tom Tromey @ 2023-05-11 14:55 ` Simon Marchi 2023-05-11 17:45 ` Tom de Vries 1 sibling, 1 reply; 6+ messages in thread From: Simon Marchi @ 2023-05-11 14:55 UTC (permalink / raw) To: Tom de Vries, gdb-patches; +Cc: Andrew Burgess On 5/10/23 09:21, Tom de Vries via Gdb-patches wrote: > Andrew pointed out that the behaviour as tested in gdb.tui/compact-source.exp > is incorrect: > ... > 0 +-compact-source.c--------------------------------------------------------+ > 1 |___3_{ | > 2 |___4_ return 0; | > 3 |___5_} | > 4 |___6_ | > 5 |___7_ | > 6 |___8_ | > 7 |___9_ | > 8 +-------------------------------------------------------------------------+ > ... > > The last line number in the source file is 5, and there are 7 lines to display > source lines, so if we'd scroll all the way down, the first line number in the > source window would be 5, and the last one would be 11. > > To represent 11 we'd need 2 digits, so we expect to see ___04_ here instead of > ___4_, even though all line numbers currently in the src window (3-9) can be > represented with only 1 digit. Just wondering: it makes sense to let the user scroll down all the way, until the point where line 5 is the first line in the window. However, do we need to print line numbers for lines that are after the end of the source file? In other words, could we leave lines 6-11 blank, and therefore we could keep using just one digit? Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Fix tui compact-source a bit more 2023-05-11 14:55 ` Simon Marchi @ 2023-05-11 17:45 ` Tom de Vries 2023-05-12 18:40 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Tom de Vries @ 2023-05-11 17:45 UTC (permalink / raw) To: Simon Marchi, gdb-patches; +Cc: Andrew Burgess, Tom Tromey [-- Attachment #1: Type: text/plain, Size: 1769 bytes --] On 5/11/23 16:55, Simon Marchi wrote: > On 5/10/23 09:21, Tom de Vries via Gdb-patches wrote: >> Andrew pointed out that the behaviour as tested in gdb.tui/compact-source.exp >> is incorrect: >> ... >> 0 +-compact-source.c--------------------------------------------------------+ >> 1 |___3_{ | >> 2 |___4_ return 0; | >> 3 |___5_} | >> 4 |___6_ | >> 5 |___7_ | >> 6 |___8_ | >> 7 |___9_ | >> 8 +-------------------------------------------------------------------------+ >> ... >> >> The last line number in the source file is 5, and there are 7 lines to display >> source lines, so if we'd scroll all the way down, the first line number in the >> source window would be 5, and the last one would be 11. >> >> To represent 11 we'd need 2 digits, so we expect to see ___04_ here instead of >> ___4_, even though all line numbers currently in the src window (3-9) can be >> represented with only 1 digit. > > Just wondering: it makes sense to let the user scroll down all the way, > until the point where line 5 is the first line in the window. However, > do we need to print line numbers for lines that are after the end of the > source file? In other words, could we leave lines 6-11 blank, and > therefore we could keep using just one digit? This demonstrator patch implements that approach. Thanks, - Tom [-- Attachment #2: 0001-gdb-tui-Don-t-show-line-number-for-lines-not-in-sour.patch --] [-- Type: text/x-patch, Size: 2738 bytes --] From 31b7ab1f107b36bf440f89f732e4b46d96d0faa2 Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Thu, 11 May 2023 19:43:32 +0200 Subject: [PATCH] [gdb/tui] Don't show line number for lines not in source file --- gdb/testsuite/gdb.tui/compact-source.exp | 2 +- gdb/tui/tui-source.c | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/gdb/testsuite/gdb.tui/compact-source.exp b/gdb/testsuite/gdb.tui/compact-source.exp index f972d961d72..59e858a503f 100644 --- a/gdb/testsuite/gdb.tui/compact-source.exp +++ b/gdb/testsuite/gdb.tui/compact-source.exp @@ -64,7 +64,7 @@ foreach_with_prefix src_window_size {7 8} { if { $max_line_nr_in_source_window == 9 } { set re_left_margin "___4_" } elseif { $max_line_nr_in_source_window == 10 } { - set re_left_margin "___04_" + set re_left_margin "___4_" } else { error "unhandled max_line_nr_in_source_window" } diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c index 9d0376000de..57f9a62c659 100644 --- a/gdb/tui/tui-source.c +++ b/gdb/tui/tui-source.c @@ -80,7 +80,7 @@ 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. */ int lines_in_file = offsets->size (); - int max_line_nr = lines_in_file + nlines - 1; + int max_line_nr = lines_in_file; int digits_needed = 1 + (int)log10 ((double) max_line_nr); int trailing_space = 1; m_digits = digits_needed + trailing_space; @@ -100,6 +100,8 @@ tui_source_window::set_contents (struct gdbarch *arch, text = tui_copy_source_line (&iter, &line_len); m_max_length = std::max (m_max_length, line_len); } + else + cur_line_no = -1; /* Set whether element is the execution point and whether there is a break point on it. */ @@ -233,11 +235,19 @@ tui_source_window::display_start_addr (struct gdbarch **gdbarch_p, void tui_source_window::show_line_number (int offset) const { - int lineno = m_content[0].line_or_addr.u.line_no + offset; + int lineno = m_content[offset].line_or_addr.u.line_no; char text[20]; char space = tui_left_margin_verbose ? '_' : ' '; - xsnprintf (text, sizeof (text), - tui_left_margin_verbose ? "%0*d%c" : "%*d%c", m_digits - 1, - lineno, space); + if (lineno == -1) + { + for (int i = 0; i <= m_digits; ++i) + text[i] = (i == m_digits) ? '\0' : space; + } + else + { + xsnprintf (text, sizeof (text), + tui_left_margin_verbose ? "%0*d%c" : "%*d%c", m_digits - 1, + lineno, space); + } waddstr (handle.get (), text); } base-commit: 38b95a529385e32adc39428231c6fc8b0e8d6f21 -- 2.35.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Fix tui compact-source a bit more 2023-05-11 17:45 ` Tom de Vries @ 2023-05-12 18:40 ` Tom Tromey 2023-05-15 4:03 ` Tom de Vries 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2023-05-12 18:40 UTC (permalink / raw) To: Tom de Vries via Gdb-patches Cc: Simon Marchi, Tom de Vries, Andrew Burgess, Tom Tromey >>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes: >> Just wondering: it makes sense to let the user scroll down all the >> way, >> until the point where line 5 is the first line in the window. However, >> do we need to print line numbers for lines that are after the end of the >> source file? In other words, could we leave lines 6-11 blank, and >> therefore we could keep using just one digit? Tom> This demonstrator patch implements that approach. FWIW I'd be fine with this. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Fix tui compact-source a bit more 2023-05-12 18:40 ` Tom Tromey @ 2023-05-15 4:03 ` Tom de Vries 0 siblings, 0 replies; 6+ messages in thread From: Tom de Vries @ 2023-05-15 4:03 UTC (permalink / raw) To: Tom Tromey, Tom de Vries via Gdb-patches; +Cc: Simon Marchi, Andrew Burgess On 5/12/23 20:40, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes: > >>> Just wondering: it makes sense to let the user scroll down all the >>> way, >>> until the point where line 5 is the first line in the window. However, >>> do we need to print line numbers for lines that are after the end of the >>> source file? In other words, could we leave lines 6-11 blank, and >>> therefore we could keep using just one digit? > > Tom> This demonstrator patch implements that approach. > > FWIW I'd be fine with this. Submitted the patch here ( https://sourceware.org/pipermail/gdb-patches/2023-May/199568.html ). Thanks, - Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-15 4:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-10 13:21 [PATCH] [gdb/tui] Fix tui compact-source a bit more Tom de Vries 2023-05-10 15:28 ` Tom Tromey 2023-05-11 14:55 ` Simon Marchi 2023-05-11 17:45 ` Tom de Vries 2023-05-12 18:40 ` Tom Tromey 2023-05-15 4:03 ` Tom de Vries
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).