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.

Sign up for GitHub

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

Jump to bottom

refactor(repl): use SWC lexer to highlight and validate #8496

Merged
merged 9 commits into from
Nov 27, 2020
Merged

refactor(repl): use SWC lexer to highlight and validate #8496

merged 9 commits into from
Nov 27, 2020

Conversation

Liamolucko
Copy link
Contributor

@Liamolucko Liamolucko commented Nov 26, 2020 •
edited
Loading

This more or less just swaps out manual parsing for the SWC lexer.

Fixes #8160. Fixes #8240.

@Liamolucko
Copy link
Contributor Author

That was weird, tools/lint.js wasn't showing any issues for some reason.

bartlomieju
bartlomieju reviewed Nov 26, 2020
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caspervonb please take a look

cli/tools/repl.rs Outdated Show resolved Hide resolved
cli/Cargo.toml Outdated Show resolved Hide resolved
cli/tools/repl.rs Outdated Show resolved Hide resolved
caspervonb
caspervonb reviewed Nov 27, 2020
View reviewed changes
cli/tools/repl.rs Outdated Show resolved Hide resolved
caspervonb
caspervonb reviewed Nov 27, 2020
View reviewed changes
cli/tools/repl.rs Outdated Show resolved Hide resolved
caspervonb
caspervonb approved these changes Nov 27, 2020
View reviewed changes
Copy link
Contributor

@caspervonb caspervonb left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, clean implementation 👍

caspervonb
caspervonb suggested changes Nov 27, 2020
View reviewed changes
Copy link
Contributor

@caspervonb caspervonb left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, approved a bit too soon, seems that big integer literals aren't accounted for here in the highlighter, e.g 0n is white.

@Liamolucko
Copy link
Contributor Author

Whoops, that's fixed now.

bartlomieju
bartlomieju reviewed Nov 27, 2020
View reviewed changes
Cargo.lock Outdated
Comment on lines 2353 to 2355
version = "0.8.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2940c75beb4e3bf3a494cef919a747a2cb81e52571e212bfbd185074add7208a"
checksum = "8ddb1139b5353f96e429e1a5e19fbaf663bddedaa06d1dbd49f82e352601209a"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What caused this update?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted this change.

@bartlomieju bartlomieju changed the title feat(cli/repl): use SWC to highlight/validate REPL refactor(repl): use SWC lexer to highlight and validate Nov 27, 2020
bartlomieju
bartlomieju approved these changes Nov 27, 2020
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice cleanup @Liamolucko

@bartlomieju bartlomieju merged commit 228ecb0 into denoland:master Nov 27, 2020
@Liamolucko Liamolucko deleted the swc-repl-highlighter branch November 27, 2020 19:24
@caspervonb
Copy link
Contributor

Great contribution, thanks @Liamolucko 👏 🥳

@bartlomieju bartlomieju mentioned this pull request Nov 29, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caspervonb caspervonb caspervonb requested changes

@bartlomieju bartlomieju bartlomieju approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@Liamolucko @caspervonb @bartlomieju