-
Notifications
You must be signed in to change notification settings - Fork 36.5k
Refactors tokenization to support custom background tokenizer. #174364
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
Conversation
alexdima
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM. I left comments for some small things.
| this._semanticTokens.flush(); | ||
| } | ||
|
|
||
| // TODO@hediet TODO@alexdima what is the difference between this and acceptEdit? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I agree that they should be merged, right now acceptEdit only updates tokens and handleDidChangeContent only updates states. Maybe in a separate cleanup PR
…ization Refactors tokenization to support custom background tokenizer.
I recommend VS Code to review the diff.
@alexdima It would be awesome if you could carefully review this change.
The textmate tokenization feature that builds on this refactoring doesn't need a very careful review, as we can add an experimental setting (default false) for that.