dracula / vim

🧛🏻‍♂️ Dark theme for Vim
https://draculatheme.com/vim
MIT License
1.34k stars 454 forks source link

feat(nvim): Add support for WinSeparator #282

Closed stnley closed 2 years ago

stnley commented 2 years ago

Neovim recently added a feature for a global statusline. A new highlight group WinSeparator was created linking to VertSplit for compatibility.

This colorscheme links VertSplit to DraculaBoundary. Because the statusline is no longer present on horizontal splits with this feature enabled, the DraculaBoundary group is used in between them.

This PR changes the look of the separator for both vertical and horizontal splits using the new upstream group. A new group DraculaSeparator is introduced for this purpose.

For comparison. The default behavior would use the DraculaBoundary group. With the statusline removed, the border is uneven. old-winseparator

Neovim master with the new DraculaSeparator group. new-winseparator

This does change the existing look of vertical splits for Neovim users (> v0.7.0). Because of that I understand this may not be desired, or we would want to change VertSplit to use the DraculaSeparator group for consistency.

benknoble commented 2 years ago

This doesn't affect me (I don't use neovim), but I do prefer more contrast between windows; it's easier to tell where one file ends and another begins that way.

stnley commented 2 years ago

@benknoble That makes sense and I suspected as much. How would you feel about this:

The DraculaSeparator group has the same look as DraculaBoundary. Then, we link VertSplit to it. This way a user can modify how the splits look in vim or neovim, and not mess up places where DraculaBoundary are used (specifically tabline I can imagine being an issue).

Something like this

--- a/colors/dracula.vim
+++ b/colors/dracula.vim
@@ -194,7 +194,7 @@ call s:h('DraculaInfoLine', s:none, s:none, [s:attrs.undercurl], s:cyan)
 call s:h('DraculaTodo', s:cyan, s:none, [s:attrs.bold, s:attrs.inverse])
 call s:h('DraculaSearch', s:green, s:none, [s:attrs.inverse])
 call s:h('DraculaBoundary', s:comment, s:bgdark)
-call s:h('DraculaSeparator', s:comment, s:none)
+call s:h('DraculaSeparator', s:comment, s:bgdark)
 call s:h('DraculaLink', s:cyan, s:none, [s:attrs.underline])

 call s:h('DraculaDiffChange', s:orange, s:none)
@@ -245,7 +245,7 @@ hi! link TabLine      DraculaBoundary
 hi! link TabLineFill  DraculaBgDark
 hi! link TabLineSel   Normal
 hi! link Title        DraculaGreenBold
-hi! link VertSplit    DraculaBoundary
+hi! link VertSplit    DraculaSeparator
 hi! link Visual       DraculaSelection
 hi! link VisualNOS    Visual
 hi! link WarningMsg   DraculaOrangeInverse

The look I prefer can then be achieved with normal customization

augroup dracula_customization
  autocmd!
  autocmd Colorscheme dracula highlight DraculaSeparator guibg=None
augroup END
benknoble commented 2 years ago

Then I would probably prefer a strong case be made for including two names that mean the same thing: boundary and separator are synonyms, no?

DraculaBoundary is currently used for Folded, Tabline, and VertSplit. If you want to introduce DraculaWinSeparator (yay, now the names are different), then I think it should be obviously used for VertSplit (as the name suggests). But then DraculaBoundary might need a rename… which may or may not break things for some folks (though so could changing how VertSplit is linked…). Argh.

In summary, I'm in favor of finding a way to move forward on this, but I'm not sure what the best way forward is ☹️

stnley commented 2 years ago

Good point, they are very similar in name. I like DraculaWinSeparator as it makes clear the relationship with the new WinSeparator group in neovim, however it makes sense for vim users as well. It separates windows!

I do not think that DraculaBoundary needs a rename at this time, however if a new major version was introduced I could see it being changed. I'd rather not break things of course.

I pushed a new commit and did link VertSplit. Considering it is using the same style I think the only clients which break are ones that override the DraculaBoundary style today.

dsifford commented 2 years ago

Sorry @benknoble, been super busy with work things. I generally agree with everything you've already mentioned. So feel free to see this through the finish line 👍