Re: [PATCH] Re: Make peg.el a built-in library?

[Eli Zaretskii <eliz@gnu.org>]

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Cc: emacs-devel@gnu.org
> Date: Sat, 26 Nov 2022 17:46:04 -0800
> 
> Here's a new version, that I hope clarifies these questions (instead of
> doing the opposite).

Thanks, a few minor comments below.

> Lastly, nobody with a maintainer's hat on has actually given the green
> light on this, and I assume we'll want to hold off until the next
> version of Emacs is released; anyway it would be good to know what
> Eli/Lars think. I haven't done any NEWS additions or anything, either.

What exactly are you asking about here?

> @c -*-texinfo-*-
> @c This is part of the GNU Emacs Lisp Reference Manual.

This would mean a suitable change to elisp.texi at the least, and probably
also to another file that is part of the ELisp reference manual sources?

> A @acronym{PEG} parser is defined as a list of named rules, each of
> which match text patterns, and/or contain references to other rules.
        ^^^^^                       ^^^^^^^
"matches" and "contains", in singular.

> Parsing is initiated with the function @code{peg-run} or the macro
> @code{peg-parse}, and parses text after point in the current buffer,
> using a given set of rules.

This function and this macro need to be formally documented with @defun and
@defmac, as we do elsewhere in the ELisp reference.

> The definition of each rule is referred to as a @dfn{parsing
> expression} (@acronym{PEX}), and can consist of a literal string, a

Ideally, each @dfn in the manual should have a @cindex entry, because people
are likely to look up these terms.

> Or set as the value of a variable, and the variable used in a
> combination of calls to @code{with-peg-rules} and @code{peg-run},
> where the ``entry-point'' rule is given explicitly:

This sentence reads awkwardly, because it starts with "Or set".  Suggest to
rephrase:

  Alternatively, use a variable whose value is a grammar, and use it in a
  combination of calls to...

> @example
> (defvar number-grammar
>         '((number sign digit (* digit))
>           (sign (or "+" "-" ""))
>           (digit [0-9])))

Btw, this begs a question: how come the value of the variable is a (quoted)
list, but the value you pass to peg-parse in the previous example was not
quoted?

> By default, calls to @code{peg-run} or @code{peg-parse} produce no
> output: parsing simply moves point.  In order to return or otherwise
> act upon parsed strings, rules can include @dfn{actions}, see
> @xref{Parsing Actions} for more information.

Again, a @cindex for "actions" is in order here.

Also, @xref produces a Capitalized "See", so you want a @ref here, not
@xref.  And please always follow the closing brace of a cross-reference with
a period or a comma, because some versions of Texinfo insist on that.  (The
only exception from this rule is @pxref inside parentheses.)

> Individual rules can also be defined using a more @code{defun}-like
> syntax, using the macro @code{define-peg-rule}:
> 
> @example
> (define-peg-rule digit ()
>   [0-9])
> @end example

define-peg-rule should be documented with a @defmac.

> @node PEX Definitions
> @section PEX Definitions

There should be a @menu in the parent @chapter's node for all the child
@section nodes.  Otherwise, makeinfo will barf.

> @item "abc"
> A literal string.

You don't mean "abc" literally here, do you?  The correct way of expressing
"a string" is

  @item @var{string}

> @item (char C)
> A single character, as an Elisp character literal.

Likewise here:

  @item @var{C}
  A single character @var{C}, as a Lisp character literal.

> @item (* E)
> Zero or more of an expression, as the regexp ``*''.  Matching is
> always ``greedy''.

Likewise.  Basically, all the elements here are meta-syntactic variables:
they stand for something else.  The right markup for them is @var.

Also, "zero or more of an expression" reads awkwardly.  I don't even think I
understand what you mean.

And please quote regexps using @samp, not literal quotes (here and
elsewhere).

> @item (+ E)
> One or more of an expression, as the regexp ``+''.  Matching is always
> ``greedy''.

Likewise about "one or more of an expression".

> @item (opt E)
> Zero or one of an expression, as the regexp ``?''.

Same.

> @item (range A B)
> The character range between A and B, as the regexp ``[A-B]''.

It is better to use CH1 and CH2 instead of A and B.

> @item [a-b "+*" ?x]
> A character set, including ranges, literal characters, or strings of
> characters.

Same comment about a and b.

> @vindex peg-char-classes
> Named character classes include the following:

Instead of listing them, just use a cross-reference to the node where
classes are documented as part of regexp syntax.

> The first action pushes the initial value of point to the stack.  The
> intervening @acronym{PEX} moves point over the next word. The second
                                                          ^^
Two spaces there.

> action pops the previous value from the stack (binding it to the
> variable @code{start}), and uses that value to extract a substring
> from the buffer and push it to the stack.  This pattern is so common
> that peg.el provides a shorthand function that does exactly the above,
       ^^^^^^
@file{peg.el}.  Or maybe just @acronym{PEG}.

> @item (substring E)
> Match @acronym{PEX} E and push the matched string to the stack.

Same comments here regarding @var markup of meta-syntactic variables.

> @item (replace E "repl")
> Match E and replaced the matched region with the string "repl".

"repl" is not a literal string, it's a meta-syntactic variable, just like E.

Finally, this needs a lot of index entries to make it a useful reference
that is easily looked up for stuff.

Thanks.