search

echasnovski / mini.nvim

Library of 40+ independent Lua modules improving overall Neovim (version 0.8 and higher) experience with minimal effort
MIT License
4.54k stars 175 forks source link

fix(icons): ensure highlights are setup when changing colorschemes #1016

Closed mehalter closed 1 week ago

mehalter commented 1 week ago

This adds an autocommand on the ColorScheme event to make sure highlights are created if necessary for mini.icons

echasnovski commented 1 week ago

Thanks for the PR!

Highlight groups are created linked to some built-in highlight groups via default links (as in, :h :hi-default). Those links survive :hi clear if they were not overridden. If they are overridden, then managing them is laid on user's shoulders.

This is a common pattern for all modules which define highlight group via default link. So either they all should react to ColorScheme or none of them. I'll think about it.

In the meantime, closing as not planned (in this form; yet).

mehalter commented 1 week ago

This is quite an interesting take. It seems to be that default highlight groups in other mini modules are updated on ColorScheme. Also nearly all colorschemes that come to mind run :hi clear on being set to make sure they do not have interaction with other colorschemes. Popular colorschemes such as TokyoNight by folke, Catppuccin, Nightfox, etc. all do this and do not work given the current pattern in mini.icons, but do luckily work with other mini modules such as mini.test since it does set it's defaults on ColorScheme 🤔

mehalter commented 1 week ago

Is there a reason to maintain different stances on the ColorScheme autocommand and default highlight situation in the same ecosystem? This block of code was copied exactly from your other modules

echasnovski commented 1 week ago

This is quite an interesting take. It seems to be that default highlight groups in other mini modules are updated on ColorScheme.

Is there a reason to maintain different stances on the ColorScheme autocommand and default highlight situation in the same ecosystem? This block of code was copied exactly from your other modules

This is is consistent. Here is once again what is the behavior: "This is a common pattern for all modules which define highlight group via default link.". All cases in 'mini.nvim' of adding autocommand for ColorScheme event contain at least one group which is not defined via default link. As those won't indeed survive :hi clear, ColorScheme event autocommand is needed.

Also nearly all colorschemes that come to mind run :hi clear on being set to make sure they do not have interaction with other colorschemes. Popular colorschemes such as TokyoNight by folke, Catppuccin, Nightfox, etc. all do this and do not work given the current pattern in mini.icons, but do luckily work with other mini modules such as mini.test since it does set it's defaults on ColorScheme 🤔

Indeed calling :hi clear at the start of color scheme is a good approach. And again, all those color schemes do work as described in the previous comment: "Those links survive :hi clear if they were not overridden. If they are overridden, then managing them is laid on user's shoulders.". Some popular color schemes now indeed override 'mini.icons' colors. Hence if the user wants to change several completely different color schemes in one session (which, let's be real, I don't think is a common use case worth optimizing for), they should take care of the overridden highlight groups.

mehalter commented 1 week ago

Thanks for the explanation ❤️