This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch][python] 5 of 5 - Frame filter documentation changes
- From: Phil Muldoon <pmuldoon at redhat dot com>
- To: Eli Zaretskii <eliz at gnu dot org>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 07 Dec 2012 15:33:47 +0000
- 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>
On 12/07/2012 03:04 PM, Eli Zaretskii wrote:
>> Date: Fri, 07 Dec 2012 14:34:50 +0000
>> From: Phil Muldoon <pmuldoon@redhat.com>
>> 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