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

Arrow key bindings not working with insertModeKeyBindings #2924

Closed
karlhorky opened this issue Jul 31, 2018 · 9 comments
Closed

Arrow key bindings not working with insertModeKeyBindings #2924

karlhorky opened this issue Jul 31, 2018 · 9 comments

Comments

@karlhorky
Copy link
Contributor

@karlhorky karlhorky commented Jul 31, 2018

Describe the bug

The configuration for navigation with arrow keys across word wrapped lines from #2403 (comment) no longer works.

Bonus: The configuration does work with a separate, unrelated configuration.

To Reproduce
Steps to reproduce the behavior:

  1. Install the plugin
  2. Enable word wrapping ("editor.wordWrap": "on")
  3. Configure using the config in broken.json below
  4. Notice that the up and down keys skip text past the wrap on word wrapped lines (notice that both ctrl+o and <C-o> do not work)
  5. Bonus: Configure using the config in working.json below
  6. Bonus: Notice that the up and down keys navigate to every line, even text on word wrapped lines.

broken.json

{
  "vim.insertModeKeyBindings": [
    {
      "before": [
        "down",
      ],
      "after": [
        "<C-o>",
        "g",
        "j"
      ]
    },
    {
      "before": [
        "up"
      ],
      "after": [
        "ctrl+o",
        "g",
        "k"
      ]
    }
  ]
}

working.json

{
  "vim.insertModeKeyBindings": [
    {
      "before": [
        "j"
      ],
      "after": [
        "<Esc>"
      ]
    },
    {
      "before": [
        "down"
      ],
      "after": [
        "<C-o>",
        "g",
        "j"
      ]
    },
    {
      "before": [
        "up"
      ],
      "after": [
        "ctrl+o",
        "g",
        "k"
      ]
    }
  ]
}

Expected behavior

The configuration should work in both cases (not only with the extra j => Esc configuration key).

Logs

Broken:

Code.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:270 [Extension Host] debug: Extension: activating vscodevim.
/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:270 [Extension Host] debug: Extension: registering event handlers.
/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:270 [Extension Host] debug: ModeHandler: handling key=<ExtensionEnable>.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=i.
2console.ts:136 [Extension Host] debug: ModeHandler: handling key=<up>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
2console.ts:136 [Extension Host] debug: ModeHandler: handling key=<up>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<up>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<up>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
2console.ts:136 [Extension Host] debug: ModeHandler: handling key=<up>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<down>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<down>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 

Working:

Code.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:270 [Extension Host] debug: Extension: activating vscodevim.
/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:270 [Extension Host] debug: Extension: registering event handlers.
/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:270 [Extension Host] debug: ModeHandler: handling key=<ExtensionEnable>.
shell.ts:482 [Violation] 'resize' handler took 196ms
[Violation] Forced reflow while executing JavaScript took 37ms
console.ts:136 [Extension Host] debug: ModeHandler: handling key=i.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<down>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=j. after=<Esc>. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. match found. before=<down>. after=<C-o>,g,j. command=undefined.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<C-o>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=j. after=<Esc>. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: ModeHandler: handling key=g.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=j.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<down>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=j. after=<Esc>. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. match found. before=<down>. after=<C-o>,g,j. command=undefined.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<C-o>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=j. after=<Esc>. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: ModeHandler: handling key=g.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=j.
wordHighlighter.ts:300 [Violation] 'setTimeout' handler took 82ms
mouseHandler.ts:94 [Violation] Handling of 'mousewheel' input event was delayed for 122 ms due to main thread being busy. Consider marking event handler as 'passive' to make the page more responsive.
[Violation] 'setTimeout' handler took 62ms
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<up>.
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=j. after=<Esc>. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<down>. after=<C-o>,g,j. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. before=<up>. after=<C-o>,g,k. 
console.ts:136 [Extension Host] debug: Remapper: insertModeKeyBindings. match found. before=<up>. after=<C-o>,g,k. command=undefined.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=<C-o>.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=g.
console.ts:136 [Extension Host] debug: ModeHandler: handling key=k.

Environment (please complete the following information):

  • Extension Name: vim
  • Extension Version: 0.15.7
  • OS Version: Darwin x64 17.7.0
  • VSCode version: 1.25.1
@karlhorky

This comment has been minimized.

Copy link
Contributor Author

@karlhorky karlhorky commented Jul 31, 2018

Note: the configuration also does not work with <up> and <down>, which cause other issues (such as also triggering when one types <down> in insert mode in short succession). Maybe this actually also deserves another issue...

@karlhorky

This comment has been minimized.

Copy link
Contributor Author

@karlhorky karlhorky commented Jul 31, 2018

cc @jpoon

@xmbhasin

This comment has been minimized.

Copy link
Contributor

@xmbhasin xmbhasin commented Jul 31, 2018

I reworked the remapper so that it more accurately matches keys to possible remaps and disabled recursive remappings (see xmbhasin/Vim@f3e4311 xmbhasin/Vim@a96b606). However, it seems to have revealed a deeper issue with the timing of the current mode change and the processing of remapped commands. With your remappings above for example, the <C-o> brings us out of insert mode but then the g and k keys continue to be processed near infinitely and will print a large string of gks or gjs. Changing the remapping to <Esc> and then gk or gj will cause the editor to move lines up or down multiple times.

This problem does not seem to present when using non-recursive bindings however. I'm not sure then how to proceed: should we remove recursive keybindings for insert mode and other modes if the same problem exists there when switching between modes mid remap, or should we chase this bug further?

Edit: I may have broken some other parts of the remap handling, but for now I still think it highlights a deeper issue.

karlhorky added a commit to karlhorky/dotfiles that referenced this issue Oct 8, 2018
@metasong

This comment has been minimized.

Copy link

@metasong metasong commented Oct 25, 2018

the easy workaround is uing the default up down of vscode's shortcut setting( ctrl+k+s ), it is overrided by vim after install the vim extension:

  {
    "key": "down",
    "command": "cursorDown",
    "when": "textInputFocus"
  },
  {
    "key": "up",
    "command": "cursorUp",
    "when": "textInputFocus"
  },
@karlhorky

This comment has been minimized.

Copy link
Contributor Author

@karlhorky karlhorky commented Jan 11, 2019

Thanks for the tip @metasong. I've given it a shot, but it has a critical problem with it:

It escapes from autosuggest boxes. Here's a gif:

kapture 2019-01-11 at 13 25 56

@karlhorky

This comment has been minimized.

Copy link
Contributor Author

@karlhorky karlhorky commented Jan 22, 2019

@xmbhasin I suppose the status of your work on this issue hasn't changed, right? Your commits reworked the remapper, but revealed another deeper issue? And then you had this open question.

I'm not sure then how to proceed: should we remove recursive keybindings for insert mode and other modes if the same problem exists there when switching between modes mid remap, or should we chase this bug further?

Can a maintainer weigh in here? Maybe @jpoon?

@karlhorky

This comment has been minimized.

Copy link
Contributor Author

@karlhorky karlhorky commented Jan 22, 2019

I think I may have come up with a fixed version of @metasong's keybindings (fixing the problem of escaping the autocomplete / suggest widget), based on the when condition from the extension keybindings.

It looks like this:

  {
    "key": "up",
    "command": "cursorUp",
    "when": "editorTextFocus && vim.active && !inDebugRepl && vim.mode == 'Insert' && !suggestWidgetMultipleSuggestions && !suggestWidgetVisible"
  },
  {
    "key": "down",
    "command": "cursorDown",
    "when": "editorTextFocus && vim.active && !inDebugRepl && vim.mode == 'Insert' && !suggestWidgetMultipleSuggestions && !suggestWidgetVisible"
  }

This doesn't need any changes to settings.

I will start using this and see if there are any other problems (including performance issues) with it.

Edit: Performance is great, much better than the original method.

@jpoon

This comment has been minimized.

Copy link
Member

@jpoon jpoon commented Jan 31, 2019

I fixed a couple of issues with the remapper (#3424, #3418), can you retry on the latest release (v1.0.4)? If it still doesn't work, can you share the debug output?

@karlhorky

This comment has been minimized.

Copy link
Contributor Author

@karlhorky karlhorky commented Mar 25, 2019

@jpoon this is indeed now working! Will close this issue.

For everyone else, the original method of setting vim.insertModeKeyBindings with <C-o>, g,j is very slow:

Kapture 2019-03-25 at 10 37 14

Compare this to the speed of rebinding the keybindings, which is as fast as VS Code normally is:

Kapture 2019-03-25 at 10 38 38

@karlhorky karlhorky closed this Mar 25, 2019
karlhorky added a commit to karlhorky/dotfiles that referenced this issue Mar 25, 2019
Switch to keybindings instead of settings

Ref: VSCodeVim/Vim#3623
Ref: VSCodeVim/Vim#2924 (comment)
jpoon added a commit that referenced this issue Apr 3, 2019
* Document display line moving best practices

To vertically move to wrapped lines when word wrap is on.

Ref: #2403 (comment)
Ref: #2924 (comment)

* Remove extra line

* Disable Prettier formatting on code block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.