Fix `Esc` going to jlab-command mode

[ianhi]

Fixes: https://github.com/jupyterlab-contrib/jupyterlab-vim/issues/31

Many thanks to @fcollonval for suggesting the fix here: https://gitter.im/jupyterlab/jupyterlab?at=6120c9f2aa48d1340c2490d2

and to @kmdalton for reminding me to have a go at fixing this.

[github-actions[bot]]

:point_left: Launch a binder notebook on branch ianhi/jupyterlab-vim/esc

[ianhi]

In light of it being friday evening I'm going to not merge or release this now. But will on monday unless anyone (in particular @fcollonval or @axelfahy) see something is wrong here

[dirkroorda]

If all plugins adopt this approach (listening to the PluginChanged events and if another plugin has changed a particular shortcut, to change it back to what it was before), you can get a never ending race between plugins to define shortcuts.

[fcollonval]

If all plugins adopt this approach (listening to the PluginChanged events and if another plugin has changed a particular shortcut, to change it back to what it was before), you can get a never ending race between plugins to define shortcuts.

Indeed

@ianhi did you try to set the workaround in this extension settings schema. This should disable the default shortcut that brings problem without the drawback mentioned by @dirkroorda

This means adding in schema/plugin.json:

    "jupyter.lab.shortcuts": {
      "command": "notebook:enter-command-mode",
      "keys": ["Escape"],
      "selector": ".jp-Notebook.jp-mod-editMode"
      "disabled": true
    },

This likely works only for JLab >= 3.1 (thanks to https://github.com/jupyterlab/jupyterlab/pull/9858).

[ianhi]

If all plugins adopt this approach (listening to the PluginChanged events and if another plugin has changed a particular shortcut, to change it back to what it was before), you can get a never ending race between plugins to define shortcuts.

I think that this won't happen as a addCommand does not seem to trigger another plugin changed signal. That said the shortcut override is clearly the right way to approach this - I'll look into that thanks!

Also looking at this again with the excitment of having that "fixed" waned I'm also realizing we need to build in a way to bring it back, so that vim mode can be toggleble.

[fcollonval]

Also looking at this again with the excitment of having that "fixed" waned I'm also realizing we need to build in a way to bring it back, so that vim mode can be toggleble.

Then I would implement the settings workaround in the code (automatically updating the user shortcut setting) so you can opt-in/opt-out at will

[ianhi]

Then I would implement the settings workaround in the code (automatically updating the user shortcut setting) so you can opt-in/opt-out at will

I don't think I understand what you mean here. Do you mean that it's possible to set the default shortcut from the plugin if I require the the IShortcutSetting (or whatever the correct name is) extension? In the latest push I just add the esc command back via app.addKeyBinding

[fcollonval]

c61b586 will work. The only drawback is when the user enable the vim mode it needs to refresh the page.

What I have in mind is something like this:

  function updateSettings(settings: ISettingRegistry.ISettings): void {
    // TODO: This does not reset any cells that have been used with VIM
    enabled = settings.get('enabled').composite === true;
    if (enabled){
      if(!hasEverBeenEnabled) {
        hasEverBeenEnabled = true;
        setupPlugin(app, tracker, jlabCodeMirror);
      }
      // Disable the command mode shortcut
      settingRegistry.get('@jupyterlab/shortcuts-extension:shortcuts', 'shortcuts').then(shortcuts => {
        // Set the edit mode shortcut has disabled
        settingRegistry.set(
        '@jupyterlab/shortcuts-extension:shortcuts', 
        'shortcuts', 
        (shortcuts.user ?? [{command: 'notebook:enter-command-mode', keys: ['Escape'], selector: '.jp-Notebook.jp-mod-editMode'}])
          .map(
            shortcut => shortcut.command === 'notebook:enter-command-mode' && shorcut.keys[0] === 'Escape' ? {...shortcut; disabled: true} : shortcut);
      });
    } else {
      settingRegistry.get('@jupyterlab/shortcuts-extension:shortcuts', 'shortcuts').then(shortcuts => {
        // Remove the command mode shortcut
        settingRegistry.set('@jupyterlab/shortcuts-extension:shortcuts', 'shortcuts', (shortcuts.user ?? []).filter(shortcut => shortcut.command === 'notebook:enter-command-mode' && shorcut.keys[0] === 'Escape');
      });
    }
  }

[ianhi]

The only drawback is when the user enable the vim mode it needs to refresh the page.

I think I can fix that by keeping track of that command then removing it when re-enabled.

What I have in mind is something like this:

That's pretty slick! Unfortunately when I try it out I am finding that it floods the code console with these warnings:

I suppose that's not a deal breaker but it's definitely not great.

[ianhi]

Ok with the latest commit I think I have solved the need to refresh when re-enabling the extension. Thoughts everyone? I'm not wedded to this and am happy to go with the settings route if we can stop the flooding of the console.

[ianhi]

Doesn't seem that anyone has an objection to the current state. So I'm going to merge so i can rebase https://github.com/jupyterlab-contrib/jupyterlab-vim/pull/58 and then I'll cut a release in the next day or so