neovim icon indicating copy to clipboard operation
neovim copied to clipboard

feat(defaults): add LSP default mappings (again)

Open gpanders opened this issue 1 year ago • 11 comments

This is the PR to re-introduce default LSP mappings after being reverted in #28649. The goal is to merge this early in the 0.11 release cycle.

I've changed the defaults to use the gl prefix rather than cr. This avoids all of the messiness with operator-pending mode that was discussed in #28634. We had originally wanted to "reserve" gl for a future "align" feature, but using it as the general purpose "LSP prefix" might be more high leverage (and we can always use gla or ga for "align", falling back to :ascii for the default ga behavior).

We can still revisit these mappings (I think in particular the <C-S> mapping in Insert mode is a bit controversial), but at some point an executive decision will need to be made. We can't please everyone with defaults, so some people are going to be unhappy, unfortunately, but the hope is that providing reasonable defaults makes the onboarding experience easier for more users.

gpanders avatar May 06 '24 13:05 gpanders

Argument against gr prefix is that if a user does want to restore the builtin gr behavior, they would have to delete all of the mappings under the gr prefix (although I suppose the same argument applies to anyone using a plugin that currently maps gl, so maybe that's actually not a very strong argument).

gpanders avatar May 06 '24 13:05 gpanders

I don't think the same argument applies to plugins that map gl. The builtin gr can be following by any character, but that's not likely to apply to plugins that map gl.

Meanwhile, another argument for using gl instead of gr is that gl is easier to type.

zeertzjq avatar May 06 '24 13:05 zeertzjq

I don't think the same argument applies to plugins that map gl. The builtin gr can be following by any character, but that's not likely to apply to plugins that map gl.

Ah I see what you mean. I am referring to the fact that if we create new gl prefixed keys, then pressing gl will require a delay to use the gl behavior provided by the plugin. If a user wants the mapping to work as before, they have to delete all other keymaps that are prefixed with gl (or change the mapping to gla or something, which is probably what I will do if this goes through).

gpanders avatar May 06 '24 13:05 gpanders

Actually, that can be solved by nnoremap <nowait> gr gr.

zeertzjq avatar May 06 '24 13:05 zeertzjq

Whereas there is no alternative, planned use-case for "gr".

"gr" could be used for LSP references

famiu avatar May 06 '24 15:05 famiu

Whereas there is no alternative, planned use-case for "gr".

"gr" could be used for LSP references

grr is LSP references. gr becomes a prefix for a group of related mappings.

Current proposal is:

grr -> vim.lsp.buf.references() gra -> vim.lsp.buf.code_action() (works in Visual mode too) grn -> vim.lsp.buf.rename()

In Insert mode:

<C-S> -> vim.lsp.buf.signature_help()

gpanders avatar May 22 '24 13:05 gpanders

Whereas there is no alternative, planned use-case for "gr".

"gr" could be used for LSP references

That is completely missing the point.

justinmk avatar May 23 '24 10:05 justinmk

gr is very commonly used for LSP references (see for example https://www.lazyvim.org/keymaps#lsp, https://github.com/nvim-lua/kickstart.nvim/blob/master/init.lua#L475 its also recommended mapping by bunch of other stuff). and g as prefix is often used as goto (see native gg, gd, gD) so stuff like code actions, rename (and possibly) format imo do not rly makes sense behind it.

Imo the lsp refactor stuff and lsp goto stuff should have separate prefixes as its 2 very different operations but thats just my 2 cents.

deathbeam avatar May 23 '24 20:05 deathbeam

and g as prefix is often used as goto (see native gg, gd, gD)

The g prefix does not mean goto. gs, gp, ga, g8, g?, gJ, gu, gU, gv, gq, gw, and g~ are all counter examples.

Imo the lsp refactor stuff and lsp goto stuff

"LSP goto stuff" is just 'tagfunc', which uses the builtin mappings from :h tag.

gr is very commonly used for LSP references

The proposal here is to use grr for references. Is moving from gr to grr that much of a problem?

gpanders avatar May 23 '24 20:05 gpanders

Imo the lsp refactor stuff and lsp goto stuff

"LSP goto stuff" is just 'tagfunc', which uses the builtin mappings from :h tag.

Well LSP goto stuff can also include implementation, type definiton, defition, declaration, which could possibly also have mappings.

gr is very commonly used for LSP references

The proposal here is to use grr for references. Is moving from gr to grr that much of a problem?

Well is there any reason to not use the mappings that ppl are already used to? The mappings are unused and LSP being integral part of neovim I think justifies not putting the whole thing behind single prefix (especially when gr doesnt even mean anything here that could be used as mnemonic)

deathbeam avatar May 23 '24 20:05 deathbeam

Well is there any reason to not use the mappings that ppl are already used to?

Yes, because default mappings have much greater constraints than arbitrary user mappings. If we make gr map to references, we cannot use gr as a prefix for anything else. So instead of grr, gra, and grn (3 default mappings), we have gr (1 mapping) and have to find separate mappings for the other two. Finding unused default mappings is extremely difficult (if you've been following the "new LSP defaults" saga then you will have observed this).

I will be 100% clear about this (and I will probably end up repeating myself): it is not possible, nor is there any expectation, that we will find default mappings that make everyone happy. The goal is to provide a better out of the box experience for new and less experienced users by providing access to builtin Neovim capabilities without needing much (or any) additional configuration. Experienced and power users will likely have separate preferences, but they already know how to create and delete mappings, so they are free to ignore the defaults.

gpanders avatar May 23 '24 20:05 gpanders

This is not setting definition Is this already covered by something else? Otherwise, imo, definition and references are the most important ones.

What determines the selection of these mappings?

bluebrown avatar May 25 '24 10:05 bluebrown

Is this already covered by something else

it's C-] by default. there is a proposal to map it to gd in #28476

arcxio avatar May 25 '24 10:05 arcxio