-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(defaults): add LSP default mappings (again) #28650
Conversation
7e0e749
to
6e3bc34
Compare
6e3bc34
to
7d936c3
Compare
Argument against |
I don't think the same argument applies to plugins that map Meanwhile, another argument for using |
Ah I see what you mean. I am referring to the fact that if we create new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gr
Argument against
gr
prefix is that if a user does want to restore the builtingr
behavior, they would have to delete all of the mappings under thegr
prefix
That needs to be solved some other way. It is a temporary problem.
- "r" ("refactor") is a more appropriate mnemonic than "l" ("language").
- "gl" will be useful for other purposes. Whereas there is no alternative, planned use-case for "gr".
gl
is easier to type
"grr" and "gra" can be typed with one hand on querty, which is relevant for refactoring-related mappings which may involve the mouse.
Actually, that can be solved by |
"gr" could be used for LSP references |
the upstream pr is reverted: neovim/neovim#28649 alternative pr for 0.11 neovim/neovim#28650
Current proposal is:
In Insert mode:
|
6c02095
to
ba00ff3
Compare
ba00ff3
to
f35c05c
Compare
Being discussed in neovim/neovim#28650.
That is completely missing the point. |
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. |
The
"LSP goto stuff" is just
The proposal here is to use |
Well LSP goto stuff can also include implementation, type definiton, defition, declaration, which could possibly also have mappings.
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 |
Yes, because default mappings have much greater constraints than arbitrary user mappings. If we make 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. |
runtime/doc/lsp.txt
Outdated
- CTRL-S is mapped in Insert mode to |vim.lsp.buf.signature_help()| | ||
|
||
If not wanted, these keymaps can be removed at any time using | ||
|vim.keymap.del()| or |:unmap|. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for gr
we may want to mention nnoremap <nowait> gr gr
(mentioned by zeer), which avoids needing to unmap every gr*
mapping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem to work. If I use that mapping then I'm not able to use any of the new defaults. It reverts to the builtin gr
behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I use vim.keymap.set('n', 'gr', 'gr', { nowait = true })
before creating the other gr
mappings, then I can still use the new mappings, but it doesn't seem to behave any differently than it does without the <nowait>
mapping. So I'm not sure what it's meant to be doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem to work. If I use that mapping then I'm not able to use any of the new defaults. It reverts to the builtin
gr
behavior.
That's exactly what is supposed to do: conveniently disable all these mappings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that makes sense, I misunderstood the original suggestion. I’ll include that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not setting What determines the selection of these mappings? |
it's C-] by default. there is a proposal to map it to gd in #28476 |
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 thancr
. 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 usegla
orga
for "align", falling back to:ascii
for the defaultga
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.