From patchwork Mon Dec 23 00:03:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 37069 Received: (qmail 47544 invoked by alias); 23 Dec 2019 00:03:51 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 47536 invoked by uid 89); 23 Dec 2019 00:03:51 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=taste, cleanups, offsets, carry X-HELO: gateway22.websitewelcome.com Received: from gateway22.websitewelcome.com (HELO gateway22.websitewelcome.com) (192.185.47.100) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 23 Dec 2019 00:03:49 +0000 Received: from cm17.websitewelcome.com (cm17.websitewelcome.com [100.42.49.20]) by gateway22.websitewelcome.com (Postfix) with ESMTP id 13B82607E for ; Sun, 22 Dec 2019 18:03:47 -0600 (CST) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id jBCNi4CrjqNtvjBCNikdGw; Sun, 22 Dec 2019 18:03:47 -0600 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=CBkSehHZWxOuASREmL65fTOT/QQ0R3+yPilZUYj8e4s=; b=qWLecvYl01Xobx8SELxowfOkv0 eXtcmurj62N6u80WwAK4qIsAjA2YdN2dczYdS3wDhBpUAfFd8DlBPEmENl89WTq9dKBI8bvIEecWk gBN82UL+lBFuBitmTfPyEwC3d; Received: from 75-166-123-50.hlrn.qwest.net ([75.166.123.50]:33574 helo=bapiya) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92) (envelope-from ) id 1ijBCM-000wlR-QH; Sun, 22 Dec 2019 17:03:46 -0700 From: Tom Tromey To: Andrew Burgess Cc: Hannes Domani , gdb-patches@sourceware.org, Tom Tromey Subject: Re: [RFC][PATCH] Call tui_before_prompt in do_scroll_vertical References: <20191221153325.6961-1-ssbssa.ref@yahoo.de> <20191221153325.6961-1-ssbssa@yahoo.de> <20191222005754.GF3865@embecosm.com> Date: Sun, 22 Dec 2019 17:03:45 -0700 In-Reply-To: <20191222005754.GF3865@embecosm.com> (Andrew Burgess's message of "Sun, 22 Dec 2019 00:57:54 +0000") Message-ID: <874kxrc3ha.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1.91 (gnu/linux) MIME-Version: 1.0 >>>>> "Andrew" == Andrew Burgess writes: >> First I tried it with tui_update_source_windows_with_line, but this didn't >> reset from_source_symtab (which was set deep in print_source_lines), >> which resulted in some weird behavior when switching from "layout split" >> to "layout asm" after scrolling down in the src window (the asm window >> was then overwritten by the src window). I wonder if we really need to change the current source symtab. If so, I guess the TUI could just call set_current_source_symtab_and_line directly. The patch below removes the call to print_source_lines -- this approach just seems too roundabout for my taste. Andrew> I was able to reproduce the layout split -> layout asm issue you Andrew> describe, but, I'm not convinced that the solution below is the right Andrew> way to go. With my patch I can't reproduce this, but I don't know whether I'm doing it properly. Hannes, could you possibly try this patch? This could maybe be made even a bit smaller, but it's hard to be sure. Despite all the cleanups, I find this area still pretty confusing. Tom commit 5a30635b352a9c0ff896c5044296087a3eb42073 Author: Tom Tromey Date: Sun Dec 22 16:52:56 2019 -0700 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 diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0f35deff350..b441c9d5031 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +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. + 2019-12-20 Tom Tromey * tui/tui.c (tui_show_source): Remove. diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 4476549be6b..985a68bc126 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,11 @@ +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. + 2019-12-20 Tom Tromey * gdb.tui/list-before.exp: New file. diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp index f9d57d1cf6d..ca88dad1cd4 100644 --- a/gdb/testsuite/gdb.tui/basic.exp +++ b/gdb/testsuite/gdb.tui/basic.exp @@ -35,6 +35,17 @@ 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 [string_to_regexp [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 $line] && [Term::get_line 1] != $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 81247d5d9a4..7761c63d6aa 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 @@ -480,7 +484,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 @@ -641,14 +645,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 0728263b8c5..bcba9e176f1 100644 --- a/gdb/tui/tui-source.c +++ b/gdb/tui/tui-source.c @@ -136,28 +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 ()) - /* line = s->nlines - win_info->content_size + 1; */ - /* elz: fix for dts 23398. */ - l.u.line_no = start_line_or_addr.u.line_no; - if (l.u.line_no <= 0) - l.u.line_no = 1; - - print_source_lines (s, l.u.line_no, l.u.line_no + 1, 0); + && line_no > offsets->size ()) + line_no = start_line_or_addr.u.line_no; + if (line_no <= 0) + line_no = 1; + + cursal.line = line_no; + update_source_window (arch, cursal); } }