sainnhe / everforest

🌲 Comfortable & Pleasant Color Scheme for Vim
MIT License
2.94k stars 132 forks source link

feat(highlight): add diagnostic_float_highlight #136

Closed zennolux closed 5 months ago

zennolux commented 6 months ago

Description

I use lspsaga.nvim for a better code diagnostic experience.

Recently, I found a little issue relating to this plugin.

Diagnostic highlight in lspsaga uses the link option of DaignosticError 's highlight (which linked to ErrorText in everforest)

But the highlight of ErrorText does not have the fg option (which is used directly by lspsaga) set up.

I added a user configuration named diagnostic_float_highlight and used ErrorFloat as the link to DiagnosticError to fix that.

Screenshots

Before:

image

image


After:

vim.g.everforest_diagnostic_float_highlight = 1

image

image

antoineco commented 6 months ago

It's not the first time LSPSaga is causing issues with sainnhe's colorschemes. IMO it is time to address the issue directly with DiagnosticError and stop linking it to some underlined group. Recent versions of Neovim now use DiagnosticUnderlineError when the LSP client has underline enabled (which is the case in the default configuration).

sainnhe commented 6 months ago

I’d doubt that is it necessary to use user configuration to fix this issue, since it looks to be an issue of one or two highlight groups.

There is a hi group named LspSagaDiagnosticError. Maybe we can fix this issue by directly modifying it?

antoineco commented 6 months ago

@sainnhe unfortunately LspSagaDiagnosticError is outdated. This plugin has changed a lot since it was last supported in your colorschemes.

Like I said, it should be safe to rethink DiagnosticError, because Neovim uses a different set of highlight groups when underline is configured at the editor level. There are multiple popular plugins which rely on DiagnosticError to have a foreground color (like ErrorSign).

sainnhe commented 6 months ago

@sainnhe unfortunately LspSagaDiagnosticError is outdated. This plugin has changed a lot since it was last supported in your colorschemes.

Like I said, it should be safe to rethink DiagnosticError, because Neovim uses a different set of highlight groups when underline is configured at the editor level. There are multiple popular plugins which rely on DiagnosticError to have a foreground color (like ErrorSign).

You mean DiagnosticError is used in many other plugins? This looks like a coincidence which I created it as a predefined highlight group. Maybe it should be directly used as a highlight group for these plugins.

As for LspSaga, I haven’t updated the support for it for a long time because I didn’t and won’t use any plugins from that author, because he once copied my code and try to deny this fact, but with my reference of his initial commits he finally added me to README. Sorry for the inconvenience :(

antoineco commented 6 months ago

@sainnhe a number of Neovim plugins rely on Diagnostic* built-in highlight groups to set their own defaults for anything related to LSP severities. For example, Lualine reads those to determine a suitable fg for its LSP diagnostic icons.

This is a well accepted convention in Neovim; it allows plugins to use sensible defaults without having to require from colorschemes authors that they explicitly support the plugin. In the case of LSPSaga, there is not even a way to override the defaults.

I hope it is now clearer why forcing Diagnostic* to an underline style is problematic: if plugins can't determine the fg color, they either default to Normal, or to some arbitrary color which isn't harmonious with the colorscheme. We should instead use a proper fg for those, and reserve the sp variant for the DiagnosticUnderline* built-in highlight groups. (Of course ErrorText, etc. can retain the underline.)

antoineco commented 5 months ago

Superseded by #137