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

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Tue, 24 Dec 2013 11:02:24 -0800
> From: Doug Evans <xdje42@gmail.com>
> 
> 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.

Thanks.

> +Guile version 2.0.9 is well tested, earlier 2.0 versions are not.

I'm not sure we should say this in the manual.  Would you like to keep
track of others testing other versions and update this as they report
to you?  I don't think so.

It might be good for NEWS, though.

> +The implementation uses Guile's @code{smob} (small object)
                                                ^^^^^^^^^^^^
This should be in @dfn, as it's new terminology.  Also, an index entry
about that would be nice.

> +@cindex guile directory

I think "guile scripts directory" is a better index entry.

> +@item guile @r{[}@var{command}@r{]}
> +@itemx gu @r{[}@var{command}@r{]}
> +The @code{guile} command can be used to evaluate a Scheme expression.

If it's a Scheme expression, then why do you use "command" in the
@item? why not "scheme-expression"?

> +See the Guile documentation for a description of this function.

A cross-reference to the appropriate node in the Guile manual would be
good there.

> +Guile's history mechanism uses the same naming as @value{GDBN}'s,
> +namely the user of dollar-variables (e.g., $1, $2, etc.).
> +However, the values are independent, @code{$1} in Guile is not the
> +same value as @code{$1} in @value{GDBN}.

This is not clear enough.  I'm guessing you wanted to say that results
of evaluations in Guile and in GDB are counted separately?  If so, I
suggest to say just that.

> +@value{GDBN} is not thread-safe.  If your Guile program uses multiple
> +threads, you must be careful to only call @value{GDBN}-specific
> +functions in the main @value{GDBN} thread.

What is "the main GDB thread" here?

> +The rest of this manual assumes the @code{gdb} module has been imported
> +without any prefix.  See the Guile documentation for @code{use-modules}
> +for more information.

Again, a cross-reference would be good here.

> +By default, any output produced by @var{command} is sent to
> +@value{GDBN}'s standard output.

What happens if logging has been turned on?

> +@emph{Note:} @value{GDBN}'s value history is independent of Guile's.
> +@code{$1} in @value{GDBN}'s value history is not @code{$1} from Guile's
> +history, nor is the reverse true.

See comment about this above.  Also, which history are you talking
about here: the one from Guile evaluation or the one from GDB?

> +@node Guile Configuration
> +@subsubsection Guile Configuration

An index entry would be nice here.

> +@defun data-directory
> +Return a string containing @value{GDBN}'s data directory.

Should we mention that this string is in UTF-8 (I think) encoding?

> +@defun make-exception key args
> +Return a @code{<gdb:exception>} object.
> +@var{key} and @var{args} are the standard Guile parameters of an exception.
> +See Guile documentation for more information.
   ^^^^^^^^^^^^^^^^^^^^^^^
Cross-reference, please.

> +@defun exception? object
> +Return @code{#t} if @var{object} is a @code{<gdb:exception>} object.

And #f otherwise, I guess.

> +@value{GDBN} does not memoize @code{<gdb:value>} objects.

Why not?

> +Therefore @code{eq?} does not work as expected.
> +However @code{equal?} does work.
> +
> +@smallexample
> +(gdb) guile (eq? (make-value 1) (make-value 1))
> +$1 = #f
> +(gdb) guile (equal? (make-value 1) (make-value 1))
> +$1 = #t
> +@end smallexample

Wouldn't this be confusing for Scheme programmers?  Is it terribly
hard to make eq? work?

> +@defun value? object
> +Return @code{#t} if @var{object} is a @code{<gdb:value>} object.

And #f otherwise?

> +Many Scheme values can be converted directly to a @code{<gdb:value>} via
> +with this procedure.

Either "via" or 'with", but not both.

> +A Scheme boolean is converted to @var{type} if provided, otherwise

You already described how "type" is handled, no need to repeat that
(here and elsewhere in this part).

> +If @var{type} is not provided,
> +a Scheme real is converted to the C @code{double} type for the
> +current architecture.

Isn't Guile built with libgmp?  If so, doesn't it support floats
which, when converted to a double, will lose accuracy?

Also, what about long double, if it is supported by the host?

> +A Scheme string is converted to a target string, using the current
> +target encoding.

What if target encoding doesn't support some of the characters in the
string?

And what does "target string" mean anyway?  A string in C and a string
in Fortran are two different objects, no?

> +If @var{value} is a Scheme bytevector and @var{type} is provided,
> +@var{value} must be the same size, in bytes, of values of type @var{type},
> +and the result is essentially created by using @code{memcpy}.

Can type be 'double' in this case?  If so, what happens with denormals
and such likes that are created by such a memcpy?  Wouldn't it be
better to allow only integer types here?

> +If @var{value} is a Scheme bytevector and @var{type} is not provided,
> +the result is an array of type @code{uint8} of the same length.

I would suggest using 'unsigned char' instead of uint8.

> +A similar function @code{value-referenced-value} exists which also
> +returns @code{<gdb:value>} objects corresonding to the values pointed to
> +by pointer values (and additionally, values referenced by reference
> +values).  However, the behavior of @code{value-dereference}
> +differs from @code{value-referenced-value} by the fact that the
> +behavior of @code{value-dereference} is identical to applying the C
> +unary operator @code{*} on a given value.  For example, consider a
> +reference to a pointer @code{ptrref}, declared in your C@t{++} program
> +as
> +
> +@smallexample
> +typedef int *intptr;
> +...
> +int val = 10;
> +intptr ptr = &val;
> +intptr &ptrref = ptr;
> +@end smallexample

Here and below you describe 2 related functions, and explain how they
are different twice, each explanation is part of its @defun.  This in
effect says the same things twice slightly differently, and is thus
confusing.

Instead, I suggest to provide a concise description of both functions,
and then explain their differences only once.

> +Each element of list @var{arg-list} must be a <gdb:value> object or an object
> +that can be converted to one.
               ^^^^^^^^^^^^^^^^
"converted to a value" is less ambiguous.

> +For C-like languages, a value is a string if it is a pointer to or an
> +array of characters or ints.

"Array of ints" meaning a wchar_t string?  If so, they might not be
ints on some platforms (e.g., Windows).

> +The optional @var{errors} argument is either @code{"strict"}
> +or @code{"replace"}.  A value of @code{"strict"} corresponds to
> +Guile's @code{SCM_FAILED_CONVERSION_ERROR} and a value of @code{"replace"}
> +corresponds to Guile's @code{SCM_FAILED_CONVERSION_QUESTION_MARK}.

Suggest a cross-reference to Guile documentation here.

> +If the optional @var{length} argument is given, the string will be
> +fetched and encoded to the length of characters specified.  If
> +the @var{length} argument is not provided, the string will be fetched
> +and encoded until a null of appropriate width is found.

Isn't this null termination description skewed towards C-like
languages?  Aren't there languages where strings don't have to be
null-terminated?

> +@defun value-max a b
> +@end defun
> +
> +@defun value-max a b
> +@end defun

One of these should be value-min, I presume.

> +@table @code
> +@findex TYPE_CODE_PTR
> +@item TYPE_CODE_PTR

Using @ftable here would have saved you from the need to type @findex
for each @item.

> +@findex TYPE_CODE_INTERNAL_FUNCTION
> +@item TYPE_CODE_INTERNAL_FUNCTION
> +A function internal to @value{GDBN}.  This is the type used to represent
> +convenience functions.

A cross-reference to where convenience functions are described would
be nice here.

> +@anchor{Fields of a Type in Guile}

Why capitalized words?

> +@node Guile Pretty Printing API
> +@subsubsection Guile Pretty Printing API

@cindex missing here.

> +@node Selecting Guile Pretty-Printers
> +@subsubsection Selecting Guile Pretty-Printers

@cindex.

> +For compatibility @samp{"b"} (binary) may also be present,
> +but we ignore it: memory ports are binary only.

Which means strings read from memory cannot be decoded?

> +on can be accessed.  If only @var{size} if specified, all memory in the
> +range [0,@var{size} can be accessed.  If both are specified, all memory
         ^^^^^^^^^^^^^
Missing closing paren.

> +@node Guile Printing Module
> +@subsubsection Guile Printing Module
> +@cindex (gdb printing)

Is this really a useful index entry?

> +@node Guile Types Module
> +@subsubsection Guile Types Module
> +@cindex (gdb types)

Likewise here.

Thanks again for doing this.