Skip to content
This repository has been archived by the owner on Apr 16, 2024. It is now read-only.

fix(statusline): increase contrast #381

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

edwintorok
Copy link
Contributor

@edwintorok edwintorok commented Aug 7, 2022

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
Copy link
Collaborator

connorgmeehan commented Aug 8, 2022

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
Copy link
Contributor Author

edwintorok commented Aug 8, 2022

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
Copy link
Collaborator

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
  1. 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
Copy link
Contributor Author

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 :)

Some colorschemes (e.g. Zenburn) get a reverse attribute in the
StatusLine highlights.
Detect this and swap foreground/background.

Signed-off-by: Edwin Török <edwin@etorok.net>
@edwintorok
Copy link
Contributor Author

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
Copy link
Collaborator

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 connorgmeehan marked this pull request as ready for review August 9, 2022 12:31
@connorgmeehan connorgmeehan merged commit 24a5a40 into doom-neovim:main Aug 9, 2022
@connorgmeehan
Copy link
Collaborator

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
Copy link
Contributor Author

Thanks, this works great!

@edwintorok edwintorok deleted the color-fixes branch August 9, 2022 22:05
randomizedthinking pushed a commit to randomizedthinking/doom-nvim that referenced this pull request Aug 30, 2022
…vim#381)

Some colorschemes (e.g. Zenburn) get a reverse attribute in the
StatusLine highlights.
Detect this and swap foreground/background.

Signed-off-by: Edwin Török <edwin@etorok.net>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants