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

Async textmate tokenization #174443

Merged
merged 7 commits into from
Feb 20, 2023
Merged

Async textmate tokenization #174443

merged 7 commits into from
Feb 20, 2023

Conversation

hediet
Copy link
Member

@hediet hediet commented Feb 15, 2023

Fixes #147066

Review #174364 first!

TODOs:

  • Release & adopt new vscode-textmate

…xperimental.asyncTokenization" (default false).
@hediet hediet requested a review from alexdima February 15, 2023 09:24
@vscodenpa vscodenpa added this to the February 2023 milestone Feb 15, 2023
@zm-cttae-archive
Copy link

zm-cttae-archive commented Feb 15, 2023

As downstream user, what will change in new vscode-textmate? Will it still be UMD & same interface?

If there's an issue or PR to discuss or explain it, do please point me there!

@hediet
Copy link
Member Author

hediet commented Feb 15, 2023

As downstream user, what will change in new vscode-textmate? Will it still be UMD & same interface?

Nothing changes!

@zm-cttae-archive
Copy link

zm-cttae-archive commented Feb 15, 2023

Sounds great! Will unsub, also some relevant xkcd's to reward you:

xkcd: 1172 - Workflow; xkcd: 2347 - Dependency

Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

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

🚀 This looks really good! I left some comments. Some notes, mostly for myself, to double check before we merge this in:

  • check that a text model that is not attached to an editor doesn't do background tokenization
  • check that this works OK on the web
  • check that vscode-textmate is not loaded if the workbench comes up without an editor
  • create build and check that this works in a built product. I think it will definitely not work because we don't package textmate.worker.ts for shipping.

@hediet
Copy link
Member Author

hediet commented Feb 17, 2023

create build and check that this works in a built product. I think it will definitely not work because we don't package textmate.worker.ts for shipping.

I need help with this one. I tried to understand how "vs/editor/common/services/editorSimpleWorker.ts" is bundled, but I wasn't able to figure it out.

Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

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

👏

@hediet hediet merged commit 48a677e into main Feb 20, 2023
@hediet hediet deleted the async-tokenization-2 branch February 20, 2023 13:27
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move tokenization to a web worker
4 participants