Monaco commands with overlapping shortcuts get ignored

[katlyn-edwards]

this.monaco.addCommand(
    monaco.KeyCode.Escape,
    () => {
       alert('hello!')
    }, 'true');

Since escape is also used for leaving input mode in Vim, this alert appears to never be fired.
It'd be cool if monaco-vim let keyboard shortcuts propogate if they were unhandled, whereas currently it seems that propagation is stopped somewhere.

[katlyn-edwards]

monaco-vim/src/cm_adapter.js

Line 331 in 34b32db

e.preventDefault();

I think the issue is here. This prevents default and stops poropagation, even though it wasnt doing anything

[katlyn-edwards]

If you change the command to return whether or not it actually did something, then you could use that information to figure out if you should preventDefault and stopPropagation.

e.g. even though esc is registered doesn't mean I want to stop prop/prevent default.
I only want to do that if esc is registered and esc left insert mode, for example

[katlyn-edwards]

You can see this use case in https://editor.bitwiser.in/ -- if you have autocomplete open and press escape, it both closes autocomplete and boots you out of editing mode.

[katlyn-edwards]

I think Monaco utilizes commnds to handle keybindings. Not sure if it's being used here, but that might be beneficial for chaining events like this.

[brijeshb42]

I think Monaco utilizes commands to handle keybindings. Not sure if it's being used here, but that might be beneficial for chaining events like this.

My initial implementation tried to use this but the problem is that vim mode needs to know about all the key presses, even during insert mode, to function properly. And monaco's addCommand won't be scalable as you would have to register a lot of keys. So I implemented a catchall keybinding that catches all key presses and sends them to codemirror implementation and receives a function in return. If that function is not null or undefined, it means codemirror handled the keypress.

I'll have to re-think this approach for situations like autocomplete dialog and any other similar ones though.

[katlyn-edwards]

I think you can get away with minimal changes without having to swap to commands.

Right now the code I linked does the following:

if (we have a key mapping for this combo) {
   prevent default
   stop propagation
   call the key mapping
}

What it needs to do is:

if (I have a key mapping) {
  actionTaken = call the key mapping
  if (actionTaken) {
     stop propagation
     prevent default
  }
}

The major change is that each command needs to return either true or false depending on whether or not action was taken.

For escape for example, you need to return true if you were in a mode like insert, and you left to get back into normal mode. If you're in normal mode already, escape does nothing so it would return false.