From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15068 invoked by alias); 7 Dec 2012 15:04:45 -0000 Received: (qmail 15059 invoked by uid 22791); 7 Dec 2012 15:04:43 -0000 X-SWARE-Spam-Status: No, hits=-4.0 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_DNSWL_NONE,RCVD_IN_HOSTKARMA_NO,SARE_TOWRITE,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout23.012.net.il (HELO mtaout23.012.net.il) (80.179.55.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 07 Dec 2012 15:04:31 +0000 Received: from conversion-daemon.a-mtaout23.012.net.il by a-mtaout23.012.net.il (HyperSendmail v2007.08) id <0MEO002001TN1X00@a-mtaout23.012.net.il> for gdb-patches@sourceware.org; Fri, 07 Dec 2012 17:04:29 +0200 (IST) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout23.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MEO001SG1VEJU90@a-mtaout23.012.net.il>; Fri, 07 Dec 2012 17:04:27 +0200 (IST) Date: Fri, 07 Dec 2012 15:04:00 -0000 From: Eli Zaretskii Subject: Re: [patch][python] 5 of 5 - Frame filter documentation changes In-reply-to: <50C1FE8A.20909@redhat.com> To: Phil Muldoon Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83624doqdz.fsf@gnu.org> References: <50B8C35F.9040000@redhat.com> <83boeet4wh.fsf@gnu.org> <50C1FE8A.20909@redhat.com> 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/msg00174.txt.bz2 > 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? > >> +@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. If that's not what this talks about, then please describe (in a mail message) what they are as best you can, and I will suggest how to reword it for the manual, if needed. > I took the notion that if a GDB Python scriptwriter is writing a > frame filter, the concept of symbols and values should be assumed to > be known. I agree, but that's not what was bugging me when I read that section. > >> 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. What _is_ important is to give the reader a way to become acquainted with how to write this stuff by presenting the material in the order that makes it easy to grasp it. For that, the reader must first have some introduction and overview of the subject, and then the details. The way you arranged the material, the overview is buried in the cookbook section, which is not the right place. OTOH, the APIs are described with only a minimal introduction, which leaves a lot in the fog. With the overview moved to the beginning, I'm quite sure it will be easier to understand how the pieces of this puzzle fit together. If you want to re-iterate some of the overview in the cookbook section, you can either repeat that or, better, suggest that the reader re-reads that, and give a cross-reference. Finally, if you bother about having an overview in a section named "API", you could have a section that starts with an overview, then 2 subsections, each describing one of the 2 APIs you need to describe, and finally a 3rd subsection with the cookbook. The principle is to arrange the text for someone who reads all of these sections in sequence, when they first learn about these features. Afterwards, the readers are supposed to get to the stuff they want via the index and the cross-references, so the arrangement of text is less important for those readers. > >> +(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. > >> +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. Thanks.