Re: [PATCH v2 02/36] Guile extension language: doc additions

[Doug Evans <xdje42 at gmail dot com>]

On Tue, Jan 21, 2014 at 1:11 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Doug Evans <xdje42@gmail.com> skribis:
>
>> On Mon, Jan 20, 2014 at 7:37 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>>> Date: Mon, 20 Jan 2014 14:00:03 -0800
>>>> From: Doug Evans <xdje42@gmail.com>
>>>> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>>>>
>>>> On Mon, Jan 20, 2014 at 1:52 PM, Doug Evans <xdje42@gmail.com> wrote:
>>>> > This patch has the doc additions.
>>>> > There's still some to be written, but I think it's in good enough shape
>>>> > to get feedback on.
>>>> >
>>>> > Changes from v1:
>>>> > - NEWS file added
>>>> > - guile.texi updated based on feedback
>>>> > - new section on gdb smobs
>>>> > - add more text to "Iterators In Guile" node
>>>> >
>>>> > 2014-01-20  Doug Evans  <xdje42@gmail.com>
>>>> >
>>>> >         * NEWS: Mention Guile scripting.
>>>> >
>>>> >         doc/
>>>> >         * Makefile.in (GDB_DOC_FILES): Add guile.texi.
>>>> >         * gdb.texinfo
>>>> >         * guile.texi: New file.
>
> Overall looks good to me.
>
>> ++The implementation uses Guile's @dfn{smob} (small object)
>> ++data type for all @value{GDBN} objects
>> ++(@pxref{Defining New Types (Smobs),,, guile, GNU Guile Reference Manual}).
>> ++This allows gluing Guile into @value{GDBN} very easily.
>
> SMOBs are a mechanism to define new Scheme data types, rather than a
> data type in itself.
>
> More importantly, I’m not sure whether it’s worth mentioning it here.
> What about replacing these two sentence with something like
> “@value{GDBN} defines several Scheme data types.”?

"works for me"

>> ++@node GDB Smobs
>> ++@subsubsection GDB Smobs
>> ++@cindex gdb smobs
>
> And definitely, the heading should be “GDB Scheme Data Types” or
> similar, IMO.

Done.

>> ++@value{GDBN} uses Guile's @dfn{smob} (small object)
>> ++data type for all @value{GDBN} objects
>> ++(@pxref{Defining New Types (Smobs),,, guile, GNU Guile Reference Manual}).
>> ++The smobs that @value{GDBN} provides are called @dfn{gmobs}.
>
> That one is OK since it helps introduce “gmob”.  ;-)
>
>> ++Every @code{gsmob} provides a common set of functions for extending
>> ++them in simple ways.  Each @code{gsmob} has a list of properties,
>> ++initially empty.  These properties are akin to Guile's object properties,
>> ++but are stored with the @code{gsmob}
>
> Perhaps add “for improved efficiency” (I assume that’s the reason,
> right?).
>
>>  +(define iter (make-iterator my-list my-list
>>  +  (lambda (iter)
>
> The indentation is wrong here (put “(make-iterator” on the next line.)
>
>>  +@smallexample
>> -+(use-modules (gdb experimental))
>> ++(use-modules (gdb iterator))
>>  +(define this-sal (find-pc-line (frame-pc (selected-frame))))
>>  +(define this-symtab (sal-symtab this-sal))
>>  +(define this-global-block (symtab-global-block this-symtab))
>>  +(define syms-iter (make-block-symbols-iterator this-global-block))
>> -+(define functions (iterator-filter symbol-function? syms-iter #f))
>> ++(define functions (iterator-filter symbol-function? syms-iter))
>
> (The obvious ‘iterator->list’ procedure would be nice.)
>
>> ++@deffn {Scheme Procedure} iterator-map proc iterator
>> ++Return a list of @var{proc} applied to each element of @var{iterator}.
>
> What about:
>
>   Return the list of objects obtained by applying @var{proc} to the object
>   pointed to by @var{iterator} and to each subsequent object.
>
>> ++@deffn {Scheme Procedure} iterator-filter pred iterator
>> ++Return as a list the elements of @var{iterator} that satisfy @var{pred}.
>
>   Return the list of elements pointed to by @var{iterator} that satisfy
>   @var{pred}.

Done.

> Thanks!
>
> Ludo’.