From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29700 invoked by alias); 7 Dec 2012 15:34:04 -0000 Received: (qmail 29612 invoked by uid 22791); 7 Dec 2012 15:34:02 -0000 X-SWARE-Spam-Status: No, hits=-3.6 required=5.0 tests=AWL,BAYES_00,KAM_STOCKTIP,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SARE_TOWRITE,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 07 Dec 2012 15:33:52 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qB7FXnhC014723 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 7 Dec 2012 10:33:49 -0500 Received: from localhost.localdomain (ovpn-116-39.ams2.redhat.com [10.36.116.39]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qB7FXl5B024382; Fri, 7 Dec 2012 10:33:48 -0500 Message-ID: <50C20C5B.4080906@redhat.com> Date: Fri, 07 Dec 2012 15:34:00 -0000 From: Phil Muldoon MIME-Version: 1.0 To: Eli Zaretskii CC: gdb-patches@sourceware.org Subject: Re: [patch][python] 5 of 5 - Frame filter documentation changes References: <50B8C35F.9040000@redhat.com> <83boeet4wh.fsf@gnu.org> <50C1FE8A.20909@redhat.com> <83624doqdz.fsf@gnu.org> In-Reply-To: <83624doqdz.fsf@gnu.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: 2012-12/txt/msg00178.txt.bz2 On 12/07/2012 03:04 PM, Eli Zaretskii wrote: >> Date: Fri, 07 Dec 2012 14:34:50 +0000 >> From: Phil Muldoon >> CC: gdb-patches@sourceware.org >> >>>> +@defun FrameWrapper.elided () >>>> + >>>> +The @code{elided} method groups frames together in a hierarchical >>>> +system. An example would be an interpreter call that occurs over many >>>> +frames but might be better represented as a group of frames distinct >>>> +from the other frames. >>>> + >>>> +The @code{elide} function must return an iterator that conforms to the >>>> +Python iterator protocol. >>> >>> "elide" or "elided"? >> >> I don't care, but this function returns frames that have been elided >> so it made sense to me. The eliding itself is done in the frame >> filter or in another Python object. > > No, I meant that to say that the method is called "elided", but the > last sentence uses "elide". Does that reference some other function? No it sure doesn't. Typo. I will fix it in the next patch. >>>> +@defun FrameWrapper.frame_args () >>>> + >>>> +This method must return an iterator that conforms to the Python >>>> +iterator protocol, or a Python @code{None}. This iterator must >>>> +contain objects that implement two methods, described here. >>>> + >>>> +The object must implement an @code{argument} method which takes no >>>> +parameters and must return a @code{gdb.Symbol} or a Python string. It >>>> +must also implement a @code{value} method which takes no parameters >>>> +and which must return a @code{gdb.Value}, a Python value, or >>>> +@code{None}. If the @code{value} method returns a Python @code{None}, >>>> +and the @code{argument} method returns a @code{gdb.Symbol}, >>>> +@value{GDBN} will look-up and print the value of the @code{gdb.Symbol} >>>> +automatically. >>> >>> I would suggest to describe what are the symbol and the value here. >>> Maybe the reader will be able to guess that, but why expect them to >>> guess? >> >> I really struggled with this section. As value and symbol are not >> simple things to describe in the scope of this function definition. > > Isn't the symbol the name of an argument and the value its > corresponding value? If so, just say that. Ok, thanks will do. >>>> The input to the first frame >>>> +filter will be an initial iterator wrapping a collection of >>>> +@code{BaseFrameWrapper} objects. The output from the previous filter >>>> +will always be the input to the next filter, and so on. >>> >>> I would suggest to move all the text of this node up to here to the >>> "Frame Filters API" section. This text provides an overview of how >>> filters and wrappers work, which IMO is sorely missed before you dive >>> into describing the API itself. Having this overview there will allow >>> the reader to better understand what you describe in the correct >>> context. In any case, the text up to here has almost nothing to do >>> with "writing a filter/wrapper", which is the subject of this section. >> >> >> What I tried to do with this functionality is provide four sections >> that stood independently. Frame Filters API and Frame Wrappers API >> which were API references. These would form a library of the API >> calls. But writing a frame filter also implies knowledge of frame >> wrappers. And putting this in the Frame Filters API seemed the wrong >> place. It's an API section, not a description of how frame filters >> and frame wrappers work together. So my opinion was that API sections >> should only reference API like content. So I decided to write a >> cookbook like section. This section, with suitable cross-references, >> brought the knowledge stored in the two API sections and approached >> the problem of how to write a Frame Filter and Frame Wrapper with an >> example focused approach. I would still like to maintain this >> approach if I can. (The last section I have not talked about here is >> Managing Frame Filters, with GDB commands). > > You are IMO assigning too much importance to the names of the > sections. Even if your concerns are justified, renaming the sections > is easy. Ah I think I misread your initial request. I did not parse the "up to here" phrase in conjunction with the position of your in-line comments with the patch; I thought you wanted to move the whole node into frame filters node. Thanks for explaining. >>>> +(gdb) disable frame-filter ``/build/test'' ``Build Test Program Filter'' >>> >>> Why the quotes? they were never mentioned before. And in any case, >>> use ".." here, not ``..'', as the conversion is not done in @example, >>> AFAIR. >> >> Quotes are needed so that options with spaces are correctly parsed >> (like the name). Quotes are not needed for the first "/build/test" >> option. I will remove those. EMACS is quite insistent on placing >> those quotes in @smallexample. > > You are right about Emacs, but that's an Emacs bug. It doesn't do > that inside @example. > >> In the PDF output I think they are output correctly, but I will fix. > > They certainly don't look right in the Info output. Ok, will fix. >>>> +This feature is currently (as of @value{GDBN} 7.6) experimental, and >>>> +may work differently in future versions of @value{GDBN}. >>> >>> Do we have to have this version-specific note in the manual? They are >>> maintenance burden in the long run. >> >> I added this as the pretty printers feature also has a similar >> statement. I can remove it. > > Please do remove it, version-specific information in the manual is a > maintenance burden. Will do, thanks Cheers Phil