Closed nkgm closed 4 months ago
Hi there! Nothing wrong with your config! It's a bug and I found the reason. It's because reactive
caches window-local highlights for hl groups and then reuses them if they're cached. I'll fix that and let you know! Thank you for reporting!
Hey @nkgm! Just fixed it. Can you check on your end as well? Thank you again for reporting!
I think I may add the ability to use a function for winhl
and hl
properties, so you'll be able to do the same (check buftype and return highlights) without creating 2 presets for this reason.
Hi @rasulomaroff, thanks for the quick fix, it's working great!
I think I may add the ability to use a function for winhl and hl properties, so you'll be able to do the same (check buftype and return highlights) without creating 2 presets for this reason.
That would be a fantastic improvement I'm personally looking forward to. As a new user I found the configuration a little overwhelming for the seemingly simple task of setting a few colors. I know this plugin does some pretty advanced stuff, so I'll have to revisit the docs, but simple things like, why can't I keep my whole config under opts
? According to the docs one might use:
opts = {
builtin = {
cursorline = true,
cursor = true,
modemsg = true,
},
configs = {
nord = nord_preset,
term = term_preset_high,
},
},
But examining more carefully (emphasis mine):
-- a key here is a preset name, a value can be a boolean (if false, presets will be disabled) -- or a table that overwrites preset's values
So one still has to use add_preset
, which feels a little counter-intuitive. At the same time I had to troubleshoot a conflict with the Nord plugin I was using at the time, causing CursorLine to disappear in n
mode, now replaced with a modern Lua alternative. I'm not complaining, in fact my setup looks better than ever, and I'm happy I persevered.
These are just my observations as a new (and impatient) user, only meant as feedback. In fact, I really appreciate the effort you've invested not only into this plugin itself, but also writing extensive docs, so thanks again!
You raised a very interesting topic @nkgm! There's a huge difference between initialization and configuration for neovim plugins. You can read this article to understand what I'm talking about. I was developing this plugin keeping that idea in mind.
Briefly, any plugin should initialize itself and work without needing to call the setup
function. reactive
will initialize itself as soon as it will be required
from anywhere in the code.
reactive
was built with the idea of having several presets from other plugins, like your theme for example, and then allowing users to configure those presets through the setup
function while also allowing users to create their own presets and loading them so it could operate both as a dependency and as a main module. This is why you have the priority
field, ability to add several presets, name them, enabling if they are lazy
etc. If you had everything in one place (setup
function), then it would look really messy especially if you configured some presets from other plugins and added your own (maybe several ones?).
In your case it could be easier to put those presets in reactive/presets/{name}.lua
and then load them in your config like that:
opts = {
builtin = {
cursor = true,
cursorline = true,
modemsg = true
},
load = { 'term', 'nord' } -- 'term' and 'nord' are for term.lua and nord.lua files
}
But it's up to you how you wanna configure it :)
Also, don't forget that reactive
has from
and to
callbacks available for a mode
configuration. Meaning that you can have separate presets for doing only functional stuff, like disabling relative numbers when going to insert mode and enabling when going from it. I know that it's not used for these purposes that often, but it can be.
I'm still in the process of searching the best way to do everything I've described. I think it's also worth it to create Wiki pages explaining how to do stuff, dos and don'ts etc. Thank you again!
I'm sorry for the lengthy response. Originally it consisted of two parts, but on second thought, the second part should be a standalone feature request issue about winhl
accepting function values as discussed, along with an example setup outlining its importance, so I'll be opening a new issue for that. Here's the part relevant to the ongoing discussion.
I went through the article and I take it you are concerned about initialization order and side-effects? While I am not a Neovim plugin author and my knowledge is somewhat limited, I can't see what's stopping my (side-effects free) setup:
local preset1 = { ... }
local preset2 = { ... }
return {
"rasulomaroff/reactive.nvim",
opts = {
builtin = {
cursorline = true,
cursor = true,
modemsg = true,
},
},
config = function(_, opts)
local reactive = require("reactive")
reactive.add_preset(preset1)
reactive.add_preset(preset2)
reactive.setup(opts)
end,
}
... from becoming this (simpler) setup:
local preset1 = { ... }
local preset2 = { ... }
return {
"rasulomaroff/reactive.nvim",
opts = {
builtin = {
cursorline = true,
cursor = true,
modemsg = true,
},
},
load = {
preset1, -- the tables created earlier
preset2,
}
-- lazy will automatically do this for us
-- config = function(_, opts)
-- require("reactive").setup(opts)
-- end,
}
load = { 'some_preset_file' }
and load = { some_preset_table }
should be equivalent, as long as some_preset_file
is side-effects free. In the case that it's not, you could allow load
to also accept function values:
local my_preset_table = ...
load = {
'my_preset_file',
my_preset_table,
{
name = 'my_inline_preset_table'
...
},
function()
vim.api.nvim_launch_nuclear_warhead()
return {
name = 'my_side_effectful_preset_table',
...
}
end
}
then it would look really messy especially if you configured some presets from other plugins and added your own (maybe several ones?)
Agreed 100%. My argument was about also including a more intuitive way (as suggested just above) for the simpler "monolithic" setups, where splitting into multiple files would only get in the way, and add_preset
wouldn't seem to add extra value. Having said that, add_preset
gets the job done and I'm more than happy to keep using it now that I understand this plugin's configuration method a bit better. I'm only bringing it up is cause it tripped me when I was starting out.
Now, on to the feature request :)
@nkgm Sorry for the late response. My main concern wasn't about side-effects, but instead, it was about consistency.
For me, setup
and any other function solve 2 different problems.
You need setup
when you need to configure the way a plugin works or configure its extensions. In this plugin's case, 'extensions' are presets. For example in telescope
there're also extensions and you can configure them in the setup
function directly (like file search, grep etc).
But you don't need setup
when you want to initialize a plugin or add some extensions to it. As I said, I consider a preset to be an extension of reactive
, because basically what reactive
does is watching mode/window changes and then perform some actions based on that. If there're presets, it will use them, otherwise just skip.
In the code that you suggested, where you put the whole preset into the load
field, it would just create more confusion because:
add_preset
method that is a part of reactive
API, which allows you to add your own preset to the plugin.load_preset
method that is also a part of reactive
API, which allows you to load your preset from a specified directory. It can be convenient when you have several presets.setup
method with load
field, which should tell a user that it operates the same way the original load_preset
does, but within setup
method.I understand that the thing you suggested is a bit different, you want to have the same method as load
, but for adding new presets, but here my concerns about it:
setup
method only because you used those built-in presets, because there's no other way to get those presets except calling this method.add_preset
and be doneHowever, even though you used those built-in presets and they really fit your needs (what I really like), they exist just for demonstrating purposes and one's just encouraged to use their own ones. Since they're built-in, they can be just changed in the future, although I don't have any plans for that now. If you really like them, I would just suggest copy-pasting and adding those into your own preset. It will even increase performance, because instead of having 3 + 1 presets, you'll have only 1. But yeah I know that performance when iterating over 4 objects instead of 1 is just a stupid thing to consider haha.
But if you do it (I mean copy-pasting), you can just use one add_preset
or load_preset
method and be done. No overhead with calling different methods etc, just one. And since you're not configuring anything, just adding your own preset, you don't need the setup
method at all.
Hope this somehow explains what I was about :)
I'm trying to set a yellowish
CursorLine
color forterminal
buffers using the following (naive?) config. When nvim starts, theCursorLine
is blue as expected. The minute I open aterminal
buffer,CursorLine
will remain yellow for every window regardless ofbuftype
inn
mode (i
works as expected). I've logged the output ofskip
and verified it returns the correct result each time. What am I doing wrong?