Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

README.md and website example grammar doesn't handle leading digit for all idents in ident_list #551

Closed
SeedyROM opened this issue Oct 18, 2021 · 2 comments · Fixed by #728

Comments

@SeedyROM
Copy link

The current example grammar will allow for every ident after the first to start with a digit.

For instance "a1 2b" is a valid ident_list with the current grammar.

Wouldn't this make sense for the ident rule?

...
ident = { !digit ~ (alpha | digit)+ }

ident_list = _{ ident ~ (" " ~ ident)+ }

I'm new to PEG style grammars so maybe I'm wrong?

@Alextopher
Copy link
Contributor

Alextopher commented Aug 3, 2022

Commenting since it's a "good-first-issue".

The issue is correct the example grammar does allow every ident after the first to start with a digit. The text above the example in the REAME says as much (both Today and in Oct 2021).

The following is an example of a grammar for a list of alpha-numeric identifiers where the first identifier does not start with a digit:

So I believe this issue may be closed with wont-fix.

However, I don't think the confusion is unwarranted, I can not think of a time I wanted an "identifier list but the first identifier is not like the others." Looking at git blame I can see at some point in the past the example matched the suggested grammar.

I also recall discovering https://pest.rs, copying the example into the Editor, and feeling confused as to why a _ did not return an error while _ does. After reading the book I understand it's the lack of any EOI requirement.

Also, if we are switching things around like this then

ident_list = _{ ident ~ (" " ~ ident)+ }

should be

ident_list = _{ ident ~ (" " ~ ident)* }
                                     ^

If there is interest in updating the example I would be willing to create a draft PR. Otherwise, I think this issue should be closed.

@tomtau
Copy link
Contributor

tomtau commented Aug 5, 2022

@Alextopher feel free to open PRs to improve the examples in README here and the book / site repositories.

@tomtau tomtau linked a pull request Oct 31, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants