From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11930 invoked by alias); 4 Dec 2011 16:55:09 -0000 Received: (qmail 11916 invoked by uid 22791); 4 Dec 2011 16:55:07 -0000 X-SWARE-Spam-Status: No, hits=-1.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout22.012.net.il (HELO mtaout22.012.net.il) (80.179.55.172) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 04 Dec 2011 16:54:50 +0000 Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0LVO00H00UO6SK00@a-mtaout22.012.net.il> for gdb-patches@sourceware.org; Sun, 04 Dec 2011 18:54:48 +0200 (IST) Received: from HOME-C4E4A596F7 ([77.124.128.163]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0LVO00H0AUZ9TD10@a-mtaout22.012.net.il>; Sun, 04 Dec 2011 18:54:48 +0200 (IST) Date: Sun, 04 Dec 2011 16:55:00 -0000 From: Eli Zaretskii Subject: Re: [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename) In-reply-to: To: iam ahal Cc: tromey@redhat.com, dje@google.com, gdb-patches@sourceware.org, pmuldoon@redhat.com, brobecker@adacore.com, pedro@codesourcery.com, drow@false.org, jan.kratochvil@redhat.com Reply-to: Eli Zaretskii Message-id: <83liqsb7mx.fsf@gnu.org> References: <20110627160029.GF20676@adacore.com> <834o33qlm9.fsf@gnu.org> <83bowq6x7f.fsf@gnu.org> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-12/txt/msg00102.txt.bz2 > Date: Sun, 4 Dec 2011 18:52:19 +0300 > From: iam ahal > Cc: Doug Evans , gdb-patches@sourceware.org, eliz@gnu.org, > pmuldoon@redhat.com, brobecker@adacore.com, pedro@codesourcery.com, > drow@false.org, jan.kratochvil@redhat.com > > On Wed, Nov 2, 2011 at 11:05 PM, Tom Tromey wrote: > > I don't recall whether this has had a doc review yet. > > You have not reviewed a text for this feature in the documentation. Below. > > I think the patch should also include a NEWS entry. > > I don't have an idea how to fill out a NEWS entry. It has descriptions > only for release versions. A NEWS entry should announce the new option, see the section "New options" for a few examples. > --- gdb-7.3.1-orig/gdb/doc/gdb.texinfo 2011-09-04 21:10:37.000000000 +0400 > +++ gdb-7.3.1/gdb/doc/gdb.texinfo 2011-12-04 18:16:03.182293844 +0400 > @@ -6052,6 +6052,19 @@ unlimited. > > @item show backtrace limit > Display the current limit on backtrace levels. > + > +@item set backtrace filename-display > +@itemx set backtrace filename-display full > +Display a full filename. This is the default. > + > +@item set backtrace filename-display basename > +Display only basename of a filename. > + > +@item set backtrace filename-display without-compilation-directory > +Display a filename without the compilation directory part. > + > +@item show backtrace filename-display > +Display the current way to display a filename. > @end table This has several problems. First of all, it is not a good idea to add these entries to this particular @table, because the text preceding it talks about a completely unrelated situation: If you need to examine the startup code, or limit the number of levels in a backtrace, you can change this behavior: So I think this needs a separate preamble, 1 or 2 sentences explaining the situation where this option is useful, and a separate @table. Second, this: > +@item set backtrace filename-display > +@itemx set backtrace filename-display full > +Display a full filename. This is the default. is too general: this option controls only the display of file names in backtraces, right? So the description should say "Display ... in backtraces." Third, how come "full" is the default? are we changing the present behavior whereby only the basename is displayed in backtraces? Or am I missing something? Fourth, I couldn't understand clearly what is the effect of without-compilation-directory value. I think we should explain it in a more clear way. Fifth, "Display the current way to display..." should be reworded to avoid using "display" twice in a row. Finally, this option should have a @cindex entry. I can fix this text, if you want, once I understand the situation with the default value of this option, and once I understand the semantics of without-compilation-directory. > + add_setshow_enum_cmd ("filename-display", class_obscure, > + filename_display_kind_names, > + &filename_display_string, _("\ > +Set a way how to display filename."), _("\ > +Show a way how to display filename."), _("\ Again, this should say "in backtraces". And please remove the "a way" part, it is not needed in both of these sentences. Just "Set how to display ..." is good enough. Thanks.