kosayoda / nvim-lightbulb

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

Create autocmd in this package #32

Closed yochem closed 2 years ago

yochem commented 2 years ago

Dear @kosayoda,

Would it be an idea to let the package create the autocmd, instead of the user? This requires less configuration for the user and makes the package work more 'out the box'. By giving the autocommand a group name that matches the package name, it is also more clear where the autocmd is made and belongs to. I think it's fairly easy to implement it, just add this to the setup function:

local id = vim.api.nvim_create_augroup('nvim-lightbulb', {})
vim.api.nvim_create_autocmd({'CursorHold', 'CursorHoldI'}, {
    callback = M.update_lightbulb, -- idk if M. should be used here or something else
    group = id
})

The drawback is that users do not have control anymore over 1) on which events it will update, and 2) on which file patterns. If you really want users to have control over this, it can be added to the config passed to the setup function.

Let me know what you think!

kosayoda commented 2 years ago

The drawback is that users do not have control anymore over 1) on which events it will update, and 2) on which file patterns. If you really want users to have control over this, it can be added to the config passed to the setup function.

This is precisely why I left the autocmd creation up to the user. I'd accept a PR providing this feature (ie. automatically create the autocmd), but it must be behind a configuration key, make it opt-in. Thanks!

yochem commented 2 years ago

Sure! I'll work on it later this month. So for my understanding, something like a create_autocmd = true in the config will do? and then also add a configuration for events (autocmd_events) and file patterns (autocmd_pattern)?

kosayoda commented 2 years ago

Ideally to fit the existing configuration structure, we'd have something like an autocmd key, with the value being a table with keys enabled, events, patterns.

Since we have setup configuration now (courtesy of @sebastianvera), the config key should only be allowed during setup and not per-call, since I don't think recreating the autocmd every call is a good idea.

We should also create an augroup to place the autocmd in. Feel free to work on some or all parts of this, and let me know if you need help finishing it.