jnurmine / Zenburn

Zenburn is a low-contrast color scheme for Vim.
http://kippura.org/zenburnpage
891 stars 149 forks source link

add g:zenburn_disable_bold_CursorLine #28

Closed mwcz closed 4 years ago

mwcz commented 4 years ago

This PR adds a new configuration, g:zenburn_disable_bold_CursorLine.

I named it zenburn_disable_bold_CursorLine instead of zenburn_bold_CursorLine to preserve zenburn's convention of config defaults always being 0.

Let me know what you think. Thanks!

mwcz commented 4 years ago

I just noticed that the CursorColumn is also being bolded. I've made a local update so that g:zenburn_disable_bold_CursorLine also disables the bold CursorColumn, but that makes the setting's name a little misleading.

I considered these:

Or, I could split it out into two settings, g:zenburn_disable_bold_CursorLine and g:zenburn_disable_bold_CursorColumn.

jnurmine commented 4 years ago

Hi, thanks for this.

It seems the CursorColumn is bolded only if g:zenburn_unified_CursorColumn=1 (and g:zenburn_high_Contrast=1 since low contrast has no bold):

image

And with g:zenburn_unified_CursorColumn=0 it looks like this:

image

What do you say if it were called g:zenburn_disable_bold_CursorBars, and:

I mean something like this; this is in the high contrast branch, the low contrast has no bold.

    if exists("g:zenburn_disable_bold_CursorBars") && g:zenburn_disable_bold_CursorBars
        " no bold here
        if exists("g:zenburn_unified_CursorColumn") && g:zenburn_unified_CursorColumn
            hi CursorColumn  guibg=#121212 ctermbg=233 cterm=none
        else
            hi CursorColumn  guibg=#2b2b2b ctermbg=235 cterm=none
        endif

        hi CursorLine    guibg=#121212 ctermbg=233 cterm=none
    else
        if exists("g:zenburn_unified_CursorColumn") && g:zenburn_unified_CursorColumn
            hi CursorColumn  guibg=#121212 gui=bold ctermbg=233 cterm=none
        else
            hi CursorColumn  guibg=#2b2b2b ctermbg=235 cterm=none
        endif

        hi CursorLine    guibg=#121212 gui=bold ctermbg=233 cterm=none
    endif

What do you think?

mwcz commented 4 years ago

Hmm, it would be clearest to me if g:zenburn_unified_CursorColumn controls color, but not boldness. Like this:

    if exists("g:zenburn_disable_bold_CursorBars") && g:zenburn_disable_bold_CursorBars
        " no bold
        if exists("g:zenburn_unified_CursorColumn") && g:zenburn_unified_CursorColumn
            hi CursorColumn  guibg=#121212                     ctermbg=233 cterm=none
        else
            hi CursorColumn  guibg=#2b2b2b                     ctermbg=235 cterm=none
        endif

        hi CursorLine    guibg=#121212                         ctermbg=233 cterm=none
    else
        " bold!
        if exists("g:zenburn_unified_CursorColumn") && g:zenburn_unified_CursorColumn
            hi CursorColumn  guibg=#121212 gui=bold            ctermbg=233 cterm=bold
        else
            hi CursorColumn  guibg=#2b2b2b gui=bold            ctermbg=235 cterm=bold
        endif

        hi CursorLine    guibg=#121212 gui=bold                ctermbg=233 cterm=bold
    endif

That's changing existing behavior though. If you'd like to preserve the existing behavior where boldness depends on g:zenburn_unified_CursorColumn=1, I'm cool with it and will add it to this PR.

jnurmine commented 4 years ago

Yeah, I guess it makes sense to sort of split g:zenburn_unified_CursorColumn to something controlling boldness and something controlling colour.

So, let's say g:zenburn_unified_CursorColumn now controls colour and the g:zenburn_disable_bold_CursorBars controls boldness.

The low contrast branch does not have bold, so it needs no changes, it'll look the same as before (it ignores the g:zenburn_disable_bold_CursorBars). (With branch I mean the conditional branches; if-else things)

The high contrast branch could be changed like you propose.

And then:

I think this should be OK. It's hard to keep backwards compatibility without adding an extra g:zenburn_new_CursorBars_handling which is default 0, and when 1, activates behaviour like above. But then things become unnecessarily messy.

TL;DR: the proposal in your comment looks good to me.

mwcz commented 4 years ago

This is ready now. Let me know if I missed anything!

(sorry for the close/re-open spam; I made the mistake of pushing mid-squash.)

jnurmine commented 4 years ago

Looks good to me, thanks!