chore(mappings): remove mappings

[alex-courtis]

This may be conceptually flawed.

Problem

There are many ways to specify a mapping key e.g. <c-k> <C-K> <TaB> <tab>.
remove_keymaps and { key = { "x", }, action = "" }, must exactly match the key specified in the default mappings.
This has caused users many issues.

Solution

Map the default then delete it. Vim will take care of any translation of key strings.
Vim gets upset when you try to delete an inexistent mapping, in the case of the user specifying a mapping that does not exist. The workaround is to create a dummy mapping before deleting.

This is not efficient but it is more foolproof.

Sanity check would be gratefully appreciated.

[alex-courtis]

This function is the interesting part.

[gegoune]

@alex-courtis Is this PR relevant in light of moving to on_attach()? I would prefer to simplify things and go gitsigns way, rather than going through hoops like this. What do you think?

[alex-courtis]

@alex-courtis Is this PR relevant in light of moving to on_attach()? I would prefer to simplify things and go gitsigns way, rather than going through hoops like this. What do you think?

I'm assuming you mean having the user simply unmap the defaults in their on_attach. That is the optimal method and the one we should document.

This one's just to support the legacy action = "" and remove_keymaps = { cases. I was thinking that we would retain remove_keymaps however it is not needed and can be deprecated too.

I've... not actually used gitsigns on_attach, just global mappings. Migrating now so that I can get a better feel for on_attach.

[alex-courtis]

Migrated gitsigns to on_attach: alex-courtis/arch@30e30bb#diff-d658f59261c2a32a47ad7ffd4dd673ebb3fc9707718115ce84de59bc3069a5cf

gitsigns uses nvim_buf_set_keymap which is awkward:
vim.api.nvim_buf_set_keymap(bufnr, 'n', 'hs', '<cmd>lua require"gitsigns".stage_hunk()<CR>', {})

If we're nice we might raise a doc PR for them:
vim.keymap.set("n", leader .. "hs", gitsigns.stage_hunk, ...)

[gegoune]

@alex-courtis Is this PR relevant in light of moving to on_attach()? I would prefer to simplify things and go gitsigns way, rather than going through hoops like this. What do you think?

I'm assuming you mean having the user simply unmap the defaults in their on_attach. That is the optimal method and the one we should document.

My understanding was that there will be no defaults, but we will document current ones and users can put them in on_attach, same way gitsigns does it. There will be no need to handle removal of default mappings at all as there will be no defaults. Just simplifies things from plugin perspective.

[alex-courtis]

@alex-courtis Is this PR relevant in light of moving to on_attach()? I would prefer to simplify things and go gitsigns way, rather than going through hoops like this. What do you think?

I'm assuming you mean having the user simply unmap the defaults in their on_attach. That is the optimal method and the one we should document.

My understanding was that there will be no defaults, but we will document current ones and users can put them in on_attach, same way gitsigns does it. There will be no need to handle removal of default mappings at all as there will be no defaults. Just simplifies things from plugin perspective.

Ah I see... full control for the user. Simple.

I'm not sure we can avoid defaults; the first time user setup will be a lot more difficult. What about something like:

  local my_on_attach = function(bufnr)
    Api.mappings.defaults()

    vim.keymap.set("n", "<C-P>", function()
      local node = Lib.get_node_at_cursor()
      print(node.absolute_path)
    end, { buffer = bufnr, noremap = true, silent = true, nowait = true, desc = "my description" })
  end

Edit: that API is a bad idea. It's unnecessarily complex and opaque. on_attach should have fully explicit defaults or no defaults.

[gegoune]

We can still have defaults for other values, but no key mappings at all. See https://github.com/lewis6991/gitsigns.nvim#keymaps

What we would have to do though is present current defaults in documentation so it can be copied over and modified as required. Could be link to wiki from documentation and readme if it turns out to be too long for those documents.

That will also allow whichkey plugins users set up any description they want.

[alex-courtis]

We can still have defaults for other values, but no key mappings at all. See https://github.com/lewis6991/gitsigns.nvim#keymaps

What we would have to do though is present current defaults in documentation so it can be copied over and modified as required. Could be link to wiki from documentation and readme if it turns out to be too long for those documents.

Yes. Sensible defaults or fully user driven.

I'll merge the legacy migration bits of this PR then create a branch for the above behaviour.

Updated #1579

That will also allow whichkey plugins users set up any description they want.

whichkey should work with the defaults, the generated legacy and user's mappings. Of course we haven't actually tested that yet...

[alex-courtis]

I tried commenting out my gitsigns on_attach and found that there are no gitsigns mappings at all.

That makes sense for gitsigns but isn't a great nvim-tree experience.

We can have the best of both worlds.

[gegoune]

That makes sense for gitsigns but isn't a great nvim-tree experience.

This is a very fair point.

[alex-courtis]

That makes sense for gitsigns but isn't a great nvim-tree experience.

This is a very fair point.

The new user workflow should be good:

• call setup with no or a few options
• use for a while with default mappings
• copy the template on_attach and unmap / add custom mappings

remove_keymaps is now legacy. on_attach is the one and only means to map/implicitly unmap.