Skip to content

Conversation

ulugbekna
Copy link
Contributor

@ulugbekna ulugbekna commented Apr 4, 2025

Hello! 👋

I'm a VS Code team member, who uses this extension daily and loves it! Thank you so much for this awesome extension! ❤️

What this PR does / why we need it:

From personal experience and through issue reports, we've figured that some Escape- and Tab-related keybindings do not play very well with VS Code features, so I thought it's worth creating a PR. I've self-hosted on my changes for several weeks and think they could be good quality-of-life improvement.

This PR fixes:

  • extension.vim_tab should not take priority over tab keybinding that accepts Next Edit Suggestions (aka Inline Edits; NES for short)
  • extension.vim_escape should not take priority over:
    • escape keybinding to hide inline suggestion/completion (eg. Copilot's ghost text completion)
    • escape keybinding to hide NES
    • escape to hide:
      • testing-explorer peek
      • suggest (aka intellisense) widget
      • find widget (escape currently cannot hide find widget if it's not focused)
      • dirty-diff widget
    • de-focusing notebook cell when vim is in normal mode

Which issue(s) this PR fixes

fixes #9490
fixes microsoft/vscode-copilot-release#7259
fixes microsoft/vscode-copilot-release#6004

Special notes for your reviewer:

…eybindings

fixes:
- `extension.vim_tab` should not take priority over `tab` keybinding that accepts Next Edit Suggestions (aka Inline Edits; NES for short)
- `extension.vim_escape` should not take priority over:
	- `escape` keybinding to hide inline suggestion/completion (eg. Copilot's ghost text completion)
	- `escape` keybinding to hide NES
	- `escape` to hide:
		- testing-explorer peek
		- suggest (aka intellisense) widget
		- find widget (escape currently cannot hide find widget if it's not focused)
		- dirty-diff widget
	- de-focusing notebook cell when vim is in normal mode
@ulugbekna
Copy link
Contributor Author

The only scenario where I'm not sure the experience is excellent is when the user wants to go to Normal mode but they're in Insert mode and there's an inline-completion/NES widget visible - one has to press escape twice - first to dismiss NES and then to go to Normal mode. During self-hosting, I used this keybindings that sequentially does both, and think it's sort of nice. There's a concern though that if user's in Insert mode and they just want to dismiss inline-completion/NES, they wouldn't be able to.

  {
    "key": "escape",
    "command": "runCommands",
    "when": "vim.mode == 'Insert' && inlineEditIsVisible",
    "args": {
      "commands": ["editor.action.inlineSuggest.hide", "extension.vim_escape"]
    }
  },

Copy link
Member

@rebornix rebornix left a comment

Choose a reason for hiding this comment

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

Thanks @ulugbekna , we haven't updated the when clause since the introduction of this keybinding. It makes perfect sense to me not to hijack the keybinding when we have visible widgets that enhance editing experience in monaco, e.g., inline suggest, inline edit. The only one I'm not sure about is findWidgetVisible, I can imagine a scenario users use find widget to navigate matches (users can control if they user builtin vim kb for search or use find widget) and switch between edit and command mode at the same time, maybe we can remove this when clause?

@rebornix
Copy link
Member

rebornix commented Apr 4, 2025

@J-Fields may I ask what's the current process for releasing updates for the extension?

@ulugbekna
Copy link
Contributor Author

ulugbekna commented Apr 4, 2025

Removed the !findWidgetVisible clause, but I personally find it not very convenient that find widget isn't dismissed when I'm in Normal mode and find widget is visible but not focused -- one has to close it using a mouse.

@djsavvy
Copy link

djsavvy commented Apr 8, 2025

I'd also recommend to get rid of the suggestWidgetVisible clause — autocomplete suggestions come up quite frequently, and I'm finding myself having to hit escape twice almost every time.

Aside from that though this is awesome! Great to be able to use the inline suggestions in normal mode.

@djsavvy
Copy link

djsavvy commented Apr 8, 2025

Actually even without the suggestWidgetVisible clause I'm still running into esc presses that do not take me to normal mode, because an inline suggestion loads in the split second before I press the escape key. I think in order to properly handle this case we actually just want to switch to normal mode on escape and not dismiss any of the completion popups.

I'm not sure what windsurf does, but I recently switched back to using vscode primarily after using windsurf, and their UX around tab/escape in vi mode seems to have it nailed down.

@rebornix
Copy link
Member

@J-Fields I have reviewed and tested this change, considering the impact it has I'm going to merge this change in. At the same time, it seems there are quite a few good PRs/fixes, I'm considering reviewing them and getting some in then releasing a new version. Let me know if this sounds reasonable to you.

@rebornix rebornix merged commit bba5de0 into VSCodeVim:master Apr 21, 2025
2 checks passed
@rossipedia
Copy link

JFC this was incredibly aggravating to figure out. This change conflicts with 20 years of vim muscle memory and completely bogged down my morning, wasting hours of time wrestling with VS Code trying to figure out why I'd still be in Edit mode after hitting escape.

I suspect you're going to have a lot of upset developers over this.

@max-sixty
Copy link
Contributor

max-sixty commented May 15, 2025

I really appreciate the effort to work well with other extensions and I would love to see this extension get the continued investment. so I'm very hesitant to bash any small changes; I fear we'll just get more ossification of the extension.

but this change does seem pretty bad, unfortunately

the result of a set of keystrokes needs to be predictable. and because an auto completion showing isn't predictable, a series of keystrokes containing esc is now unpredictable

otoh, @rebornix maybe you tried it out and got used to it?


this seems to disable the change:

  // https://github.com/VSCodeVim/Vim/pull/9560/files
  {
    "key": "Escape",
    "command": "-extension.vim_escape",
    "when": "editorTextFocus && vim.active && !inDebugRepl && !inlineSuggestionVisible && !inlineEditIsVisible && !testing.isPeekVisible && !testing.isInPeek && !suggestWidgetVisible && !dirtyDiffVisible && (vim.mode == 'Insert' || !notebookEditorFocused)"
  },
  {
    "key": "Escape",
    "command": "extension.vim_escape",
    "when": "editorTextFocus && vim.active && !inDebugRepl"
  },

how about an alternative, like shift+esc, to exit the autocompletions? I guess that would need to be something on the VS Code side, or some suggestions for users to add in their own config?

@ryanto
Copy link

ryanto commented May 15, 2025

I really appreciate the work and time that's gone into getting vim mode, vscode, and copilot all working together. Thanks!

Just wanted to let you know that this change was a bit surprising.

Today while in insert mode, whenever I hit escape nothing happened. It took me a while to figure out it's because there was an inline-completion popping in at the last second. I thought I completely broke my vscode.

@bjude
Copy link

bjude commented May 16, 2025

Another vote here for rethinking this. It should be reverted ASAP until a better path forward is found. Changing the behaviour to require 2 <esc> presses for the overwhelmingly common case of going from insert -> normal mode while an autocomplete suggestion is visible feels horrible to use

@devm33
Copy link

devm33 commented May 16, 2025

Here's an alternate approach, curious to get folks thoughts on it.

In your VS Code settings (settings.json) add:

   "vim.normalModeKeyBindings": [
        {
            "before": [
                "<tab>"
            ],
            "commands": [
                "acceptSelectedSuggestion",
                "editor.action.inlineSuggest.commit"
            ]
        },
        {
            "before": [
                "<esc>"
            ],
            "commands": [
                "hideSuggestWidget",
                "editor.action.inlineSuggest.hide"
            ],
            "after": [
                "<esc>"
            ],
        }
    ],
    "vim.insertModeKeyBindings": [
        {
            "before": [
                "<esc>"
            ],
            "commands": [
                "hideSuggestWidget",
                "editor.action.inlineSuggest.hide"
            ],
            "after": [
                "<esc>"
            ],
        }
    ],

And then undo the change here by rolling back to 1.29.0 or adding the following to your keybindings.json:

  {
    "key": "tab",
    "command": "-extension.vim_tab",
  },
  {
    "key": "Escape",
    "command": "extension.vim_escape",
    "when": "editorTextFocus && vim.active && !inDebugRepl"
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants