From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id AB6CB3856242 for ; Wed, 10 May 2023 09:03:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AB6CB3856242 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1683709385; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=fXCy/KmwRW54e4T3oV/I+TYidvlIHnFnQayuxTXdKYo=; b=OGMXItYFPTEukJCnams98W/uwIML2vst+RLK+1im7LiTh1TQuXi+Ha20LwH+np2GxEM7fB oDCbJVk/JIp7U+4HRoTwn4MPLy0lQdVFUFWvLHttiSi0rU2nD46hPpt4qZEtcbwj9R6hB6 1zGqGp5oK6yfYy4zkt+MFHw251BZ78k= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-368-opnsPcrfNna3oNaS20W4DQ-1; Wed, 10 May 2023 05:03:04 -0400 X-MC-Unique: opnsPcrfNna3oNaS20W4DQ-1 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-3077fa61967so3421678f8f.1 for ; Wed, 10 May 2023 02:03:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683709383; x=1686301383; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fXCy/KmwRW54e4T3oV/I+TYidvlIHnFnQayuxTXdKYo=; b=lak9LQ2EP4hZ8u6Cud6dpO4q4qwzeEJEeY0DZlVmErLRln9sP0y/vtumIs1sHCZjOM FMkVJlGqgAKJOJKpEmRKMUliaQ+YRSCJJVcP9/LTCiynb3MT755umdU3e7EnFDB5wzRi g4M7C94PE5ZwFzELYxU8oMiCKU5pvymmcLdZfB823qzqoFFvWqclkIFZkf0+t+Tj+rQt sTJo2WqRQFYu1cR5w9y5xjNf9NhB6kWrlgV0NQqB9XaX2/PvTlVTepYrcEAyGzS39nG6 dH2EH7Sb2ePAcp0RvMvcicEaNmAMugbcrh2MPNyn1hcq9h35mZ5zHU2NHc0dFmw6tU1R I1KA== X-Gm-Message-State: AC+VfDweiR1h3rroZ5MBCZCX6S7RJNWOXYcVlMV9Wydtdh3Wl02xL7tN raChAaqLtjbETWyslmaIHGIT/rK1j22/EtgLTaPq2NPM33fJO9kJLsMWBvOX2OAY4TYRUi3PqIj ZPBNcZxB+8kI2RnEE52oHow== X-Received: by 2002:adf:f302:0:b0:306:3b78:fe33 with SMTP id i2-20020adff302000000b003063b78fe33mr11818269wro.32.1683709383040; Wed, 10 May 2023 02:03:03 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6DIL68Nf8CO5vYyYbO3/pAQbnQnKO+FUz/FN6h40JBz1TspL3YZZjqo/Z3yq1nyUhV4MCVGw== X-Received: by 2002:adf:f302:0:b0:306:3b78:fe33 with SMTP id i2-20020adff302000000b003063b78fe33mr11818248wro.32.1683709382599; Wed, 10 May 2023 02:03:02 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id h14-20020a5d6e0e000000b0030631dcbea6sm16741766wrz.77.2023.05.10.02.03.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 May 2023 02:03:02 -0700 (PDT) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org Cc: Tom Tromey Subject: Re: [PATCH] [gdb/tui] Make tui compact-source more compact In-Reply-To: <20230510062015.20974-1-tdevries@suse.de> References: <20230510062015.20974-1-tdevries@suse.de> Date: Wed, 10 May 2023 10:03:00 +0100 Message-ID: <87jzxg7e4b.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tom de Vries via Gdb-patches writes: > I noticed with tui compact-source on, that when reducing the height of the > src window we go from: > ... > |___09_ i++; | > |___10_ i++; | > +--------------------------------------+ > ... > to: > ... > |___09_ i++; | > +--------------------------------------+ > ... > In other words, the digits used to print 9 remains 2, even though only 1 is > needed. The benefit of using two digits here is that as we scroll down, and lines 10+ appear on the screen, the source code doesn't move 1 character to the right. This will happen again when line 100 appears. The docs (as you quoted in commit 2093c2af3c58) say: 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. So the '09' is the expected behaviour for a file with a double digit number of lines. I think I prefer the original behaviour -- your change certainly makes sense when looking at a pretty small src window, at a pretty small file, but this feels like an edge case. That said, I don't feel strongly enough to block the change if you really want to see it merged -- but you'll need to update the docs. > > This is due to this line in tui_source_window::set_contents: > ... > int max_line_nr = std::max (lines_in_file, last_line_nr_in_window); That line is not correct I think. Better would be something like: int max_line_nr = lines_in_file + nlines - 1; this accounts for that at the limit we can scroll such that only the last line of the source file is displayed, then the rest of the source window can also have line numbers assigned. A source file with 9 lines will (currently) still see a source shift as the (empty) line 10 appears. > ... > which takes both the last line in the window and the total lines in the source > file into account. > > Make "tui compact-source" more compact by just using: > ... > int max_line_nr = last_line_nr_in_window; > ... > such that we have: > ... > |___9_ i++; | > +--------------------------------------+ > ... > > Tested on x86_64-linux. > --- > gdb/testsuite/gdb.tui/compact-source-2.exp | 68 ++++++++++++++++++++++ > gdb/tui/tui-source.c | 3 +- > 2 files changed, 69 insertions(+), 2 deletions(-) > create mode 100644 gdb/testsuite/gdb.tui/compact-source-2.exp > > diff --git a/gdb/testsuite/gdb.tui/compact-source-2.exp b/gdb/testsuite/gdb.tui/compact-source-2.exp > new file mode 100644 > index 00000000000..416c76e1d26 > --- /dev/null > +++ b/gdb/testsuite/gdb.tui/compact-source-2.exp > @@ -0,0 +1,68 @@ > +# 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 . > + > +# Check that "set tui compact-source on" has the intended effect. This comment will be true once the docs have changed. > + > +require allow_tui_tests > + > +tuiterm_env > + > +standard_testfile > + > +# Let's generate the source file. We want a short file, with more than 10 > +# lines, but with the first line in main below 10. > +set src_list \ > + [list \ > + "int" \ > + "main (void)" \ > + "{" \ > + " int i = 1;" \ > + " i++;" \ > + " i++;" \ > + " i++;" \ > + " i++;" \ > + " i++;" \ > + " return 0;" \ > + "}"] > +set line_four [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 > +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_$line_four *$re_border" > + > +with_test_prefix window-resize=1 { Minor nit: that prefix doesn't seem to reflect what actually changes (height -= 1 vs height == 1). Thanks, Andrew > + Term::command "wh src -1" > + Term::check_contents "compact source" \ > + "${re_border}___4_$line_four *$re_border" > +} > diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c > index 1233e945cab..bef3d3163dd 100644 > --- a/gdb/tui/tui-source.c > +++ b/gdb/tui/tui-source.c > @@ -79,9 +79,8 @@ 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 = 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; > > base-commit: 2093c2af3c58da1fe63807dfea07d56afc6e7a8a > -- > 2.35.3