From 9ae6bf640dc7c950e6f36097a3d2d760a132a542 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Sun, 22 Dec 2019 16:52:56 -0700 Subject: [PATCH] gdb: Fix scrolling in TUI Hannes Domani pointed out that my previous patch to fix the "list" command in the TUI instead broke vertical scrolling. While looking at this, I found that do_scroll_vertical calls print_source_lines, which seems like a very roundabout way to change the source window. This patch removes this oddity and fixes the bug at the same time. I've added a new test case. This is somewhat tricky, because the obvious approach of sending a dummy command after the scroll did not work -- due to how the TUI works, sennding a command causes the scroll to take effect. gdb/ChangeLog 2019-12-22 Tom Tromey PR tui/18932: * tui/tui-source.c (tui_source_window::do_scroll_vertical): Call update_source_window, not print_source_lines. gdb/testsuite/ChangeLog 2019-12-22 Tom Tromey PR tui/18932: * lib/tuiterm.exp (Term::wait_for): Rename from _accept. Return a meangingful value. (Term::command, Term::resize): Update. * gdb.tui/basic.exp: Add scrolling test. Change-Id: I9636a7c8a8cade37431c6165ee996a9d556ef1c8 --- gdb/ChangeLog | 6 ++++++ gdb/testsuite/ChangeLog | 8 ++++++++ gdb/testsuite/gdb.tui/basic.exp | 13 +++++++++++++ gdb/testsuite/lib/tuiterm.exp | 16 ++++++++++------ gdb/tui/tui-source.c | 23 +++++++++++++---------- 5 files changed, 50 insertions(+), 16 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0ff44e50016..4c789119cec 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2020-01-09 Tom Tromey + + PR tui/18932: + * tui/tui-source.c (tui_source_window::do_scroll_vertical): Call + update_source_window, not print_source_lines. + 2020-01-09 Andrew Burgess * tui/tui.c (tui_enable): Register tui hooks after calling diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 0f2333bc1c4..16f3b5503df 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2020-01-09 Tom Tromey + + PR tui/18932: + * lib/tuiterm.exp (Term::wait_for): Rename from _accept. Return a + meangingful value. + (Term::command, Term::resize): Update. + * gdb.tui/basic.exp: Add scrolling test. + 2020-01-09 Andrew Burgess * gdb.tui/tui-layout-asm.exp: New file. diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp index c3a3fdd4f5e..be822f8a915 100644 --- a/gdb/testsuite/gdb.tui/basic.exp +++ b/gdb/testsuite/gdb.tui/basic.exp @@ -35,6 +35,19 @@ gdb_assert {![string match "No Source Available" $text]} \ Term::command "list main" Term::check_contents "list main" "21 *return 0" +# Get the first source line. +set line [Term::get_line 1] +# Send an up arrow. +send_gdb "\033\[A" +# Wait for a redraw and check that the first line changed. +if {[Term::wait_for [string_to_regexp $line]] \ + && [Term::get_line 1] != $line\ + && [Term::get_line 2] == $line} { + pass "scroll up" +} else { + fail "scroll up" +} + Term::check_box "source box" 0 0 80 15 Term::command "layout asm" diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp index 0307745d879..7adaf1b71ab 100644 --- a/gdb/testsuite/lib/tuiterm.exp +++ b/gdb/testsuite/lib/tuiterm.exp @@ -388,8 +388,10 @@ namespace eval Term { _clear_lines 0 $_rows } - # Accept some output from gdb and update the screen. - proc _accept {wait_for} { + # Accept some output from gdb and update the screen. WAIT_FOR is + # a regexp matching the line to wait for. Return 0 on timeout, 1 + # on success. + proc wait_for {wait_for} { global expect_out global gdb_prompt variable _cur_x @@ -424,7 +426,7 @@ namespace eval Term { timeout { # Assume a timeout means we somehow missed the # expected result, and carry on. - return + return 0 } } @@ -443,6 +445,8 @@ namespace eval Term { set wait_for $prompt_wait_for } } + + return 1 } # Like ::clean_restart, but ensures that gdb starts in an @@ -490,7 +494,7 @@ namespace eval Term { # be supplied by this function. proc command {cmd} { send_gdb "$cmd\n" - _accept [string_to_regexp $cmd] + wait_for [string_to_regexp $cmd] } # Return the text of screen line N, without attributes. Lines are @@ -682,14 +686,14 @@ namespace eval Term { # Due to the strange column resizing behavior, and because we # don't care about this intermediate resize, we don't check # the size here. - _accept "@@ resize done $_resize_count" + wait_for "@@ resize done $_resize_count" incr _resize_count # Somehow the number of columns transmitted to gdb is one less # than what we request from expect. We hide this weird # details from the caller. _do_resize $_rows $cols stty columns [expr {$_cols + 1}] < $gdb_spawn_name - _accept "@@ resize done $_resize_count, size = ${_cols}x${rows}" + wait_for "@@ resize done $_resize_count, size = ${_cols}x${rows}" incr _resize_count } } diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c index 1503cd4c636..13f2dc7cfe1 100644 --- a/gdb/tui/tui-source.c +++ b/gdb/tui/tui-source.c @@ -136,26 +136,29 @@ tui_source_window::do_scroll_vertical (int num_to_scroll) { if (!content.empty ()) { - struct tui_line_or_address l; struct symtab *s; struct symtab_and_line cursal = get_current_source_symtab_and_line (); + struct gdbarch *arch = gdbarch; if (cursal.symtab == NULL) - s = find_pc_line_symtab (get_frame_pc (get_selected_frame (NULL))); + { + struct frame_info *fi = get_selected_frame (NULL); + s = find_pc_line_symtab (get_frame_pc (fi)); + arch = get_frame_arch (fi); + } else s = cursal.symtab; - l.loa = LOA_LINE; - l.u.line_no = start_line_or_addr.u.line_no - + num_to_scroll; + int line_no = start_line_or_addr.u.line_no + num_to_scroll; const std::vector *offsets; if (g_source_cache.get_line_charpos (s, &offsets) - && l.u.line_no > offsets->size ()) - l.u.line_no = start_line_or_addr.u.line_no; - if (l.u.line_no <= 0) - l.u.line_no = 1; + && line_no > offsets->size ()) + line_no = start_line_or_addr.u.line_no; + if (line_no <= 0) + line_no = 1; - print_source_lines (s, l.u.line_no, l.u.line_no + 1, 0); + cursal.line = line_no; + update_source_window (arch, cursal); } } -- 2.30.2