Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement separate *NC control highlight groups #176

Merged
merged 3 commits into from
Nov 20, 2022

Conversation

HiPhish
Copy link
Contributor

@HiPhish HiPhish commented Nov 19, 2022

This is the continuation of #174, it fixes the remaining issue I mentioned over there. Now the background should work regardless of which window carries the control UI. I wish Vim's highlight system had a sort of "inheritance" feature that is between link and defining from scratch. Then we would not have to do this ugly highlight group patching.

screenshots

For each of the control highlight groups there is now a corresponding highlight group with the same name plus NC as suffix. When entering or exiting the window which carries the support groups we reset the 'winbar' option such that the highlight groups are either normal or *NC.

The reason for this is that the user might have a different background for the highlight groups WinBar and WinBarNC. Whenever the user exits or enters a window, the editor will flip between these two groups, so we have to flip our custom highlight groups accordingly as well.

For each of the control highlight groups there is not a corresponding
highlight group with the same name plus 'NC' as suffix. When entering or
exiting the window which carries the support groups we reset the
'winbar' option such that the highlight groups are either normal or
'*NC'.

The reason for this is that the user might have a different background
for the highlight groups 'WinBar' and 'WinBarNC'. Whenever the user
exits or enters a window, the editor will flip between these two groups,
so we have to flip our custom highlight groups accordingly as well.
The fallback NC highligh groups are just aliases to their regular
versions. We need them because the window bar rendering code assumes
they exist.
@HiPhish
Copy link
Contributor Author

HiPhish commented Nov 19, 2022

I have fixed the merge conflicts. A word of note: you correctly found that vim.api.nvim_get_hl_by_name only exists in Neovim 0.8+, but the ability to pass a table to vim.cmd only exists in 0.8+ as well. I put an if has("nvim-0.8") block around the entire patching thing so none of it will be run on older versions of Neovim. It will still look ugly like before, but it will work.

We could glue together strings for vim.cmd if you prefer that, but I think it's better not accumulate technical debt. If we decide to abandon support for older version of Neovim we just have to remove a few if checks instead of rewriting a bunch of vim.cmd arguments.

What is you opinion?

@rcarriga
Copy link
Owner

I'm OK leaving it as is, this module is pretty stable and I'm not planning to maintain older versions for much longer so it'll be cleaned up anyway 😄 Thanks for the followup!

@rcarriga rcarriga merged commit 69a3982 into rcarriga:master Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants