bug#73188: PEG parser does not support full PEG grammar

[Ekaitz Zarraga]

Saluton Ludovic,

On 2024-10-13 22:29, Ludovic Courtès wrote:
> Hi Ekaitz,
>
> Ekaitz Zarraga <ekaitz@elenq.tech> skribis:
>
> > This commit adds support for PEG as described in:
> >
> >      <https://bford.info/pub/lang/peg.pdf>
>
> I would make this a comment below the ‘define-module’ form.

Okay I will add it.

> > It adds support for the missing features (comments, underscores in
> > identifiers and escaping) while keeping the extensions (dashes in
> > identifiers, < and <--).
> >
> > The naming system tries to be as close as possible to the one proposed
> > in the paper.
> >
> > * module/ice-9/peg/string-peg.scm: Rewrite PEG parser.
> > * test-suite/tests/peg.test: Fix import
>
> Nice work!

Thank you

> Questions:
>
>    1. Is the name change for lexical elements (camel case instead of
>       lower-case + hyphens) user-visible?  I guess no but better be safe
>       than sorry.

I think they can be, in a very weird way. If using `peg-as-peg` or 
something they can be used, but the ones coming from the PEG in text, 
which makes way more sense written like in the paper. I'm not sure if 
there's another way to make them available, but I don't think there is.

I exported `Grammar` as `peg-grammar` because of this. So the users 
should just use `peg-grammar` for their things.

>       I have a preference for lower-case + hyphens out of consistency
>       with the rest of Scheme, but I can see how keeping the same names
>       as in the reference material helps.

Yeah, I wasn't sure about it. And I'm still not very sure. But I think 
keeping the same names the paper does helps.

>    2. Could you add tests for the missing features that this adds, and
>       maybe extend ‘api-peg.texi’ accordingly too?

It doesn't really add much new in this first case, but it makes it work 
as expected in PEG, which is what documentation already claimed to do, 
and the code didn't actually implement. Mostly what this commit adds is 
escaping support in the PEG string literals.

>    3. You can choose to assign copyright to the FSF or to not do that¹.
>       In the latter case, please add a copyright line for you where
>       appropriate.

I don't care (maybe I should?). I just want this to work properly.

> I’m really not a PEG expert though so I’d prefer more eyeballs here, but
> I trust your judgment.

I don't know how wise this is :)

> There are three (guix import *) modules that use (ice-9 peg) and that
> come with tests.  It would be nice to check that those tests still pass
> with the modified string-peg.scm.

This is very interesting and I didn't know. All tests from Guile pass, 
but I'll try these and report here.

> Thanks!
>
> Ludo’.
>
> ¹ https://lists.gnu.org/archive/html/guile-devel/2022-10/msg00008.html


Thanks for the review, Ludo.

Next days I plan to review some older patches in the mailing list that 
offer a very straightforward solution for error reporting and try to 
include them with this and the extra features I added.

For the time being, I'll review what you mentioned, rebase the support 
for `[^...]` and resend them together.

I appreciate your time spent here, Ludo. Really.