feat(mapping): deprecate user mappings and add on_attach

[kyazdani42]

See the help doc. I think this is better than providing our custom binding methods
The idea was taken from gitsigns.nvim.
We have a new keymap module that uses the API directly. I've taken the same map from the actions/init.lua file and migrated the action keys to callback keys. We will need to update the script for the default mappings too once this lands.

[alex-courtis]

To confirm: we are completely removing actions e.g. edit_in_place and the user's mappings will call the API directly.

This is the right way forward. Users invoking nvim_buf_set_keymap directly is obvious and clear and will be more future proof.
The mapping / action code is messy and can be difficult / unintuitive for the user. This makes the problem Go Away.

Its missing a way for users to reuse the existing actions.

This shouldn't be a problem if they are mapping directly to the API.

We will need to provide all the default mappings in the help, so that users can copy-paste to create their own e.g.

vim.api.nvim_buf_set_keymap(bufnr, 'n', 'hs', '<cmd>lua require"gitsigns".stage_hunk()<CR>', {})
vim.api.nvim_buf_set_keymap(bufnr, 'n', 'Y', '<cmd>lua require"nvim-tree.actions.fs.copy-paste".copy_path()<CR>', {})

Users could not use regular vimscript keymaps, however that is acceptable.

users can easily migrate to this

We will need to create a page to explain the migration process with examples.

We will unfortunately need to provide a legacy migration path, pointing them to the migration doc. Users will be very upset if we don't do this.

Perhaps we could provide a function to convert their existing mappings to nvim_buf_set_keymaps...

We could start thinking on how to handle a public API for the plugin.

We can do that at any time before this. It's on my list ;)

[kyazdani42]

@alex-courtis i've migrated the keymap code to a new module. We will do a 2 step deprecation, 1st switching the default to the on_attach (in the next week or two) with a warning message, and in a month or two we'll remove the whole code and options related to this.
For now we can leave it like this, so users that wish to try this can, and we make sure the UX is all right, then we'll do the migration.
We could also write a script that will generate the new configuration for the user. Might not be too complicated.
We might need to take time and notice maintainers of things like lunarvim because they have internal stuff to change and it might take some time for them to implement.

[alex-courtis]

on_attach (in the next week or two) with a warning message, and in a month or two we'll remove the whole code and options related to this.

Let's wait until we have completed everything and are ready to cut over to the new keymap. You know how upset users get when we show warnings ;)

There may be an unforseen issue that requires us to make an api or on_attach change, which is OK for early adopters but not for everyone.

We might need to take time and notice maintainers of things like lunarvim because they have internal stuff to change and it might take some time for them to implement.

Yes. We can give them some time; there is no rush to remove the legacy mappings.

[alex-courtis]

Pushed a draft mechanism to write current mappings -> on_attach

Feel free to revert / change etc.

Run :NvimTreeCreateOnAttach

We could probably just simplify the helper to

  local function map(key, fn)
    vim.keymap.set("n", key, function(...)
      fn(...)
    end, { buffer = bufnr, noremap = true })
  end

however I'm not sure what else you have in mind with the mappings...

[alex-courtis]

Not sure how we are going to do the DEFAULT_KEYMAPS for update-help.sh

Might have to do something horrible like:

  {
    key = { "<CR>", "o", "<2-LeftMouse>" },
    callback = { "node", "open", "edit", },
    desc = "open a file or folder; root will cd to the above directory",
  },

...

function M.set_keymaps(bufnr)
  local opts = { noremap = true, silent = true, nowait = true, buffer = bufnr }
  for _, km in ipairs(M.keymaps) do
    local keys = type(km.key) == "table" and km.key or { km.key }
    for _, key in ipairs(keys) do
      local fn = Api
      for _, t in ipairs(km.callback) do
        fn = fn[t]
      end
      vim.keymap.set("n", key, fn, opts)
    end
  end
end

...

[kyazdani42]

i'm moving your commit in a separate PR so we can discuss it more. I think we can probably have a better strategy for migrating user configs under the hood too.
I think the default keymap is not too problematic, we could simply stringify the lua code for the callback.
I'm merging this since it's non breaking. let's iterate :)

[alex-courtis]

i'm moving your commit in a separate PR so we can discuss it more. I think we can probably have a better strategy for migrating user configs under the hood too. I think the default keymap is not too problematic, we could simply stringify the lua code for the callback. I'm merging this since it's non breaking. let's iterate :)

This was the right desicion. Early feedback is useful. #1461