Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Eli Zaretskii]

> branch: master
> commit 8bee4060ea42c61e52ebe6487ff97bc095261050
> Author: Eric Abrahamsen <eric@ericabrahamsen.net>
> Commit: Eric Abrahamsen <eric@ericabrahamsen.net>
> 
>     Add peg.el as a built-in library
>     
>     * lisp/progmodes/peg.el: New file, taken from ELPA package.
>     * test/lisp/peg-tests.el: Package tests.
>     * doc/lispref/peg.texi: Documentation.

Thanks.  A few comments about this:

I fixed the manual additions, but there are a few FIXME's there;
please take care of them.

The doc string of define-peg-rule doesn't reference a mandatory
argument ARGS.  It also doesn't tell what happens if PEXS is omitted
or nil.

The doc string of define-peg-rule doesn't say how the NAME argument is
used.

The doc string of with-peg-rules doesn't explain the meaning of NAME
in each rule.

Some public functions in peg.el lack doc strings, and others don't
mention their arguments in the first line of the doc string.

Last, but not least: the test suite fails to compile, even after I
moved it to the correct subdirectory of test/:

    ELC      lisp/progmodes/peg-tests.elc

  In toplevel form:
  lisp/progmodes/peg-tests.el: Warning: Possible left recursion: r -> r
  lisp/progmodes/peg-tests.el: Warning: `let' with empty body
  lisp/progmodes/peg-tests.el: Warning: `let' with empty body

  In peg-ex-arith:
  lisp/progmodes/peg-tests.el:210:2: Warning: `peg-rule eol' called with 1 argument, but accepts only 0
  lisp/progmodes/peg-tests.el:212:10: Warning: reference to free variable `_'
  lisp/progmodes/peg-tests.el:212:12: Warning: reference to free variable `sum'
  lisp/progmodes/peg-tests.el:212:16: Warning: reference to free variable `eol'
  lisp/progmodes/peg-tests.el:213:9: Warning: reference to free variable `product'
  lisp/progmodes/peg-tests.el:215:13: Warning: reference to free variable `value'
  lisp/progmodes/peg-tests.el:217:21: Warning: value from call to `substring' is unused
  lisp/progmodes/peg-tests.el:217:31: Warning: reference to free variable `number'
  lisp/progmodes/peg-tests.el: Error: Too many arguments for inlined function (#[0 "l\207" [((guard (eolp)))] 1 peg-function] "
  ")

  In peg-ex-uri:
  lisp/progmodes/peg-tests.el:229:23: Warning: reference to free variable `absoluteURI'
  lisp/progmodes/peg-tests.el:229:35: Warning: reference to free variable `relativeURI'
  lisp/progmodes/peg-tests.el:230:43: Warning: reference to free variable `fragment'
  lisp/progmodes/peg-tests.el:237:28: Warning: reference to free variable `scheme'
  lisp/progmodes/peg-tests.el:237:44: Warning: reference to free variable `hier-part'
  lisp/progmodes/peg-tests.el:237:54: Warning: reference to free variable `opaque-part'
  lisp/progmodes/peg-tests.el:239:9: Warning: reference to free variable `net-path'

  lisp/progmodes/peg-tests.el:241:14: Warning: reference to free variable `abs-path'
  lisp/progmodes/peg-tests.el:242:29: Warning: reference to free variable `query'
  lisp/progmodes/peg-tests.el:244:19: Warning: reference to free variable `authority'
  lisp/progmodes/peg-tests.el:245:18: Warning: reference to free variable `path-segments'
  lisp/progmodes/peg-tests.el:246:19: Warning: reference to free variable `segment'

  lisp/progmodes/peg-tests.el:247:27: Warning: reference to free variable `pchar'
  lisp/progmodes/peg-tests.el:247:41: Warning: reference to free variable `param'
  lisp/progmodes/peg-tests.el:249:15: Warning: reference to free variable `unreserved'
  lisp/progmodes/peg-tests.el:249:26: Warning: reference to free variable `escaped'

  lisp/progmodes/peg-tests.el:250:14: Warning: reference to free variable `uric'
  lisp/progmodes/peg-tests.el:252:39: Warning: reference to free variable `rel-path'
  lisp/progmodes/peg-tests.el:253:14: Warning: reference to free variable `rel-segment'
  lisp/progmodes/peg-tests.el:255:19: Warning: reference to free variable `server'
  lisp/progmodes/peg-tests.el:255:26: Warning: reference to free variable `reg-name'
  lisp/progmodes/peg-tests.el:256:31: Warning: value from call to `substring' is unused
  lisp/progmodes/peg-tests.el:256:41: Warning: reference to free variable `userinfo'
  lisp/progmodes/peg-tests.el:258:21: Warning: reference to free variable `hostport'
  lisp/progmodes/peg-tests.el:261:25: Warning: reference to free variable `host'
  lisp/progmodes/peg-tests.el:261:55: Warning: reference to free variable `port'
  lisp/progmodes/peg-tests.el:263:14: Warning: reference to free variable `hostname'
  lisp/progmodes/peg-tests.el:263:23: Warning: reference to free variable `ipv4address'
  lisp/progmodes/peg-tests.el:264:17: Warning: reference to free variable `domainlabel'
  lisp/progmodes/peg-tests.el:264:34: Warning: reference to free variable `toplabel'
  lisp/progmodes/peg-tests.el:265:17: Warning: reference to free variable `alphanum'
  lisp/progmodes/peg-tests.el:268:14: Warning: reference to free variable `alpha'
  lisp/progmodes/peg-tests.el:271:20: Warning: reference to free variable `digit'
  lisp/progmodes/peg-tests.el:275:17: Warning: reference to free variable `uric-no-slash'
  lisp/progmodes/peg-tests.el:276:14: Warning: reference to free variable `reserved'
  lisp/progmodes/peg-tests.el:278:15: Warning: `set' called with 1 arg, but requires 2
  lisp/progmodes/peg-tests.el:278:15: Warning: `set' called with 1 argument, but requires 2
  lisp/progmodes/peg-tests.el:279:29: Warning: reference to free variable `mark'
  lisp/progmodes/peg-tests.el:280:17: Warning: reference to free variable `hex'
  lisp/progmodes/peg-tests.el:282:11: Warning: `set' called with 1 arg, but requires 2
  lisp/progmodes/peg-tests.el:282:11: Warning: `set' called with 1 argument, but requires 2
  lisp/progmodes/peg-tests.el:284:15: Warning: reference to free variable `lowalpha'
  lisp/progmodes/peg-tests.el:284:24: Warning: reference to free variable `upalpha'


  In peg-ex-lisp:
  lisp/progmodes/peg-tests.el:305:10: Warning: reference to free variable `_'
  lisp/progmodes/peg-tests.el:305:16: Warning: reference to free variable `string'
  lisp/progmodes/peg-tests.el:305:23: Warning: reference to free variable `list'
  lisp/progmodes/peg-tests.el:305:28: Warning: reference to free variable `number'
  lisp/progmodes/peg-tests.el:305:35: Warning: reference to free variable `symbol'
  lisp/progmodes/peg-tests.el:309:29: Warning: `set' called with 1 arg, but requires 2
  lisp/progmodes/peg-tests.el:309:29: Warning: `set' called with 1 argument, but requires 2
  lisp/progmodes/peg-tests.el:309:43: Warning: reference to free variable `digit'
  lisp/progmodes/peg-tests.el:310:16: Warning: reference to free variable `terminating'
  lisp/progmodes/peg-tests.el:312:28: Warning: reference to free variable `symchar'

  lisp/progmodes/peg-tests.el:319:22: Warning: `set' called with 1 arg, but requires 2
  lisp/progmodes/peg-tests.el:319:22: Warning: `set' called with 1 argument, but requires 2

  In end of data:
  lisp/progmodes/peg-tests.el:319:42: Warning: the function `eob' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule terminating' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule symchar' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule symbol' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule string' is not known to be defined.
  lisp/progmodes/peg-tests.el:307:43: Warning: the function `any' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule comment' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule sexp' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule digit' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule upalpha' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule lowalpha' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule alpha' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule alphanum' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule mark' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule hex' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule escaped' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule unreserved' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule reserved' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule uric-no-slash' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule uric' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule opaque-part' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule reg-name' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule scheme' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule port' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule ipv4address' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule toplabel' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule domainlabel' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule hostname' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule host' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule hostport' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule userinfo' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule server' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule authority' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule rel-segment' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule rel-path' is not known to be defined.
  lisp/progmodes/peg-tests.el:252:50: Warning: the function `opt' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule relativeURI' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule fragment' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule query' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule pchar' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule param' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule segment' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule path-segments' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule abs-path' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule net-path' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule hier-part' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule absoluteURI' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule URI-reference' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule _' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule number' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule value' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule product' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule sum' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule expr' is not known to be defined.
  lisp/progmodes/peg-tests.el: Warning: the function `peg-rule other' is not known to be defined.
  Makefile:163: recipe for target `lisp/progmodes/peg-tests.elc' failed
  make[1]: *** [lisp/progmodes/peg-tests.elc] Error 1

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Eric Abrahamsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> branch: master
>> commit 8bee4060ea42c61e52ebe6487ff97bc095261050
>> Author: Eric Abrahamsen <eric@ericabrahamsen.net>
>> Commit: Eric Abrahamsen <eric@ericabrahamsen.net>
>> 
>>     Add peg.el as a built-in library
>>     
>>     * lisp/progmodes/peg.el: New file, taken from ELPA package.
>>     * test/lisp/peg-tests.el: Package tests.
>>     * doc/lispref/peg.texi: Documentation.
>
> Thanks.  A few comments about this:

Thanks for the all the notes. I should have this sorted out later today.

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Eric Abrahamsen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> branch: master
>>> commit 8bee4060ea42c61e52ebe6487ff97bc095261050
>>> Author: Eric Abrahamsen <eric@ericabrahamsen.net>
>>> Commit: Eric Abrahamsen <eric@ericabrahamsen.net>
>>> 
>>>     Add peg.el as a built-in library
>>>     
>>>     * lisp/progmodes/peg.el: New file, taken from ELPA package.
>>>     * test/lisp/peg-tests.el: Package tests.
>>>     * doc/lispref/peg.texi: Documentation.
>>
>> Thanks.  A few comments about this:
>
> Thanks for the all the notes. I should have this sorted out later today.

Hi Stefan, I'm still working on integrating peg.el in to Emacs, and am
not having any luck getting peg-tests.el to compile. Actually, any use
of `peg-parse', compiled or interactive, seems to fail. Say you've got
this simple example from the manual:

(peg-parse
  (number sign digit (* digit))
  (sign   (or "+" "-" ""))
  (digit  [0-9]))

Inside `peg-parse', the only way this code is going to work correctly is
if it takes the `with-peg-rules' branch, because these pexs aren't
defined as rules yet. But the call to `peg-normalize' always succeeds,
so we always take the `peg-run' branch, and the code fails on undefined
rules.

(defmacro peg-parse (&rest pexs)
  (if (and (consp (car pexs))
           (symbolp (caar pexs))
           (not (ignore-errors (peg-normalize (car pexs)))))
      ;; `pexs' is a list of rules: use the first rule as entry point.
      `(with-peg-rules ,pexs (peg-run (peg ,(caar pexs)) #'peg-signal-failure))
    `(peg-run (peg ,@pexs) #'peg-signal-failure)))

`peg-normalize' called on a cons applies `peg--macroexpand' to the list,
which calls its "head" implementation, and we end up with

(call number sign digit (* digit))

`peg' sees that and assumes this has been defined as a rule.

So `peg-normalize' should be failing on a list like this, but I don't
know how, exactly.

I'm hoping you have some pointers!

Thanks,
Eric

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Eli Zaretskii]

Ping!  Eric, any progress with this?

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Date: Tue, 02 Apr 2024 20:03:05 -0700
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>
> 
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> branch: master
> >>> commit 8bee4060ea42c61e52ebe6487ff97bc095261050
> >>> Author: Eric Abrahamsen <eric@ericabrahamsen.net>
> >>> Commit: Eric Abrahamsen <eric@ericabrahamsen.net>
> >>> 
> >>>     Add peg.el as a built-in library
> >>>     
> >>>     * lisp/progmodes/peg.el: New file, taken from ELPA package.
> >>>     * test/lisp/peg-tests.el: Package tests.
> >>>     * doc/lispref/peg.texi: Documentation.
> >>
> >> Thanks.  A few comments about this:
> >
> > Thanks for the all the notes. I should have this sorted out later today.
> 
> Hi Stefan, I'm still working on integrating peg.el in to Emacs, and am
> not having any luck getting peg-tests.el to compile. Actually, any use
> of `peg-parse', compiled or interactive, seems to fail. Say you've got
> this simple example from the manual:
> 
> (peg-parse
>   (number sign digit (* digit))
>   (sign   (or "+" "-" ""))
>   (digit  [0-9]))
> 
> Inside `peg-parse', the only way this code is going to work correctly is
> if it takes the `with-peg-rules' branch, because these pexs aren't
> defined as rules yet. But the call to `peg-normalize' always succeeds,
> so we always take the `peg-run' branch, and the code fails on undefined
> rules.
> 
> (defmacro peg-parse (&rest pexs)
>   (if (and (consp (car pexs))
>            (symbolp (caar pexs))
>            (not (ignore-errors (peg-normalize (car pexs)))))
>       ;; `pexs' is a list of rules: use the first rule as entry point.
>       `(with-peg-rules ,pexs (peg-run (peg ,(caar pexs)) #'peg-signal-failure))
>     `(peg-run (peg ,@pexs) #'peg-signal-failure)))
> 
> `peg-normalize' called on a cons applies `peg--macroexpand' to the list,
> which calls its "head" implementation, and we end up with
> 
> (call number sign digit (* digit))
> 
> `peg' sees that and assumes this has been defined as a rule.
> 
> So `peg-normalize' should be failing on a list like this, but I don't
> know how, exactly.
> 
> I'm hoping you have some pointers!
> 
> Thanks,
> Eric
> 
> 
> 

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Stefan Monnier]

BTW, why is it in `lisp/progmodes`?
It seems just as likely to be used to parse some network protocol like
IMAP as anything else.  I'd put it in `lisp/emacs-lisp` or just `lisp`


        Stefan

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eric Abrahamsen <eric@ericabrahamsen.net>,  emacs-devel@gnu.org
> Date: Sat, 13 Apr 2024 14:00:40 -0400
> 
> BTW, why is it in `lisp/progmodes`?

Not my decision, but OTOH I don't see that as a significant problem.
We have a few other files there that are not necessarily "for
programs".

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Stefan Monnier]

> Ping!  Eric, any progress with this?

I fixed the test failures.


        Stefan

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eric Abrahamsen <eric@ericabrahamsen.net>,  emacs-devel@gnu.org
> Date: Sat, 13 Apr 2024 15:36:14 -0400
> 
> > Ping!  Eric, any progress with this?
> 
> I fixed the test failures.

Thanks.

Eric, this leaves the documentation-related comments in my message
from 2 weeks ago; could you please take care of those?

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Eric Abrahamsen]

On 04/14/24 07:58 AM, Eli Zaretskii wrote:
>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Eric Abrahamsen <eric@ericabrahamsen.net>,  emacs-devel@gnu.org
>> Date: Sat, 13 Apr 2024 15:36:14 -0400
>> 
>> > Ping!  Eric, any progress with this?
>> 
>> I fixed the test failures.
>
> Thanks.
>
> Eric, this leaves the documentation-related comments in my message
> from 2 weeks ago; could you please take care of those?

Done! Thank you for the push, and thanks to Stefan for fixing the tests.

I hope the docs read a bit better now -- I've also removed the mention
that `peg-parse' could also accept a single list of PEXS, as I don't
think it can any more (and I don't think it needs to be *so* flexible as
all that).

Eric

Re: master 8bee4060ea4 2/2: Add peg.el as a built-in library

[Eli Zaretskii]

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
> Date: Mon, 15 Apr 2024 20:18:56 -0700
> 
> 
> On 04/14/24 07:58 AM, Eli Zaretskii wrote:
> >> From: Stefan Monnier <monnier@iro.umontreal.ca>
> >> Cc: Eric Abrahamsen <eric@ericabrahamsen.net>,  emacs-devel@gnu.org
> >> Date: Sat, 13 Apr 2024 15:36:14 -0400
> >> 
> >> > Ping!  Eric, any progress with this?
> >> 
> >> I fixed the test failures.
> >
> > Thanks.
> >
> > Eric, this leaves the documentation-related comments in my message
> > from 2 weeks ago; could you please take care of those?
> 
> Done! Thank you for the push, and thanks to Stefan for fixing the tests.
> 
> I hope the docs read a bit better now -- I've also removed the mention
> that `peg-parse' could also accept a single list of PEXS, as I don't
> think it can any more (and I don't think it needs to be *so* flexible as
> all that).

Thanks, but it looks like you made some editing mistake, which deleted
too much of the text, so the commit was reverted.  Please take a look
and fix as needed; I tried to figure out what you wanted to do, but
couldn't, presumably because too much of the text was inadvertently
lost when you installed the changes.