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

LSP: semantic tokens support #21100

Merged
merged 2 commits into from
Dec 8, 2022
Merged

Conversation

jdrouhard
Copy link
Contributor

@jdrouhard jdrouhard commented Nov 18, 2022

This is a continuation of #15723. This will take into account all the feedback accumulated over the year(s) there and take advantage of newer core LSP functionality to hopefully provide a good baseline for semantic tokens support.

  • Determine if core should provide basic highlighting support
  • Fix document version synchronization
  • Fix documentation to match new module and API
  • Update tests for new functionality and API

@github-actions github-actions bot added lsp lua stdlib labels Nov 18, 2022
src/nvim/auevents.lua Outdated Show resolved Hide resolved
@jdrouhard jdrouhard force-pushed the lsp_semantic_tokens branch 2 times, most recently from 57fa6fe to 628029f Compare November 20, 2022 04:51
@jdrouhard jdrouhard force-pushed the lsp_semantic_tokens branch 2 times, most recently from e7c7c79 to cbb982b Compare November 22, 2022 16:45
@lewis6991 lewis6991 linked an issue Nov 23, 2022 that may be closed by this pull request
@jdrouhard jdrouhard requested review from mfussenegger and gpanders and removed request for mfussenegger November 23, 2022 16:07
@jdrouhard
Copy link
Contributor Author

@mfussenegger @gpanders @clason

I think this is ready for an initial review. I've been using this branch pretty extensively the past few days and it feels pretty solid. I'll begin on tests, but just wanted to start soliciting feedback on overall design and API stuff before I get too far testing something that may change.

runtime/lua/vim/lsp/semantic_tokens.lua Outdated Show resolved Hide resolved
runtime/doc/lsp.txt Outdated Show resolved Hide resolved
@jdrouhard jdrouhard force-pushed the lsp_semantic_tokens branch 4 times, most recently from bf4b212 to 4dfaca7 Compare November 24, 2022 16:17
Copy link
Contributor Author

@jdrouhard jdrouhard left a comment

Choose a reason for hiding this comment

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

@mfussenegger - thanks for taking the time to help with the API design and for looking things over.

@bfredl - I'd like to tag you for some input here as you have significant involvement with highlighting in general.

General question - should I be using nvim_buf_add_highlight(), or should I use the extmark API directly? The former doesn't let me specify a priority for these highlights, but I believe it's a slightly faster code path.

runtime/doc/lsp.txt Outdated Show resolved Hide resolved
runtime/lua/vim/lsp/semantic_tokens.lua Outdated Show resolved Hide resolved
runtime/lua/vim/lsp/semantic_tokens.lua Outdated Show resolved Hide resolved
@clason
Copy link
Member

clason commented Nov 24, 2022

General question - should I be using nvim_buf_add_highlight(), or should I use the extmark API directly? The former doesn't let me specify a priority for these highlights, but I believe it's a slightly faster code path.

extmarks, hands down. It should follow (and integrate with) treesitter highlighting as closely as possible. In particular, we need proper priority support.

@rebelot

This comment was marked as off-topic.

@clason

This comment was marked as off-topic.

@rebelot

This comment was marked as off-topic.

@clason

This comment was marked as off-topic.

@clason
Copy link
Member

clason commented Nov 25, 2022

I really want to keep modifiers out of this PR, as this feature has been derailed often enough.

When this is merged, we'll open a dedicated issue to discuss how these should behave based on 1. how modifiers are actually used in practice (e.g., what are the "relevant" combinations?) and 2. how this fits into the basic highlighting design in Neovim. I cannot stress enough that we do not want to add a special-case code path for these few highlights if it's at all possible to fit this into a general (improved) design.

@jdrouhard jdrouhard force-pushed the lsp_semantic_tokens branch 2 times, most recently from f3fa521 to 8d1406c Compare November 28, 2022 00:14
@m-demare m-demare mentioned this pull request Mar 2, 2023
3 tasks
Julian added a commit to Julian/lean.nvim that referenced this pull request Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lsp: add semantic tokens support
9 participants