Parse optionals and repeats without regexes #1826
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The motivation for this refactoring is to next add repeats with separators. |
rustbot
added
the
S-waiting-on-review
traviscross
force-pushed
the
TC/parse-repeats-without-regex
branch
from
747ca2f to
7e7e973
Compare
Rather than parsing optionals and repeats fully in the recursive
descent style, we were using regular expressions to do part of the
matching and parsing. That's fine for what it is, but as we think
about extending the grammar language surrounding repeats further, it
might be more straightforward for this to be parsed in the more usual
way. So let's do that. Doing this also results in better and more
targeted errors when parsing malformed syntax.
We had been supporting a space between an expression and the optional
and repeat sigils `?`, `*`, and `+` (but not between an expression and
the `{a..b}` ranged repeat syntax). In making this change, we drop
this support and adjust the affected productions. We were only using
this in a handful of places, and the clarity of the productions seem
the same or better by removing these spaces.
We verified that, setting aside the removal of these spaces, the
rendered output of the Reference is byte identical before and after
this change.
traviscross
force-pushed
the
TC/parse-repeats-without-regex
branch
from
7e7e973 to
3570070
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rather than parsing optionals and repeats fully in the recursive descent style, we were using regular expressions to do part of the matching and parsing. That's fine for what it is, but as we think about extending the grammar language surrounding repeats further, it might be more straightforward for this to be parsed in the more usual way. So let's do that. Doing this also results in better and more targeted errors when parsing malformed syntax.
We had been supporting a space between an expression and the optional and repeat sigils
?,*, and+(but not between an expression and the{a..b}ranged repeat syntax). In making this change, we drop this support and adjust the affected productions. We were only using this in a handful of places, and the clarity of the productions seem the same or better by removing these spaces.We verified that, setting aside the removal of these spaces, the rendered output of the Reference is byte identical before and after this change.
cc @ehuss