utilyre / barbecue.nvim

Visual Studio Code inspired breadcrumbs plugin for the Neovim editor
MIT License
776 stars 31 forks source link

[FEAT]: nvim_set_hl should set "default" #84

Closed GnikDroy closed 1 year ago

GnikDroy commented 1 year ago

Requirements

Problem

Consider a scenario where I am manually setting the highlight groups for barbecue via a colorscheme plugin.

colorscheme plugin loads first -> sets hl groups for barbecue barbecue loads second -> overrides the hl groups already set by the colorscheme plugin

Solution Suggestion

Using the default flag avoids this problem completely. This will allow me to keep my highlight groups in a unified location (in the colorscheme file) if I want to. I don't think there are any drawbacks.

Workaround

Configuring the load order of plugins. If barbecue loads first, the colorscheme plugin correctly overrides the hl groups as expected.

utilyre commented 1 year ago

This is the intended behavior, see theming.

GnikDroy commented 1 year ago

I have read the document. It doesn't mention why the default parameter is not used, though.

Just to be sure, I am talking about :h nvim_set_hl • default: Don't override existing definition |:hi-default| in the following:

https://github.com/utilyre/barbecue.nvim/blob/cd7e7da622d68136e13721865b4d919efd6325ed/lua/barbecue/theme.lua#L140-L144

I notice that any particular group overrides theme.normal (possibly to allowing us to change background for all groups)

But none of them set the default parameter, even though they are supposed to be "default" values.

I can see that it has its own theming mechanism, like lualine. But unlike lualine, barbecue doesn't have theme = auto, or something similar.

Can you explain why this is the intended behaviour?

utilyre commented 1 year ago

I can see that it has its own theming mechanism, like lualine. But unlike lualine, barbecue doesn't have theme = auto, or something similar.

Yes, it does have! See the Configurations section on the readme.

GnikDroy commented 1 year ago

Apologies. Somehow missed that.

Still can't figure out why "default" is not set though. What does it break?

utilyre commented 1 year ago

barbecue_* highlights are meant to be generated on the fly according to the theme. So if the default option gets enabled, it'd be possible to end up with highlight values that weren't expected.

GnikDroy commented 1 year ago

Edit: on further inspection, I think this is fine.

Although, I wonder if we could get rid of the plugin load order issue.

NOTE: Make sure barbecue loads after your colorscheme.

utilyre commented 1 year ago

It's just good practice to load your colorscheme before any other plugin, from lazy.nvim readme:

NOTE: since start plugins can possibly change existing highlight groups, it's important to make sure that your main colorscheme is loaded first. To ensure this you can use the priority=1000 field. (see the examples)