Skip to content

lexer: Skip over unrecognized characters and only return None for EOF#591

Merged
OmarTawfik merged 2 commits intoNomicFoundation:mainfrom
Xanewok:lexer-skip-unrecognized
Sep 6, 2023
Merged

lexer: Skip over unrecognized characters and only return None for EOF#591
OmarTawfik merged 2 commits intoNomicFoundation:mainfrom
Xanewok:lexer-skip-unrecognized

Conversation

@Xanewok
Copy link
Copy Markdown
Contributor

@Xanewok Xanewok commented Sep 4, 2023

Does what it says on the tin.

Helps recover past unrecognized characters while still stopping at EOF, see test change in a9777a5 (#586).

cc @AntonyBlakey since we talked about this

@Xanewok Xanewok requested a review from a team as a code owner September 4, 2023 21:06
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Sep 4, 2023 •
edited
Loading

⚠️ No Changeset found

Latest commit: 8e1505e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Xanewok Xanewok force-pushed the lexer-skip-unrecognized branch from 3944897 to 0b2368e Compare September 4, 2023 21:07
@OmarTawfik OmarTawfik added this pull request to the merge queue Sep 6, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 6, 2023
@OmarTawfik OmarTawfik enabled auto-merge September 6, 2023 10:37
@OmarTawfik
Copy link
Copy Markdown
Contributor

OmarTawfik commented Sep 6, 2023

@Xanewok looks like it needs rebasing. But otherwise, LGTM.

@Xanewok
Copy link
Copy Markdown
Contributor Author

Xanewok commented Sep 6, 2023

Updated with a merge to facilitate re-review since we're already squashing on final merge, hope that's okay.

@OmarTawfik OmarTawfik added this pull request to the merge queue Sep 6, 2023
Merged via the queue into NomicFoundation:main with commit 97e5ebd Sep 6, 2023
@OmarTawfik
Copy link
Copy Markdown
Contributor

OmarTawfik commented Sep 8, 2023

Updated with a merge to facilitate re-review since we're already squashing on final merge, hope that's okay.

@Xanewok I think that should be fine. Thanks!

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

Reviewers

@OmarTawfik OmarTawfik OmarTawfik approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Xanewok @OmarTawfik