Undo/Redo for a text based customEditor isn't working

[danarad05]

Bug Description:

When opening a text based custom editor document and making changes - undo/redo is caught but doesn't work

Steps to Reproduce:

1. Run debug hosted plugin for custom-editor-sample from vscode-extension-samples
2. open cscratch document and make changes to it
3. run ctrl+z or undo/redo from Edit menu
4. cscratch document isn't updated

Referring to this document for instance:

Additional Information

• Operating System: windows
• Theia Version: 1.12.0

[Hanksha]

Hi,

I think I'm having a similar issue although I'm not sure if it fits in this issue. We have custom editors (no text editor) and the undo/redo shortcuts don't trigger the right command handler for the active custom editor. It works if I add the undo/redo commands to the toolbar or trigger them by any other means than the shortcuts.

I looked into it a bit and found that when using the keybinding, the monaco command handler for undo is always enabled, even though there is no focused monaco editor, so our custom handler never get called. See https://github.com/eclipse-theia/theia/blob/master/packages/monaco/src/browser/monaco-command.ts#L170

I'd like to provide a fix for this but I'm not sure if these monaco command handlers should only be enabled for focused monaco editors. If we use active editors as well (referring to monaco-command.ts#L170) if I activate my custom editor and there's a text editor opened (but hidden), it's still using that monaco command handler even though the text editor widget is technically not active.

[Hanksha]

Hi @vince-fugnitto Do you have any feedback on that?

[vince-fugnitto]

Hi @vince-fugnitto Do you have any feedback on that?

@Hanksha I haven't looked into the details but I believe the problematic check is codeEditorService.getActiveCodeEditor() which will ultimately return the focused editor if it exists, or the last focused editor. You can experiment a bit and see if it works for you without this check, but it probably makes sense not to look back in history regarding the last focused editor and only check the current one.

[Hanksha]

@vince-fugnitto Alright thanks, I'll look into it.

[Hanksha]

@vince-fugnitto I think it was done this way so it can imitate the behavior of VS Code where you can undo/redo on the last active text editor without being focus on it, from example from the explorer view.

I think what we want here is to use the monaco command handler as a default if nothing else could handle the command but I don't know if it's possible.

[vince-fugnitto]

@Hanksha I don't think we have prioritized handlers for keybindings, but you may be able to use a when-clause context (when a custom-editor is active) to register handlers for undo/redo which should take priority over monaco,

For reference I believe vscode does the following https://github.com/microsoft/vscode/blob/726b7f51e0063b065436be42aaee4853447143d8/src/vs/workbench/contrib/customEditor/browser/customEditors.ts#L84-L93.

[Hanksha]

@vince-fugnitto Is that CustomEditorService available for theia extensions? I'm not using the vscode plugin API at all.

[vince-fugnitto]

@vince-fugnitto Is that CustomEditorService available for extensions? I'm not using the vscode API at all.

@Hanksha I'm just pointing out what vscode does, if you aren't using the plugin-api then what are you using? Sorry I'm confused since this issue is about CustomEditor VS Code APIs.

[Hanksha]

@vince-fugnitto I'm writing a theia extension and using widgets as editors which are not text based (not vscode editors) and it has the same issue as using CustomEditor VS Code API. That why I mentioned in my original comment I was not sure if it would fit this issue or not.

[vince-fugnitto]

I'm not sure how your non-text based editor looks like or is implemented so I can't really comment on what might be the issue for undo/redo, or how undo/redo should work in such a case. Our EditorWidget relies on TextEditor being the underlying resource, if you have an example that is not proprietary perhaps you can share it, else I'm not sure how much help I can provide.

[Hanksha]

Unfortunately I don't have an example that is not proprietary but here's what I use:

• I have a widget, let's call it "MyCustomWidget" that extends BaseWidget from @theia/core/lib/browser, that widget has some UI in it.
• I register it in my frontend module via

bind(WidgetFactory).toDynamicValue(ctx => ({
      id: "myid",
      createWidget: (options: MyCustomWidgetOptions) => {
          const child = ctx.container.createChild();
          child.bind(MyCustomWidgetOptions).toConstantValue(options);
          return child.get(MyCustomWidget);
      }
}));

• then I register a WidgetOpenHandler so I can open it via the OpenerService

@injectable()
export class MyCustomWidgetOpenHandler extends WidgetOpenHandler<MyCustomWidget> {
    id = "myid";
    label = "My Custom Widget";

    canHandle(uri: URI): MaybePromise<number> {
        if (some condition) return 1;

        return 0;
    }

    protected createWidgetOptions(uri: URI): object {
        return {
            uri: uri.toString()
        };
    }
}

• From there I can open the custom widget like an editor, in the main shell area, from one of my views etc.

Once that widget is opened, the undo/redo keybindings don't work either because the handler registered by the monaco-command is always enabled.

[Hanksha]

@vince-fugnitto I also found out that there are two commands registered for the undo keybinding:

• "core.undo" from "common-frontend-contribution.ts" with keybinding "ctrl+z"
• "undo" from "monaco-keybindings.ts" with keybinding "cmdctrl+z"

For both the "when" is undefined so from what I saw the keybinding for "undo" gets triggered but not "core.undo".

[vince-fugnitto]

@Hanksha I'm not sure what behaviour you want when you execute undo but can't you simply register a command + keybinding for your given widget when it is visible and call the core.undo command if you don't want other keybindings to take priority?

[Hanksha]

@vince-fugnitto Wouldn't that mean that if the user want then to edit the keybinding for the undo/redo he'd have to do it for all the registered commands?
See

[vince-fugnitto]

@Hanksha do you mind opening a discussion instead, I think it's adding a lot of noise to this issue which is strictly about a VS Code CustomEditor API bug.

[Hanksha]

@vince-fugnitto Sure.

[Hanksha]

@vince-fugnitto Done #12474