Skip to content
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

Add support for ctrl+w in insert mode #453

Merged
merged 2 commits into from
Jul 16, 2016
Merged

Conversation

ascandella
Copy link
Contributor

This deletes backwards, and is for some reason one of my most commonly
used keyboard shortcuts.

Fixes #452

@@ -357,7 +357,22 @@ class CommandEsc extends BaseCommand {

@RegisterAction
class CommandCtrlOpenBracket extends CommandEsc {
keys = ["ctrl+["]
keys = ["ctrl+["];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but necessary to get gulp tslint passing locally.

@ascandella
Copy link
Contributor Author

ascandella commented Jul 16, 2016

Note: I see that for some other control- commands, there are references in extension.ts and package.json, e.g.:

"command": "extension.vim_ctrl+b",

I don't fully understand what these do. I tested this change interactively in the debug editor, so I know it works, I guess these give people some knobs to twiddle? Or maybe this only happens to work because ctrl+w is already registered for vim_switchWindow?

@rebornix
Copy link
Member

rebornix commented Jul 16, 2016

Here is the thing, we are listening to vscode's type event but it will not pass any modifier to this event, which means, we can't catch ctrl keystroke anyway. The only suggested way to handle those keys are using key bindings.

So in our case, we translate ctrl-w to switch_window. But since I didn't know we can use ctrl-w to perform delete backwards so I made this key binding apply to all Vim Modes. While what you need to is simply add restriction to switch_window by checking vim.mode, see samples here. Then finally add a new contribution in this package.json file for ctrl+w in Insert Mode.

@ascandella
Copy link
Contributor Author

Makes sense. I'm a little confused by https://github.com/VSCodeVim/Vim/blob/master/extension.ts#L102

'rfbduc['.split('').forEach(key => {
  registerCommand(context, `extension.vim_ctrl+${key}`, () => handleKeyEvent(`ctrl+${
});

Which appears to register the various control keys, but notably w is not one of them. I'll update package.json now.

@rebornix
Copy link
Member

rebornix commented Jul 16, 2016

@sectioneight good catch man. Just move w to the list. Both lines are added by me but I don't see any reason to keep ctrl+w specific.

@rebornix
Copy link
Member

@sectioneight miss an important link in previous comments, take a look at VS Code key binding doc then you can have an idea which keys will be passed to type or not and which keys should be handled in contribution always.

@ascandella
Copy link
Contributor Author

Thanks for the pointers! Can you take a look at the most recent diff? I think we're on the same page (making ctrl+w look like the rest of the ctrl+* keys).

@rebornix
Copy link
Member

LGTM 💯

@jpoon
Copy link
Member

jpoon commented Jul 16, 2016

One last thing, would you mind resolving the conflicts with master?

@ascandella
Copy link
Contributor Author

@jpoon yup, updated, I think we both fixed the tslint error in different branches :)

This deletes backwards, and is for some reason one of my most commonly
used keyboard shortcuts.
@jpoon jpoon merged commit aa8f605 into VSCodeVim:master Jul 16, 2016
@jpoon
Copy link
Member

jpoon commented Jul 16, 2016

@sectioneight 🎉

@ascandella
Copy link
Contributor Author

Dope. Really happy with how easy it is to implement features. Looking
forward to helping getting the rest of my favorites in :)
On Fri, Jul 15, 2016 at 9:37 PM Jason Poon [email protected] wrote:

@sectioneight https://github.com/sectioneight 🎉


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#453 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAF7P8jWemaS9VZKZH7QlisQ5s6EituBks5qWGAbgaJpZM4JN5Qm
.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants