doom-neovim / doom-nvim

A Neovim configuration for the advanced martian hacker
GNU General Public License v2.0
1k stars 108 forks source link

fix(statusline): increase contrast #381

Closed edwintorok closed 2 years ago

edwintorok commented 2 years ago

Some colorschemes (e.g. Zenburn) have a light foreground for the statusline, and most of the special* colors are light too, resulting in very low contrast. In such a case swap statusline background/foreground colors.

This makes ZenBurn theme's statusline readable, even though it doesn't use the statusline colors the theme intended. This is more of a workaround, but works for me so far. Perhaps a better way would be to avoid using so many other colors that were not intended for the statusline and instead try to just tweak the bold/etc. attributes? What do you think?

FWIW with the previous version of Doom the statusline worked without needing to tweak colors.

connorgmeehan commented 2 years ago

Thanks for the PR, I'll test out zenburn soon and see how this effects other colorschemes. Possible solutions off the top of my head are

  1. Adding an option to disable the auto-theming/include some basic themes. doom.features.statusline.settings.theme = "auto"|"doom-one"|"..."
  2. Adding an option to override colorscheme colours doom.features.statusline.settings.colorscheme.background = #ccc
  3. Adding an option to override highlight lookup doom.features.statusline.settings.colorscheme_highlights.background = { "ReliableHighlight" }
edwintorok commented 2 years ago

I think safe combinations of colors that should work on most colorschemes are: bg + (normal fg, any of the other TS fgs), statusline bg + statusline fg, and statuslinenc bg + statuslinenc fg. See what zenbones does for some ideas (but zenbones generates the main highlight colorscheme too): generating lualine themes, generating lightline themes. Getting this right so that it works with all themes is probably non-trivial.

I like the idea for setting a statusline theme (in this case it'll generate those 3 colors based on that particular theme, right?)

Although one thing I notice about StatusLine is that it has gui=bold,reverse set (at least in the previous version of doom-nvim, if I type hi StatusLine it prints:

StatusLine     xxx cterm=bold,reverse ctermfg=236 ctermbg=186 gui=bold,reverse
                   guifg=#313633 guibg=#ccdc90

So perhaps all that doom-nvim should do here is detect that and swap bg/fg to get things working? It only copies the bg/fg color of the StatusLine, but not its attributes (like bold, reverse), etc.

connorgmeehan commented 2 years ago

Hmmm, I checked out your PR and it seems to break some other color schemes including the default doom-one colorscheme. I think this is due to passing in the statusline highlight

Doom one

Screen Shot 2022-08-08 at 11 08 43 pm

Dawnfox

Screen Shot 2022-08-08 at 11 08 28 pm

Can you also send me a link to which zenbones you're using just to make sure I'm not testing the wrong themes? I can fix this one as I feel bad with you going through this spagetti code and it's a good opportunity for me to tidy it up and break the colorscheme generation into its own file.

Also, I agree auto theming definitely needs to be smarter and I'd really like to get irregular themes like zenbones working. Checking if the highlight is reversed is a really good point but I don't think we can trust theme authors to code this in a way that will work for us because there's so much variety in the way themes setup their highlights.

I like the idea of hijacking lualine support so maybe I can refactor statusline to

  1. Check if the theme supports lualine and use that colour palette.
  2. Fallback to auto generation if no lualine theme found.

Also some notes for myself on how I might refactor the color scheme generator.

1. Check if it's a dark / light theme by checking the brightness of the `Normal` highlight 2. Use that to invert the `Statusline`, `StatusLineNC`, `TabLine` highlights if necessary. a. i.e. `phha/zenburn.nvim` uses a very bright green but if you look at the repo pictures that's not what the author intended. This should be inverted in most cases. Screen Shot 2022-08-08 at 10 38 38 pm 3. Once the base background colour scheme is decided, pick the 3 main colours based on contrast with the background. a. The chosen colours should be have a high contrast in brightness / value between statusline background (get the HSV of background and make sure there is a big difference in V) b. The chosen colours should have a decent amount of hue contrast with each other so they look distinct. image
edwintorok commented 2 years ago

I use these colorthemes:

You can find the colorschemes I used in the old 3.x doom-nvim config of mine: https://github.com/edwintorok/doom-nvim/blob/local/colors/zenburnish_hc.vim (generated from https://github.com/edwintorok/doom-nvim/blob/local/shipwright_build.lua)

Note that using HSV(or HSL) to deal with colorschemes is somewhat "deprecated" these days, even though it is what theme generators like lush.nvim use: OKLab/OKLCh is a lot better and mostly compatible with code that deals with 3 color coordinates in terms of hue, lightness and chroma: https://bottosson.github.io/posts/oklab/ CAM16-UCS/etc. are more comprehensive (and take the effect of surround into consideration, etc.) but are too complex to use in a colorscheme IMHO. There are Lua implementations of this colorscheme, although perhaps using them just to sort colors would be an overkill.

If you intend to work on this can you try whether detecting the 'gui=reverse' works? Might be a simpler fix than refactoring the whole colorscheme code :)

edwintorok commented 2 years ago

Meanwhile I've tried detecting the 'reverse' attribute, seems to work fine so far for StatusLine (and in fact I've done the detection in safe_get_highlight in case other highlights have similar settings). Seems to work fine so far on ZenBurn, could you try whether it works with your themes?

P.S.: the refactor you mentioned is probably a good idea anyway, perhaps as a followup to this, thanks for offering to do it

connorgmeehan commented 2 years ago

Wow just read up about OKLab and it's super interesting, I've always been dissatisfied with HSL. Yeah I wont have an opportunity to do the larger refactor for a week or so so I'm definitely keen for a quick fix.

Here's the result of my testing it's looking pretty good to me. jnurmine/Zenburn

Screen Shot 2022-08-09 at 9 01 30 pm

doom-one (default)

Screen Shot 2022-08-09 at 9 03 46 pm

EdenEast/nightfox.nvim (dawnfox/nightfox)

Screen Shot 2022-08-09 at 9 04 22 pm Screen Shot 2022-08-09 at 9 04 46 pm

mcchrish/zenbones.nvim (zenburned)

Screen Shot 2022-08-09 at 9 06 37 pm

I was able to improve some of the errors played around with adding more highlight fallbacks which I'll commit after merging this.

Screen Shot 2022-08-09 at 10 29 42 pm
connorgmeehan commented 2 years ago

I pushed the fixes to improve compatibility with non treesitter themes. Could you test it out and see if that fixes your issue as well?

edwintorok commented 2 years ago

Thanks, this works great!