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

[Feature Request] Semantic highlighting for TypeScript/JavaScript #2872

Open
2 tasks done
stefandobre opened this issue Jan 3, 2022 · 4 comments
Open
2 tasks done
Labels
feature-request Request for new features or functionality typescript

Comments

@stefandobre
Copy link

stefandobre commented Jan 3, 2022

Context

  • This issue is not a bug report. (please use a different template for reporting a bug)
  • This issue is not a duplicate of an existing issue. (please use the search to find existing issues)

Description

This feature was already implemented in vscode, and it's built on top of the same TypeScript compiler that monaco uses, so from what I can tell it should just be possible to port the same code over to monaco-typescript.

I understand the Typescript worker's semantic tokens provider is not the most efficient implementation. I also understand its visual impact when used on top of Monarch will be more disruptive than on top of Textmate, at least for very large files. Yet, the pros clearly outweigh the cons.

If implemented, it could still be marked as experimental and disabled by default. It could also come with a more declarative option for when it should be enabled, e.g max number of characters, and/or max number of lines. The debounce time should also be configurable. When working on small snippets, there's no harm in running it more often.

The Monarch grammars for TS/JS leave a lot to be desired. Semantic highlighting would be a huge step forward.

@stefandobre stefandobre added the feature-request Request for new features or functionality label Jan 3, 2022
@Pistonight
Copy link

This issue has been open for almost 3 years. Is there any update?

I looked briefly at the bundled TS worker, it seems to have semantic tokenization functionality there. Maybe it can be exposed in the TypeScriptWorker API, then it can be hooked up with registerDocumentSemanticTokenProvider?

@Pistonight
Copy link

Looking into it a bit more, I think this plan should work in theory:

  • Expose getEncodedSemanticClassification in TypeScriptWorker that calls the same named function in typescript language service
  • Create an adapter in languageFeatures.ts that implements DocumentSemanticTokenProvider, which converts it to LSP format with delta lines, etc (might need to split tokens that span multiple lines?)
    • probably also implement the version with range
  • Register the adapter in tsMode.ts
  • Update ModeConfiguration and defaults to include new option for semantic highlighting

Since this is open for so long, I don't think this is being worked on by anyone right now, I will try to hack a PoC some time this week to see if this works. @hediet is this feature wanted by the maintainers and something that can be reviewed when I open a PR?

@Pistonight
Copy link

So the semantic token part is actually pretty straightforward, and I got the POC working here Pistonight@ac88467

However, the current out-of-box themes aren't rich enough. At a minimum, they don't have support for functions. Also, semantic tokens aren't the final solution to what is missing from the Monarch grammar. For example, console.log() has semantic tokens variable.defaultLibrary and member.defaultLibrary, which doesn't indicate log is a function - this should be provided by the monarch grammar.

Since there are still many questions to be answered, I think the initial step is to expose getEncodedSemanticClassifications in the worker. This will unblock user-land implementation of a better TypeScript experience with custom theme + semantic token provider

@Pistonight
Copy link

Alright, I have completed the POC implementation here: https://github.com/Pistonight/monaco-typescript-semantic-token-poc. It's hosted on GH pages here https://tspoc.pistonite.dev/ so people can try it out.

I did implement the performance-related configurations suggested (max length and debounce). However, the hosted POC app has max lines disabled, so you can test the performance with large files.

I do plan to upstream this if my PRs can get reviewed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality typescript
Projects
None yet
Development

No branches or pull requests

3 participants