sainnhe / sonokai

High Contrast & Vivid Color Scheme based on Monokai Pro
MIT License
1.65k stars 119 forks source link

Interest in support nvim lsp semantic highlighting? #85

Closed jrwrigh closed 1 year ago

jrwrigh commented 1 year ago

nvim 0.9 now supports LSP semantic highlighting integrated with treesitter:https://neovim.io/doc/user/lsp.html#lsp-semantic-highlight

Interested in supporting this? I can try and copy over the existing LspSemantic* tokens to the nvim spec.

(side note, the @lsp.{token_name} syntax doesn't break anything in normal vim, right? It's just interpreted as a string? Just thinking about compatibility concerns, but I'm not super familiar with vim/neovim highlighting implementation details).

antoineco commented 1 year ago

@ highlight groups do break Vim as well as Neovim versions lower than 0.8.0. They need to be added behind this conditional:

https://github.com/sainnhe/sonokai/blob/a4d96b68fb1ba00fb2e91d2f46705de9cddbb348/colors/sonokai.vim#L500

I'm also using Neovim 0.9.0 and LSP highlights seem to be working without further changes. Are you having any specific issue?

jrwrigh commented 1 year ago

Not a specific issue, but a lot of issues.

This is running nvim 8.3.0: image

This is running nvim 0.9.0: image

As far as I can tell, the treesitter parsing is identical, but the highlighting is completely different/wrong.

antoineco commented 1 year ago

Good point, it seems like Neovim 0.9.0 links LSP highlight groups to Vim's default groups, whereas this colorscheme uses different highlighting paradigms whether the code is highlighted using classic Vim syntax or Treesitter. We should re-link those to Treesitter highlights.

jrwrigh commented 1 year ago

Ok, linking the lsp variables I was able to get this: image

with this diff:

diff --git i/colors/sonokai.vim w/colors/sonokai.vim
index a85a823..974e605 100644
--- i/colors/sonokai.vim
+++ w/colors/sonokai.vim
@@ -579,6 +579,28 @@ if has('nvim-0.8.0')
   highlight! link @variable TSVariable
   highlight! link @variable.builtin TSVariableBuiltin
 endif
+if has('nvim-0.9.0')
+  highlight! link @lsp.type.type TSType
+  highlight! link @lsp.type.class TSType
+  highlight! link @lsp.type.enum TSType
+  highlight! link @lsp.type.interface TSType
+  highlight! link @lsp.type.struct TSType
+  highlight! link @lsp.type.typeParameter TSType
+  highlight! link @lsp.type.parameter TSParameter
+  highlight! link @lsp.type.variable TSVariable
+  highlight! link @lsp.type.property TSProperty
+  highlight! link @lsp.type.enumMember TSProperty
+  highlight! link @lsp.type.events TSLabel
+  highlight! link @lsp.type.function TSFunction
+  highlight! link @lsp.type.method TSMethod
+  highlight! link @lsp.type.keyword TSKeyword
+  highlight! link @lsp.type.modifier TSOperator
+  highlight! link @lsp.type.comment TSComment
+  highlight! link @lsp.type.string TSString
+  highlight! link @lsp.type.number TSNumber
+  highlight! link @lsp.type.regexp TSStringRegex
+  highlight! link @lsp.type.operator TSOperator
+endif
 " }}}
 " github/copilot.vim {{{
 highlight! link CopilotSuggestion Grey

which is more-or-less a copy paste of the LspSemantic* tokens, but replacing them with the @lsp.type.* ones. So much improved, but the diagnostic (highlighting at the very top) is still wrong (greyed out, should at least be underlined).

antoineco commented 1 year ago

Thanks a lot for the initial investigation!

About the diagnostic message, is the example with or without g:sonokai_diagnostic_text_highlight = 1?

jrwrigh commented 1 year ago

About the diagnostic message, is the example with or without g:sonokai_diagnostic_text_highlight = 1?

That's without that setting. I discovered through :Inspect (new 0.9.0 command, very nice, much like) that the highlight group for it was DiagnosticUnnecessary, which wasn't handled by the sonokai color scheme. Adding:

  highlight! link DiagnosticUnnecessary WarningText

to the same if statement block above gets that back to normal. I just chose WarningText as it's what is used for DiagnosticUnderlineWarn.

Now I get this: image which I think is correct. lsp is now correctly overriding TS to see it as a function declaration rather than a variable to the function CEED_QFUNCTION (which is just a macro).