sainnhe / everforest

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

Improve contrast for CurrentWord highlight #145

Closed eldar closed 2 months ago

eldar commented 3 months ago

First of all, thank you for a really great theme which has become my default.

Description

While really happy with the theme, I found it quite difficult to distinguish the highlighted word under cursor as implemented in plugins such as vim-illuminate or vim-cursorword. I use this feature a lot to quickly glance at all uses of the variable in the current context, but with the current colour for the highlight (bg2) the contrast between the highlight and the background is too low and it makes it really difficult to find all the instances. I raised the tone to bg3 and that change alone significantly improved the usefulness of this feature without significantly impacting aesthetics.

Screenshots

Here are a few screenshots illustrating this issue and why I believe the default should be changed. Here is the current behavior, note the little contrast between the highlighted variable and the background. In complex pieces I have to stress my eyes a lot to identify the highlight. I should also note that I have the highlight of the current line enabled NeoVIM which makes the highlight even less noticeable on the current line.

everforest_bg2

Here is the same piece of code and the same highlight with Catppuccin theme where the contrast is sufficient for the highlight to be easily detectable.

catpuccin

Here is the proposed change to bg3. Note that the highlighted word is visible much better.

everforest_bg3

I understand that I could manually override bg2 palette colour but that will result in undesireable changes in other components of the theme. I believe that this fairly self-contain change will improve the quality of the theme.

antoineco commented 3 months ago

@sainnhe I don't have a strong opinion about this since I don't use CurrentWord myself. What do you think?


A note regarding local customizations: you don't need to override any palette color to achieve this customization inside your own Vim configuration. You can instead use an autocmd to customize CurrentWord specifically (like you did in this PR) as outlined in the colorscheme's Vim help.

eldar commented 3 months ago

Hi @antoineco. Thanks for pointing me to autocmd - I was able to override CurrentWord in my config which solves the problem for me personally. It would still be nice though to change the default to something that works better. Highlighting the symbol under cursor is default in VS Code, so I could see how others would be using this feature and potentially encounter difficulties with the contrast.

antoineco commented 3 months ago

I personally think that this is a sensible change. bg3 is currently only used for inactive tabs and list characters so using it for CurrentWord is unlikely to result in a conflict with another highlight.

I would still like to give sainnhe a few days to respond. Meanwhile I will enable the feature on my end just to get a better sense for it, especially in combination with other highlights such as vim.lsp.buf.document_highlight().

sainnhe commented 3 months ago

Sorry for late reply. I just tried bg3 and IMO it does enhance readability in some cases, but it will also be distracting when there are a lot of symbols. Take a look at this example:

Before:

图像2024-8-28 11 07

After:

图像2024-8-28 11 06

When I’m coding, I wouldn’t expect there were a lot of distractions when I’m moving cursors. The referred symbols should only attract my attention when I consciously want to find them.

But I do agree it’s necessary for people who think the contrast is not enough. At present we have a configuration option to control the behavior of current word, maybe we can add another available value like “high contrast” to allow people to make the bg brighter.

antoineco commented 2 months ago

@sainnhe while I agree with you that a higher contrast can cause distractions, in practice I found out that most situations aren't as extreme as the example you posted. There will surely be places where a symbol is being referenced in a sequence—like in your screenshot—but I would argue that such examples are the exception and not the norm.

I've been using the setting for a week now in Go, Rust, Lua, Nix and Bash projects/files. The only "real-life" cases containing repeated symbols I found were in initialization/setup code, like in this snippet for instance.

Anyway, I just wanted to explain my views on this PR in more details. I don't have a strong opinion about this change, especially since it is trivial to override by following the documentation.

sainnhe commented 2 months ago

I write go and java, and I agree that this may not cause much trouble in go, but it’s not the case in java.

In java, the variables, classes, objects and methods can have very long names, and it will be very annoying if there are are a lot of strong highlights in my screen, especially these highlights are continuous changing when I’m moving cursors.

I still hold my opinion that the best solution for this problem is adding a new configuration option and keep the current behavior the default.

eldar commented 2 months ago

Well, the whole point of the highlight under cursor feature is to make variables visible. If it is barely discernible, it is not helpful. I don't mind the defaults either way because it is configurable - the reason I found this unusual was my prior experience with VS Code default theme or Catppuccin which are quite popular.

sainnhe commented 2 months ago

@eldar Thanks for your idea! I've added a new available value of g:everforest_current_word in commit https://github.com/sainnhe/everforest/commit/93b89630861a4c8a1be07bc54f6d53487acb2e57. You can try it out in the master branch, and if you have any problems, welcome to feedback :)