Re: [patch][python] 5 of 5 - Frame filter documentation changes

[Eli Zaretskii <eliz@gnu.org>]

> 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?

> >> +@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.