Improve bindings to better match VS-Code

[3ddyBoi]

Release Notes:

• Changed default keybindings in the VS Code keymap so that alt-[up|down] now move lines up/down andalt-shift-[up|down] duplicate lines up/down. Previous bindings for selecting larger/smaller syntax nodes are now bound to ctrl-shift-[left|right]. (#4652)(#7151)

[cla-bot]

We require contributors to sign our Contributor License Agreement, and we don't have @3ddyBoi on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

[3ddyBoi]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[mrnugget]

Hey! Thanks for the contribution! Merging this would be a breaking change, since it breaks existing keybindings (people do use alt-up/alt-down). And since we also don't strive for 100% compatibility with VS Code, I don't think it's worth it, especially since these keybindings can be changed again. So I'm going to close it since it doesn't seem like something that we want to do right now.

[texastoland]

CC @as-cii @maxbrunsfeld for reconsideration:

Merging this would be a breaking change,

I disagree with your assessment. It's a "fixing" change. The reason people use any given keymap is to avoid context switching. Until Zed is a full IDE replacement.

since it breaks existing keybindings (people do use alt-up/alt-down).

Copied from #4652 (comment):

It looks like alt-up and alt-down are taken by editor::SelectLargerSyntaxNode and editor::SelectSmallerSyntaxNode. But I think those are from JetBrains. The VS Code defaults should be:

VS Code has long provided default shortcuts for these exact commands (inspired by JetBrains products).

And since we also don't strive for 100% compatibility with VS Code

But it should 100% not break compatibility with VS Code. The mis-mapped shortcuts aren't even niche. They're ones I use every few minutes. DuplicateLine is another frustrating example. Not only is it mapped to a different shortcut but the shortcut it's mapped to is a different VS Code default.

especially since these keybindings can be changed again.

Didn't make sense. Is the implication that Microsoft willy nilly makes breaking changes? NGL I wish they would sometimes!

---

For context my first response to the broken keymap was to uninstall Zed. But I reinstalled it and really like it. I spent too much time identifying which ones were crossed, tracking down related issues, recommending the fix that motivated this PR, and finally following up here. I think a narrowly scoped PR that fixes a demonstrable UX issue for the intersection of {VS Code users} and {Zed users using the VS Code keymap} (all of them?) deserves further discussion.

[mrnugget]

Yeah, I see what you're saying. I guess the problem is that our default keymap is the VS Code keymap, but it's not 100% compatible, like you said.

So based on the comment you quoted (that alt-up/alt-down in their current form come from JetBrains), do you think it's better to move those to the jetbrains keymap and keep them there by default?

Didn't make sense. Is the implication that Microsoft willy nilly makes breaking changes? NGL I wish they would sometimes!

No, sorry, that wasn't clear on my part: I meant that people can change the keybindings in their own keymaps.json file any time. But I do agree with you that defaults are very important, especially since (as I realized only after your comment) our default keymap is called "VS Code keymap" when it's first presented to new users.

I think that convinces me and I'd reopen the PR (if we move those keybindings to JetBrains instead of deleting them, or maybe giving them a new mapping, like alt-shift-up/alt-shift-down?).

Curious now what @as-cii @maxbrunsfeld think here.

[mrnugget]

This PR over here changes the alt-up/alt-down bindings and also adds replacement: #8749

[3ddyBoi]

Should we continue in the other PR if it was more complete than this? If not I can fix the changes you requested in your comment and complete this one.

or maybe giving them a new mapping, like alt-shift-up/alt-shift-down

In VSCode these are taken by duplicate line up/down.
I noticed I forgot to add alt shift up in this PR, but I can also do that.

[mrnugget]

Let's continue in here. I think what we need:

1. New keybinding for selecting larger/smaller syntax node
2. Keybinding for duplicating lines
3. Keybinding for moving lines

Then we need to make a decision whether we want to merge this, but let's wait for Antonio/Max here.

[isheebo]

is it possible to create a Zed-specific keymap and also a 100% VScode keymap that maybe used by those trying to transition? I've had challenges replicating certain functionalities that I am used to in VSCode.

In particular, toggling the terminal window in zed, works with cmd + j. In VSCode, that would be cmd + `. Though the latter can work in zed, it is not consistent as a terminal toggle.

Either creating a separate zed-specific keymap or exposing an option in the settings file, where the keybindings can be set.

[pimmee]

Chiming in to say these incorrect keymap is something that I also noticed immediately after moving to Zed from VSCode and selecting the VSCode keymap (specifically delete line and move line up/down), as these keybindings are something I use all the time.

I agree with the author that the expected behavior should be that these keymaps match VSCode if selecting VSCode keymap, which I think will make the transition even smoother for users in the future.

[3ddyBoi]

New keybinding for selecting larger/smaller syntax node

This would be ctrl-shift-right/left for VSCode. I have moved the old defaults into jet-brains keymap since it's a jet-brains bind.

Keybinding for duplicating lines

This would be alt-shift-up/down for VSCode. The old bind cmd-shift-d was under the comment // Bindings from Sublime Text but since sublime doesn't have it's own keymap and the default map is indicated to be like VSCode we could maybe just remove it.

Keybinding for moving lines

This would be alt-up/down for VSCode. The old bind ctrl-cmd-up/down was also under // Bindings from Sublime Text so maybe just remove this to since sublime doesn't have it's own keymap.

[3ddyBoi]

In particular, toggling the terminal window in zed, works with cmd + j. In VSCode, that would be cmd + `. Though the latter can work in zed, it is not consistent as a terminal toggle.

cmd + j in VSCode is Toggle Panel Visibility, in Zed its ToggleBottomDock I would argue this is a correct map, but it might be an idea to add a more specific bind to open terminal.

[texastoland]

toggling the terminal window in zed, works with cmd+j. In VSCode, that would be cmd+`.

This is already in Zed's default keymap.

VS Code has long provided default shortcuts for these exact commands (inspired by JetBrains products).

This would be ctrl-shift-right/left for VSCode.

Correct and linked above for reference.

[isheebo]

It is true that the key binding is already a part of Zed's keymap. In VScode, both ctrl + ` and cmd + j toggle the appearance of the lower bottom panel i.e. the terminal.

In Zed, pressing cmd + j consistently toggles the bottom panel but ctrl + ` does not.

The argument I was trying to make is that this behaviour is inconsistent with VScode where pressing either of the commands in quick succession toggles the visibility of the bottom panel (terminal).

[texastoland]

@isheebo I recommend opening a focused issue to not sidetrack this PR 👍🏼

[maxbrunsfeld]

I think we should make this change, even though it will be somewhat disruptive. Since we call it the VS Code keymap, we should match VS Code where possible. There are always going to be some concepts that are not 1:1, but all of the bindings changed here.

The disruption stems from mistakes made a long time ago, before we had separate default keycaps for VS Code, JetBrains, etc, and we didn't match VS Code consistently. I'll personally have to get used to ctrl-shift-right instead of alt-up, but I can deal.

[maxbrunsfeld]

I can see why you had to do this, but I don't like using SendKeystrokes in this file. Could you add an optional field to the DuplicateLine action instead? It could be done similarly to the advance_downwards field of the ToggleComments action below. For compatibility, we'll need to annotate the field with #[serde(default)], to make it optional in key binding files.

[texastoland]

PS thanks @mrnugget and @maxbrunsfeld both for a second look 🙏🏼 I appreciate the sensitivity that it not only emulates VS Code but is also the default for everyone. In fairness the only reason I learned VS Code shortcuts years ago was because their JetBrains keymap was lacking. There's certainly room to carve out a dedicated Zed keymap in the future 🔥

[3ddyBoi]

I wrote some logic, but I did not have time to modify the tests for the duplicate upwards logic.

[mrnugget]

I hopped on the branch, implemented the behavior for DuplicateLine { move_upwards: true} and added tests. I also updated the release notes in the PR description.

But I'm not going to merge it yet because I'm not super happy with the move_upwards field name but couldn't come up with a better name. What it does in practice is keep_cursor_position, but I'm not sure that's a better. If someone has any thoughts, let me know, otherwise I'll try to think about it this afternoon and if no lightbulb moment happens, we can merge.

[mrnugget]

I can't come up with a better name than move_upwards. @maxbrunsfeld do you have one? In any case: waiting for your review/approval.

[3ddyBoi]

Maybe anchor_cursor?

[maxbrunsfeld]

move_upwards works for me. If we want to rename it later, we can always add a serde alias for backwards-compatibility.