Subject: Re: bug#19478: [PATCH] Improve SXPath documentation

From: [email-masked] (Ludovic Courtès)
Date: Wed, 04 Mar 2015 10:23:28 +0100

rekado <[email-masked]> skribis:

> Attached is a patch that takes the comments from the sources and adds
> them to the Texinfo sources.  I chose to rewrite a few comments to make
> them a little clearer and added some Texinfo markup, but most of the
> documentation is unchanged from Oleg's comments in the source.

Nice!

> This is the first time I wrote Texinfo documentation (it's not even
> close to being the obstacle to contributing that some people on another
> mailing list make it out to be)

Good to hear.  ;-)

> There is also one instance where I think I did the right thing but the
> results are wrong: I added a link to an example further down the page
> but in my compiled version of the manual I end up far from the anchor
> when I follow the link.
>
> This is the section containing the reference:
>
>     Similarly to XPath, SXPath defines full and abbreviated notations for
>     location paths.  In both cases, the abbreviated notation can be
>     mechanically expanded into the full form by simple rewriting rules.  In
>     case of SXPath the corresponding rules are given in the documentation of
>     the @code{sxpath} procedure.  @xref{sxpath-procedure-docs,,SXPath
>     procedure documentation}.
>     ...
>
> And here's the anchor:
>
>     @anchor{sxpath-procedure-docs}
>     @deffn {Scheme Procedure} sxpath path
>     Evaluate an abbreviated SXPath.
>     ...

I just tried and this seems to be a bug in the Emacs Info reader.  The
standalone Info reader brings you to the right place, and the HTML and
PDF outputs are OK.

Could you report it?

Some superficial comments follow.  I realize some of these may be
present in the original SXPath source, but it seems best to fix them
anyway.

BTW, I think we should add a sentence at the beginning of the “SXPath”
section saying that the material is taken from the SXPath source by Oleg
et al.

>  mechanically expanded into the full form by simple rewriting rules.  In
> -case of SXPath the corresponding rules are given as comments to a sxpath
> -function, below.  The regression test suite at the end of this file shows
> -a representative sample of SXPaths in both notations, juxtaposed with
> -the corresponding XPath expressions.  Most of the samples are borrowed
> +case of SXPath the corresponding rules are given in the documentation of

Missing “the” (“In the case of SXPath”.)

> +The regression test suite at the end of the file SXPATH-old.scm shows a

@file{SXPATH-old.scm}

> +representative sample of SXPaths in both notations, juxtaposed with the
> +corresponding XPath expressions.  Most of the samples are borrowed
>  literally from the XPath specification, while the others are adjusted
> -for our running example, tree1.
> +for our running example, @code{tree1}.

The issue was already there, but apparently ‘tree1’ is not used
elsewhere in the documentation.  Perhaps it’s best to remove it?

> +@subsubsection Basic converters and applicators

Capital C and capital A.

> +A converter is a function mapping a nodeset (or a single node) to another
> +nodeset.  Its type can be represented like this:
> +
> +@smallexample
> +  type Converter = Node|Nodeset -> Nodeset
> +@end smallexample

Rather @example (@smallexample means small typeface) and no space before
‘type’ (same for the other examples below.)

>  @deffn {Scheme Procedure} node-typeof? crit
> +This function implements a 'Node test' as defined in Sec. 2.3 of XPath
> +document.  A node test is one of the components of a location step.  It

Missing “the” (“of the XPath document”.)

> +@subsubsection Converter combinators

Capital C.

> +Combinators are higher-order functions that transmogrify a converter or
> +glue a sequence of converters into a single, non-trivial converter.  The
> +goal is to arrive at converters that correspond to XPath location paths.
> +
> +From a different point of view, a combinator is a fixed, named
> +@emph{pattern} of applying converters.  Given below is a complete set of

When introducing a term, use @dfn; so @dfn{pattern}.

> +@smallexample
> +(define (node-closure f)
> +     (node-or
> +       (select-kids f)
> +       (node-reduce (select-kids (node-typeof? '*)) (node-closure f))))

Align below the ‘e’ of ‘define’.

Could you send an updated patch?

Thanks!

Ludo’.