Skip to content

peg: repeat1-related parsers handle repeated ignores#3171

Open
olus2000 wants to merge 1 commit intofactor:masterfrom
olus2000:peg-hide-in-repeat
Open

peg: repeat1-related parsers handle repeated ignores#3171
olus2000 wants to merge 1 commit intofactor:masterfrom
olus2000:peg-hide-in-repeat

Conversation

@olus2000
Copy link
Copy Markdown
Contributor

Current behavior of repeat0, repeat1 and related parsers is to pass any ignore values generated by the encapsulated parsers in the resulting AST.

This pull request changes this behavior to be similar to the seq family of parsers which will exclude ignores from their output.

Note that the behavior I implemented considers a repeat1 parser which successfully matched its enclosing parser but got only ignores a successful match nevertheless, creating a case where repeat1 may produce an empty sequence AST. In my opinion if the content of repeat1 matched the whole repeat1 should be a success, no matter what the resulting AST is.

@mrjbq7
Copy link
Copy Markdown
Member

mrjbq7 commented Feb 1, 2026

Interesting! Was going to merge it, wanted to think about it for a moment.

@olus2000
Copy link
Copy Markdown
Contributor Author

olus2000 commented Feb 1, 2026

Of course, what I implemented is definitely not the correct solution. If you'd prefer another behavior I'm happy to hear it, especially for the question of repeat1 succeeding with empty output.

The usecase this comes from is lexing a language where comments are allowed between any two tokens. I wanted a way to never include the comments in the tokens in the first place instead of filtering them out after parsing.

@melted
Copy link
Copy Markdown
Contributor

melted commented Feb 1, 2026

I don't have a problem with repeat1 returning an empty output, since the name repeat1 describes what it parses, not what the output is. If the parser it takes doesn't return an output, then I don't think it's a terrible surprise for a user that repeat1 doesn't have anything to pass back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants