mrjones2014 / legendary.nvim

🗺️ A legend for your keymaps, commands, and autocmds, integrates with which-key.nvim, lazy.nvim, and more.
MIT License
1.13k stars 19 forks source link

[Feature]: plugin integrations #183

Closed mrjones2014 closed 1 year ago

mrjones2014 commented 1 year ago

Similar Issues

Description

Just toying with an idea, what if we could integrate with various other plugins that add default keymaps? For example diffview.nvim adds a bunch of keymaps when diffview is open, we could built an integration that would automatically show them in the legendary finder.

doctoromer commented 1 year ago

I tried to integrate it with packer.nvim and failed. My goal is to have keymaps of lazy-loaded plugins displayed in the legendary API before the plugin is actually loaded. I tried to use the helpers module, but it clashes with packer's bindings for lazy loading.

mrjones2014 commented 1 year ago

For now you should be able to manually add the keymaps to legendary. There isn’t a great way to do it automatically at the moment.

I need to put a lot more thought into this.

doctoromer commented 1 year ago

Is there a way to add a keymap without actually mapping it? My goal is to prevent collision with packer's lazy-loaded-by-keys plugins.

mrjones2014 commented 1 year ago

Yes, just don’t provide an implementation (2nd list element), just keys and description. See: https://github.com/mrjones2014/legendary.nvim/blob/master/doc/table_structures/KEYMAPS.md

doctoromer commented 1 year ago

Oh, your'e right. I must have missed that. Thank you very much!

mrjones2014 commented 1 year ago

We’ll be starting with nvim-tree. My comment from #289:

Hey there, this is awesome, thanks for filing the issue!

This may be better suited actually for #184, nvim-tree is probably a good candidate for a first one for a proof of concept.

I'm not sure exactly what they will look like, but I have a few ideas. The best idea I have currently is along the lines of:

Legendary.nvim must be loaded before other plugins. We have a set of plugins, or plugins can be provided by the other plugin itself, at legendary.plugins.[plugin name]. This way, plugins can be looked up dynamically; that is, if they wanted to, nvim-tree could provide its own legendary.nvim integration by adding lua/legendary/plugins/nvim_tree.lua in their repo, just for an example.

The plugin is just a plugin-specific script that must handle registering its keymaps, commands, autocmds, and functions. It may set up autocmds to do so on BufEnter for example, or it may call some custom logic to determine the correct buffer to attach to.

So for this example, and nvim-tree / legendary.nvim plugin would look like:

User's config:

require('legendary').setup({
  plugins = {
    nvim_tree = true,
  }
})

File: lua/legendary/plugins/nvim_tree.lua

vim.api.nvim_create_autocmd('FileType', {
  pattern = 'NvimTree',
  once = true,
  callback = function()
        local   nvimtreemappings = require("nvim-tree.keymap").keymaps
        local   keystodisable    = vim.tbl_map(function(e) return e.key end, nvimtreemappings)
        -- Set up Legendary plugin keybindigns
        local keymaps = vim.tbl_map(function(e)
            -- just set up a description-only keymap
            local entry = {}
            entry.mode  = "n"
            entry[1]    = e.key        -- Key
            if type(e.key) == "table" then entry[1] = e.key[1] end
            entry.description = "Nvim-tree: " .. e.desc
            entry.opts = { buffer = bufnr, noremap = true }
            return entry
        end, nvimtreemappings)
        require('legendary').keymaps(keymaps)
  end
})
mrjones2014 commented 1 year ago

See also:

hinell commented 1 year ago

@mrjones2014 You may also want to take a look at how persisted.nvim deals with Telescope integration over here.

hinell commented 1 year ago

@mrjones2014 Minor feedback here. I think Legendary dont' needs a sophisticated module-resolving system akin to Telescope one. The best way to handle plugins IMO by registering a plugin table (object) with a meta information and init function that is provided by api instance e.g.:

   local legendary = require("legendary")
   legendary.plugins:add({
      name = "FooPlugin",
      init = function(api)
         api.register({ keymaps = { ... } })
      end
)

I have seen this approach many times in Web development tools.

mrjones2014 commented 1 year ago

I disagree.

This doesn't allow the user to easily load only the plugins they want, unless you mean the user will be writing the snippet you gave above themselves, in which case I think that's just really un-ergonomic. This also doesn't easily allow other plugin authors to provide their own legendary.nvim plugin integrations.

I have seen this approach many times in Web development tools.

That's an extremely different use case.

hinell commented 1 year ago

@mrjones2014 Flexibility may be achieved both by consumers and plugin-writers:

    -- init.lua
    local PluginA = require("cool/legendary/extensionA")
    local PluginB = require("cool/legendary/extensionB")
    local PluginC = require("myExtensions/legendary/keymaps")

    legendary.plugins:add(PluginA)
    legendary.plugins:add(PluginB)
    legendary.plugins:add(PluginC)
    legendary.plugins:add({
        name = "foo-keymaps"
        init = function(legendaryApi) ... end
    })
mrjones2014 commented 1 year ago

That's a lot less ergonomic than just being able to do:

require('legendary').setup({
  plugins = {
    plugin_a = true,
    plugin_b = true,
  },
})

It also potentially breaks lazy-loading setups.

Is there any actual downside to a module resolution system like I described above? I see only benefits, and I see many downsides with your suggested approach, with no benefit over the module resolution system.

hinell commented 1 year ago

@mrjones2014 Well this way it's too simplified. It's going to be a nightmare if two different modules use the same name. It also deprives user from a control over module system.

This way you also can't add plugins on the fly.

Upd: and I don't even know how you are going to sanely debug plugins cause this way of configuration is basically a black box

hinell commented 1 year ago

Any progress on this?

I have a question about package manager status intergration. Like, how would a third-party plugin check that Legendary.nvim is installed? In packer we can do:

local L = packer_plugins["legendary.nvim"]
if L and L.loaded) then
 ...
end

UPD: Turns out we can use pcall() like:

local ok, mod = pcall(require, "Legendary")
mrjones2014 commented 1 year ago

Haven’t made any progress yet, I have been abroad in the Netherlands, so I haven’t had time to work on open source lately.

mrjones2014 commented 1 year ago

Looking into this again, we also need: https://github.com/nvim-tree/nvim-tree.lua/issues/1895

mrjones2014 commented 1 year ago

We could technically do it by re-binding the keymaps in nvim-tree's on_attach but then we'd be creating duplicate keymaps every time you open nvim-tree.

mrjones2014 commented 1 year ago

This should be possible now, by setting the keymaps generated to have filters = { filetype = 'NvimTree' }

hinell commented 1 year ago

@mrjones2014 Please add docs & examples on how to implement extensions. Thanks.

Upd, seems like it's done via filters. Not very obvious....

mrjones2014 commented 1 year ago

Filters are one of a few tools you can use. I consider the API unstable currently, as is noted in the docs. I’m still iterating in it, I will add thorough docs once I feel like it’s in a good place.

airtonix commented 1 year ago

@mrjones2014 Flexibility may be achieved both by consumers and plugin-writers:

    -- init.lua
    local PluginA = require("cool/legendary/extensionA")
    local PluginB = require("cool/legendary/extensionB")
    local PluginC = require("myExtensions/legendary/keymaps")

    legendary.plugins:add(PluginA)
    legendary.plugins:add(PluginB)
    legendary.plugins:add(PluginC)
    legendary.plugins:add({
        name = "foo-keymaps"
        init = function(legendaryApi) ... end
    })

That's a lot less ergonomic than just being able to do:

require('legendary').setup({
  plugins = {
    plugin_a = true,
    plugin_b = true,
  },
})

It also potentially breaks lazy-loading setups.

Is there any actual downside to a module resolution system like I described above? I see only benefits, and I see many downsides with your suggested approach, with no benefit over the module resolution system.

This is the composition vs configuration as convetion approach.

ruby on rails went down the latter. you'll end spending the rest of your life tweaking offical default configs due to n+inifity requests in tickets.

I mean, you should step back and rexamine what it really is you're trying to achieve:

I dare say the actual goal here is :

registering other plugins keymaps so the user has superior UX?

In which case, have you discussed monkey patching the nvim keymap functions so you catch all and any keymaps? This seems entirely relevant to: https://github.com/mrjones2014/legendary.nvim/issues/258

mrjones2014 commented 1 year ago

a super god like config convention that's tricky: "lol i anticipated every possible edge case, so just put yourpluginname = true"

Tells me you have not bothered reading the documentation or look at the actual implementation that was merged, so that's super cool. Only a moron could think they've anticipated every possible edge case.

registering other plugins keymaps so the user has superior UX?

Yes that's the goal.

have you discussed monkey patching the nvim keymap functions so you catch all and any keymaps

This decreases control, it doesn't tell us the source of a keymap, and it doesn't allow users to have some but not all keymaps in their legendary.nvim config. Sometimes you don't want everything in the finder, just the stuff you don't use all the time or don't always remember the keymap or command.

Finally, don't be a dick :slightly_smiling_face:

hinell commented 1 year ago

Sorry for bothering, but it if ever legendary should not be responsible for configuring plugins. this should always be done at require("plugin").setup(). I just came from Telescope.nvim and can already tell you how bad it is. It would be a major design flaw.

Thanks.