elihunter173/dirbuf.nvim

Avoid creating default keymaps

sanka047 opened this issue · 6 comments

Personally, I tend to like to configure keymaps on my own (or have the ability to override defaults). The buffer-specific keymaps are fine, but the global mapping on - sometimes interferes (rarely) with my use of macros. I can contribute a PR, but I'll wait for feedback/discussion, since changing defaults might break other users' configs.

Possible solutions:

  • Adding a mapping table that gets used to setup mappings in dirbuf. To override defaults, simply set the default values to nil. This is similar to how many other neovim plugins handle mappings (see telescope, etc).
  • [PREFERRED] Add a config option to disable default mappings. Use the mapping table described above or an on_attach function (similar to how lspconfig, etc handles mappings) as mentioned in #38. This feels simpler? And I like the flexibility of an on_attach hook in general.

I can see adding a default_mappings option to dirbuf.setup() as mentioned in your preferred solution.

But before changing anything, what are your thoughts on the current behavior where default mappings are only created if no mapping to the <Plug> mapping exists and they don't conflict with other mappings? (The second condition is only relevant for global mappings.)

I’d be okay with that solution for buffer specific mappings. That sounds reasonable, but I’m not quite as sure for global mappings. If the user doesn’t want the mapping at all, and would prefer to just run :Dirbuf % instead, they’d want the global mapping to be completely disabled.

Admittedly, I’m biased towards letting the user configure mappings themselves since that’s always felt more “vim” to me 🤷

Maybe separate options for no_global_mappings and no_buffer_mappings?

If the user doesn’t want the mapping at all, and would prefer to just run :Dirbuf % instead, they’d want the global mapping to be completely disabled.

With the current behavior, a global keybindings can be disabled by remapping a key to itself, e.g. nnoremap - - to disable dirbuf.nvim's <Plug>(dirbuf_up) global mapping. This behavior was taken from vim-dirvish.

I do think this is a bit unintuitive and I'm currently leaning towards adding a default_mappings option which could be one of "all", "none", "global-only", and "buffer-only" with a default of "all". However, I'm torn then with this setting if default keybindings should always be created even if they clobber other keybindings or if the default keybindings should still be "polite" and only set themselves up if that mapping hasn't been taken yet (which global mappings currently do). I'm also not sure how I feel about changing the current behavior at all because currently keybindings can be disabled, even if it's somewhat unintuitive, and I want to avoid breaking configs in general.

Yeah, I think that's fair and mainly why I wanted to discuss options. How about the following:

default_mappings = {
    global = "none" | "respect_user_mappings" | "all",
    buffer = "none" | "respect_user_mappings" | "all",
}

I think respect_user_mappings name needs to be massaged a bit and would need clear documentation for each of the options in the README, but perhaps this offers enough granularity? And you can use respect_user_mappings as the default for both fields?

I can see adding a default_mappings option to dirbuf.setup() as mentioned in your preferred solution.

But before changing anything, what are your thoughts on the current behavior where default mappings are only created if no mapping to the <Plug> mapping exists and they don't conflict with other mappings? (The second condition is only relevant for global mappings.)

The problem with this approach is that it relies on the order in which plugins load, if two of them are trying to grab the same mapping. And that's not always predictable...

FWIW I would prefer a simple boolean of disable_default_mappings or similar.

By the way, I'm not sureon_attach is needed. A dirbuf.lua in after/ftplugins has worked well for me so far for setting other dirbuf-specific stuff. on_attach is most helpful for things like LSP/Treesitter which span many types of files, but the filetype for dirbuf is always predictable.

The - keymapping is violence — and I say this as someone who loves dirbuf!

Global keymaps should be configured only by request, in a config option, never by default. I'd only make an excuse for plugins where the keymap is the entire point of the plugin — i.e. something like Surround or Sneak (or Unimpaired, or something that provide extra text objects).

When I first discovered this, I just disabled dirbuf for a while instead of trying to figure how to change the keymap. I mean, all it was doing was exploding my current buffer and producing a ton of error messages, anyway. Users who do this might not stick around to find all the good that the plugin has to offer!