search

kosayoda / nvim-lightbulb

VSCode 💡 for neovim's built-in LSP.
MIT License
801 stars 30 forks source link

Separate configuration setup and actual usage #2

Closed weilbith closed 2 years ago

weilbith commented 3 years ago

Hey :wave: Cool minimal plugin you have written here. :+1: I was wondering what you think about having a separate function to change the configurations and one to display the bulb. It looks like many Lua plugins follow this pattern. But it is just an idea, as I felt like this would be cool while setting the plugin up locally.

kosayoda commented 3 years ago

Hi! Thanks for the suggestion. I'm not sure what benefits this would bring though? My rationale for having the configuration be in the call to the lightbulb update was that you could set your own autocommands to use different settings for diffferent filetypes/contexts.

weilbith commented 3 years ago

Hmm. Maybe you are right. Its just a little awkward to have such a huge augroup due to the multiline configuration object. But as I said it mainly was due to some structure. But this would also cause more complexity in the plugins code... :thinking: If nobody else likes the idea I think we should just close the issue. :smiley:

kosayoda commented 3 years ago

The defaults I think are pretty sane, and configuration shouldn't take a lot more than a couple of lines really at this point. To make things neater, you could always write a wrapper function like:

LightBulbFunc = function()
    require("nvim-lightbulb").update_lightbulb {
        -- Gigantic configuration
    }
end

then using that in the augroup.

If nobody else likes the idea I think we should just close the issue.

No harm leaving it up, it's not like it's cluttering the issues page xD

wbthomason commented 3 years ago

Just wanted to add on here: maybe a reasonable option would be to add a separate configuration function to be used to set default settings, but keep the signature of update_lightbulb to allow per-filetype/context customization?

Of course, as you mention, writing a wrapper is always an option too.

kosayoda commented 3 years ago

Just wanted to add on here: maybe a reasonable option would be to add a separate configuration function to be used to set default settings, but keep the signature of update_lightbulb to allow per-filetype/context customization?

That could work; I do not have the time to implement this in the near future, but would gladly accept a PR that preserves backwards compatibility.