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

Semantic tokens #9360

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Semantic tokens #9360

wants to merge 5 commits into from

Conversation

kirawi
Copy link
Member

@kirawi kirawi commented Jan 17, 2024

Based on #8021

TODO

Notes

@kirawi kirawi added S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. A-language-server Area: Language server client A-helix-term Area: Helix term improvements labels Jan 17, 2024
@kirawi
Copy link
Member Author

kirawi commented Jan 18, 2024

My main focus has been trying to workout how to use semantic highlighting to define new local definitions. It took a while since I went through some overly complicated designs before arriving at the one I'm proposing now:

  • Syntax's API would be extended to allow callers to pass a function that would define locals per each tree-sitter layer and each local.scope defined in locals.scm. The difference between each layer and each scope is significant from Syntax's perspective since it would have different local.scopes if it references a different language. I could see this as a function that returns a vector of tuples of the identifier itself and the highlight. Ideally this API would also allow us to only work on a range of the text.
  • However, it's important that we also remove outdated local.definitions. I think this should also be an extension of the API where a function is passed and can mutate the locals per each layer/scope (only those defined by the method above). In the case of semantic tokens, I think it would just be removed if either there are no more local.references with the definition's corresponding identifier or if there is now a local.definition. The reason I think a function is necessary is to accommodate stack-graphs where the existence of references does not matter in terms of correctness. I don't think this makes the API more complicated than if this change was scoped to just semantic highlighting.

Aside from the above, the actual rendering of semantic highlights would involve a system very similar to inlay hints.

@kirawi kirawi added the A-core Area: Helix core improvements label Jan 18, 2024
@kirawi
Copy link
Member Author

kirawi commented Jan 21, 2024

@the-mikedavis I'm actually not sure that the approach I outlined above is optimal because a LocalDef's range would have to be mapped on every change to the document which I think is out-of-scope for Syntax. Since you have a lot of experience with tree-sitter, do you have any suggestions for how I can add "external" local definitions for a document?

I am thinking of storing a vector of LocalDefs in Document and mapping them or removing outdated definitions on Document::apply. Then I could pass them to HighlightIter.

@kirawi
Copy link
Member Author

kirawi commented Jan 22, 2024

In the future, I think #1115 can be handled through the same code path as semantic tokens.

@the-mikedavis
Copy link
Member

Yeah extra information from the LSP that needs to be mapped through changes should live on the Document. LocalDef should still be private to syntax.rs though, I think it should be possible just to pass in a std::ops::Range<usize> since that's really the only info the language server gives you.

#1115 is more complicated and I don't think it will overlap with this. Knowing all of the definitions in the document (which you could figure out when running injections queries) doesn't help since you also need to run highlights across potentially the entire text in order to determine the LocalDef's highlight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Helix core improvements A-helix-term Area: Helix term improvements A-language-server Area: Language server client S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants