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

Several improvements to the Rust lexer #1711

Merged
merged 8 commits into from
Sep 22, 2021
Merged

Conversation

thomcc
Copy link
Contributor

@thomcc thomcc commented Jun 8, 2021

Each improvement is its own commit, and I tried to comment the code fairly well for anything too complex.

Also, it should basically all have tests (perhaps not exhaustive ones where trivial, e.g. I didn't ), except for the keyword additions which seem too simple to be worth testing.

I don't think this breaks highlighting of anything, e.g. I was careful to left the weird pre-Rust-1.0 stuff as I found it, and I'm very confident that the post-1.0 stuff is only more valid than before (I made these changes while consulting https://doc.rust-lang.org/reference/tokens.html, and for some reason, I've written lexers for Rust before).

I also tried to err on the side of accepting invalid code so long as it doesn't cause anything else to parse incorrectly.


P.S. I see that this project supports back to Ruby 2.0, but I tried to test for it, and couldn't follow the dev guide (bundler doesn't work that far back), so I'm somewhat relying on CI to tell me if anything is messed up. (Sorry, I haven't regularly written Ruby in years)

@tancnle tancnle added the needs-review The PR needs to be reviewed label Sep 22, 2021
@tancnle tancnle self-assigned this Sep 22, 2021
@tancnle tancnle self-requested a review September 22, 2021 12:36
@tancnle tancnle removed their assignment Sep 22, 2021
@tancnle
Copy link
Collaborator

tancnle commented Sep 22, 2021

Thanks for your contribution and hard work on this PR @thomcc ❤️. I have added a small commit to fix up the styles in specs. If you are not objecting, this is good to be merged 👍🏼

@tancnle tancnle added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels Sep 22, 2021
@thomcc
Copy link
Contributor Author

thomcc commented Sep 22, 2021

Fine by me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author-action The PR has been reviewed but action by the author is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants