-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
Improve highlights for Neovim LSP #229
Conversation
Just a heads-up: these will change again (for the last time?) once neovim/neovim#12655 is merged. |
And now the PR is merged; the new LSP diagnostic highlight names are listed here: https://github.com/neovim/neovim/blob/35325ddac04b1b59b7982797021cdaabdabc87fb/runtime/doc/lsp.txt#L376 |
9bb9071
to
51b5c3c
Compare
51b5c3c
to
4a9a08d
Compare
Done. Seems visually nice to me. |
The doc mentions that "Sign, underline and virtual text highlights (by default) are linked to their This is what onedark.vim does (disclaimer: PR was from me). |
4a9a08d
to
fb62426
Compare
Done, @ojroques! Thanks for the tip. |
Glad to help. I would have kept these lines though since they differ from the defaults: call s:hi("LspDiagnosticsUnderlineWarning" , s:nord13_gui, "", s:nord13_term, "", "undercurl", "")
call s:hi("LspDiagnosticsUnderlineError" , s:nord11_gui, "", s:nord11_term, "", "undercurl", "")
call s:hi("LspDiagnosticsUnderlineInformation" , s:nord8_gui, "", s:nord8_term, "", "undercurl", "")
call s:hi("LspDiagnosticsUnderlineHint" , s:nord10_gui, "", s:nord10_term, "", "undercurl", "") The underline text is quite useful to spot warnings/errors. |
After neovim/neovim#12655 has been merged, highlights have changed a little, so this commit updates the obsolete highlight groups.
fb62426
to
28d35c2
Compare
You're totally right, my bad. Just included those again. |
With |
How come? I'm using my fork with this PR applied and everything seems fine. |
okay, works perfectly on another machine. this is weird. |
Might be your Neovim version maybe? Anyway, good it's working! |
My fork includes my PR (which was opened upstream) to fix LSP-related highlights. This shall be reverted once nordtheme/vim#229 is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @gbrlsnchs, @clason, @ojroques and @crispgm 👋, thanks for your contributions 👍
Also thanks for your patience, the large amount of tasks for the Nord project are really time consuming and free time is kind of rare.
Changes looking good to me 👍🏻
The problems regarding the undercurl
attribute can be tackled later on in a separate PR by guarding the usage through the nord_underline
theme configuration variable. When the terminal is capable to rendering underlines it should also work fine for curly lines.
Closes #248
To ensure compatibility with the latest versions of Neovim LSP the highlighting groups for diagnostics have been adapted to the changes of neovim/neovim#12655 [1]. See :help lsp-highlight-diagnostics [2] for more details. Note that LSP will be available as of Neovim 0.5 which is (at the time of this commit) still in development and only available as nighly build. Also see great articles from Nord Vim contributors like "Neovim (0.5) Is Overpowering" [3] for more information about Neovim 0.5 features, including LSP. [1]: neovim/neovim#12655 [2]: https://neovim.io/doc/user/lsp.html [3]: https://crispgm.com/page/neovim-is-overpowering.html Co-authored-by: Sven Greb <[email protected]> Closes nordthemeGH-229 Closes nordthemeGH-248
No description provided.