From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id rP0tFetQlGTL1AoAWB0awg (envelope-from ) for ; Thu, 22 Jun 2023 09:47:23 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=BfVVq+xC; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 4C2961E0BB; Thu, 22 Jun 2023 09:47:23 -0400 (EDT) Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 2F9551E0AC for ; Thu, 22 Jun 2023 09:47:21 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AD5A33858413 for ; Thu, 22 Jun 2023 13:47:20 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AD5A33858413 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1687441640; bh=erJzDNVdbI/NmPjb8/0Bm31TQ+xJJvKA4NAzRLMFkoA=; h=To:Cc:Subject:In-Reply-To:References:Date:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=BfVVq+xClSdfBhzhBpXIbCf4aG8+ozQ0iZPYTOxfmLPg3AlyUxQgtbaTwADqYFKTm cy27wxyjncbQWraoCs1/SWfi77gEhWH/y+IzsRPbErUOey12b+zMlY/NaxjNJU8jZX WQP8/n9OBdobfI8EKoRgLFK0sAx4Ny2N+YlohAUA= 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 5BD053858D35 for ; Thu, 22 Jun 2023 13:46:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5BD053858D35 Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-342-a0_5srqLNmaILXvb_diZaw-1; Thu, 22 Jun 2023 09:46:56 -0400 X-MC-Unique: a0_5srqLNmaILXvb_diZaw-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-3f7e7cfcae4so39407895e9.1 for ; Thu, 22 Jun 2023 06:46:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687441614; x=1690033614; 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=erJzDNVdbI/NmPjb8/0Bm31TQ+xJJvKA4NAzRLMFkoA=; b=VTDPpXqcRAzGqdkvgHS7CDY4Ge6sbyEdEz406e3HpDDQJIUhwn9Qf3oVLLrLSslPhH M6TodztO6ssWq0uMKDLgEjjI9yY9BlymBYDp5Q7dQO6V2ZznA3bpFZoj1FBwiWWt+a2V /m3TvfZ/MS8k0Twp4r4eZwxbe6GaMfHc2w/sm50PW55WuKknFzAToOY7ByOef0YbPbtR p+cUxwyLsAD7yHICqCD8Sa9lS2sJTAmGLqQ90sVbPR7TZ8NwtP+151rRMKdJn0Z68p/e DtyfnTg77yDYVxGr7zu4JWKneUh/MRzpGDCq0Q9dqkVcA74n03V1jpus4mFJZElcNFLX VAiA== X-Gm-Message-State: AC+VfDwnPREezGwzSvswIa9bxdECKuieNLHNBifVA6J3GL+0TnfQUrwd lhBly23mHfD09hdR5VonPZb1gZ8yuxkFXD7nEK3dfeLYOdM7FrNbR0GCKWFrRvQE1BLR5u92w2o qUTtP5n2ne6fv5vahDljYnjwk6Jz2dU8xebWK7hchTwqw1RxU30M0EiF+FyNPYNL+qoWKBb+izk +jGegGaQ== X-Received: by 2002:a7b:cb88:0:b0:3f9:b0ed:b729 with SMTP id m8-20020a7bcb88000000b003f9b0edb729mr10267815wmi.18.1687441613906; Thu, 22 Jun 2023 06:46:53 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5eNMdOeB3DxyuQRNWVjM5ePfSG9hW7RMewBKseQajxj8p4nDt8pAB4PNBV8k9ZxBNPnoHJwg== X-Received: by 2002:a7b:cb88:0:b0:3f9:b0ed:b729 with SMTP id m8-20020a7bcb88000000b003f9b0edb729mr10267795wmi.18.1687441613376; Thu, 22 Jun 2023 06:46:53 -0700 (PDT) Received: from localhost (2.72.115.87.dyn.plus.net. [87.115.72.2]) by smtp.gmail.com with ESMTPSA id n16-20020a1c7210000000b003f50d6ee334sm7686243wmc.47.2023.06.22.06.46.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Jun 2023 06:46:52 -0700 (PDT) To: Bruno Larsen via Gdb-patches , gdb-patches@sourceware.org Cc: Bruno Larsen , Eli Zaretskii Subject: Re: [PATCH v3 2/4] gdb/cli: Improve UX when using list with no args In-Reply-To: <20230621104545.2530552-3-blarsen@redhat.com> References: <20230621104545.2530552-1-blarsen@redhat.com> <20230621104545.2530552-3-blarsen@redhat.com> Date: Thu, 22 Jun 2023 14:46:51 +0100 Message-ID: <878rcbtxro.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.8 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Andrew Burgess via Gdb-patches Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Bruno Larsen via Gdb-patches writes: > When using "list" with no arguments, GDB will first print the lines > around where the inferior is stopped, then print the next N lines until > reaching the end of file, at which point it wanrs the user "Line X out > of range, file Y only has X-1 lines.". This is usually desireable, but > if the user can no longer see the original line, they may have forgotten > the current line or that a list command was used at all, making GDB's > error message look cryptic. It was reported in bugzilla as PR cli/30497. > > This commit improves the user experince by changing the behavior of > "list" slightly when a user passes no arguments. If the line that would > be printed is past the end of the file, GDB will now warn that the > previous list command has reached the end of file, and the current line > wil be listed again. -1 from me. I don't like the idea of this auto-wrap around. I don't feel like this really addresses the original complain anyway. The complaint seems to be: 1. User does 'list' and sees the current location, 2. User runs "other commands" which scrolls the list contents off the screen, 3. User does 'list', but is now unsure what the current location is. The only way this change helps is if the user keeps "list"-ing until they wrap around, at which point they can find the current location. Also, I don't understand what's so cryptic about the error message. As far as I can tell, GDB's auto-repeat system only auto-repeats if the last command is repeatable. So in order to see the error message at all the last command typed _must_ have been a 'list' command, so why wouldn't the error message apply to that command? I guess maybe the cryptic nature comes from, why is GDB trying to list line number X at all? But surely that should suggest to the user that if they don't want to see like X, they should try 'list NN' instead? Maybe the right thing to do here is provide more helpful text when reaching the end of the file? Or maybe extend the 'help list' text to give more guidance to users on how to return to the current location? I did see Keith's suggestion for relisting the last few lines. I think I'm neutral on that idea, I wouldn't object to it, but the current just a warning makes it clear GDB has stopped progressing, which I think could be useful. Thanks, Andrew > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30497 > Reviewed-By: Eli Zaretskii > --- > gdb/NEWS | 7 +++++++ > gdb/cli/cli-cmds.c | 36 +++++++++++++++++++++++++++++++-- > gdb/doc/gdb.texinfo | 7 +++++-- > gdb/source.c | 18 +++++++++++++++++ > gdb/source.h | 4 ++++ > gdb/testsuite/gdb.base/list.exp | 8 ++++---- > 6 files changed, 72 insertions(+), 8 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index fd42864c692..348e73dd15f 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -77,6 +77,13 @@ > as the array would be printed by the 'print' command. This > functionality is also available for dprintf when dprintf-style is > 'gdb'. > + > +* Using the 'list' command with no arguments in a situation where the > + command would attempt to list past the end of the file now warns the > + user that the end of file has been reached, and prints the default > + location. Previously, it would error out. The default location for > + this purpose is the last solitary line printed, if there was one, > + else the lines around the main function. > > * New commands > > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index b0b9c08c2ec..5973aebfad3 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -1244,8 +1244,40 @@ list_command (const char *arg, int from_tty) > list_around_line (arg, cursal); > } > > - /* "l" or "l +" lists next ten lines. */ > - else if (arg == NULL || arg[0] == '+') > + /* "l" lists the next few lines, unless we're listing past the end of > + the file. If it would be past the end, re-print the current line. */ > + else if (arg == nullptr) > + { > + if (can_print_line (cursal.symtab, cursal.line)) > + print_source_lines (cursal.symtab, > + source_lines_range (cursal.line), 0); > + else > + { > + warning (_("previous list command has already reached the end " > + "of the file. Listing default location again")); > + try > + { > + /* Find the current line by getting the PC of the currently > + selected frame, and finding the line associated to it. */ > + frame_info_ptr frame = get_selected_frame (nullptr); > + CORE_ADDR curr_pc = get_frame_pc (frame); > + cursal = find_pc_line (curr_pc, 0); > + } > + catch (const gdb_exception &e) > + { > + /* If there was an exception above, it means the inferior > + is not running, so reset the current source location to > + the default. */ > + clear_current_source_symtab_and_line (); > + set_default_source_symtab_and_line (); > + cursal = get_current_source_symtab_and_line (); > + } > + list_around_line (arg, cursal); > + } > + } > + > + /* "l +" always lists next few lines. */ > + else if (arg[0] == '+') > print_source_lines (cursal.symtab, > source_lines_range (cursal.line), 0); > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index b10c06ae91f..55010f69a1c 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9142,9 +9142,12 @@ Print lines centered around the beginning of function > @item list > Print more lines. If the last lines printed were printed with a > @code{list} command, this prints lines following the last lines > -printed; however, if the last line printed was a solitary line printed > +printed; however, if those lines are past the end of the source > +file, or if the last line printed was a solitary line printed > as part of displaying a stack frame (@pxref{Stack, ,Examining the > -Stack}), this prints lines centered around that line. > +Stack}), this prints lines centered around that line. If no > +@code{list} command has been used and no solitary line was printed, > +it prints the lines around the function @code{main}. > > @item list - > Print lines just before the lines last printed. > diff --git a/gdb/source.c b/gdb/source.c > index 9997cccb31b..1aa08c6e080 100644 > --- a/gdb/source.c > +++ b/gdb/source.c > @@ -1484,6 +1484,24 @@ print_source_lines (struct symtab *s, source_lines_range line_range, > line_range.stopline (), flags); > } > > +/* See source.h. */ > + > +bool > +can_print_line (struct symtab *s, int line) > +{ > + const std::vector *offset_p; > + > + /* If we can't get the offsets, we definitely can't print the line. */ > + if (!g_source_cache.get_line_charpos (s, &offset_p)) > + return false; > + if (offset_p == nullptr) > + return false; > + > + /* Otherwise we are able to print LINE if there are at least that many > + lines in the symtab. */ > + return line <= offset_p->size (); > +} > + > > > /* Print info on range of pc's in a specified line. */ > diff --git a/gdb/source.h b/gdb/source.h > index 8fbc365680d..ae6764322d0 100644 > --- a/gdb/source.h > +++ b/gdb/source.h > @@ -192,6 +192,10 @@ class source_lines_range > int m_stopline; > }; > > +/* Check if the line LINE can be found in the symtab S, so that it can be > + printed. */ > +extern bool can_print_line (struct symtab *s, int line); > + > /* Variation of previous print_source_lines that takes a range instead of a > start and end line number. */ > extern void print_source_lines (struct symtab *s, source_lines_range r, > diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp > index 18ecd13ac0f..35e099ebaff 100644 > --- a/gdb/testsuite/gdb.base/list.exp > +++ b/gdb/testsuite/gdb.base/list.exp > @@ -175,8 +175,8 @@ proc_with_prefix test_list_forward {} { > "list 25-34" > gdb_test "list" "35\[ \t\]+foo \\(.*\\);.*${last_line_re}" \ > "list 35-42" > - gdb_test "list" "Line number 44 out of range; \[^\r\n\]+ has 43 lines\." \ > - "end of file error after \"list\" command" > + gdb_test "list" "warning: previous list command has already reached the end of the file. Listing default location again.*1\[ \t\]+#include \"list0.h\".*" \ > + "list past end of file" > } > > # Test that repeating the list linenum command doesn't print the same > @@ -194,8 +194,8 @@ proc_with_prefix test_repeat_list_command {} { > "list 25-34" > gdb_test " " "35\[ \t\]+foo \\(.*\\);.*${last_line_re}" \ > "list 35-42" > - gdb_test "list" "Line number 44 out of range; \[^\r\n\]+ has 43 lines\." \ > - "end of file error after using 'return' to repeat the list command" > + gdb_test "list" "warning: previous list command has already reached the end of the file. Listing default location again.*1\[ \t\]+#include \"list0.h\".*" \ > + "list past end of file" > } > > proc_with_prefix test_list_backwards {} { > -- > 2.40.1