Register show references command under the vscode known id.

[tsmaeder]

Registers the show references command from monaco editor both under the old theia id and the vs code "standard" id.
It's a fix for #4084

This is untested code to illustrate what I have in mind.

[akosyakov]

There is already editor.action.showReferences command registered by Monaco, that cannot be replaced, since UI features like Find Reference depends on it.

Monaco command expects 1-based position/locations, while VS Code expects 0-based. I would expect that we intercept editor.action.showReferences command execution from plugins, delegate to editor.action.showReferences Monaco command, and translate arguments/results

textEditor.commands.showReferences is used by existing language contributions (like Java) to convert LSP show references requests to call editor.action.showReferences Monaco command. It cannot be removed.

Update:

No, actually i'm wrong, i've just checked Monaco register editor.action.referenceSearch.trigger for Find All References UI feature, not editor.action.showReferences. So this change should work.

[akosyakov]

It leads to the infinity loop:

To test I've registered the following command in MonacoEditorCommandHandlers:

this.registry.registerCommand({
    id: 'lalala',
    label: 'Lalala'
}, {
    execute: editor => this.registry['commands'].executeCommand(EditorCommands.SHOW_REFERENCES.id, editor.uri.toString(), editor.cursor, [])
});

An implementation of editor.action.showReferences should by pass our command registry and invoke internal Monaco command.

I wonder whether it is generally sustainable approach. In this case Monaco does not contribute such command to our registry, but it does contribute other 150 commands started with editor.action prefix. @svenefftinge @benoitf

[akosyakov]

Do we need constants for deprecated commands? There cannot be any clients for it.

[tsmaeder]

It leads to the infinity loop:

Yes, it does; I was not expecting the monaco command registry to delegate back up to the theia command registry. But the same would apply when we register the command in plugin-vscod-commands-contribution.ts, right?

[akosyakov]

I was not expecting the monaco command registry to delegate back up to the theia command registry.

It is done to support Monaco UI features from the command palette and menu items, for instance: find references, go to definition, redo, undo, copy, paste, and etc.

But the same would apply when we register the command in plugin-vscod-commands-contribution.ts, right?

I am suggesting not to register but intercept calls from plugins and delegate to core or Monaco commands.

[akosyakov]

Generally, i have following doubts with just renaming constants:

• it assumes that arguments/return types of core/Monaco commands are compatible to VS Code types, which is not true;
• it assumes that we can register editor.action or workbench.action commands, which is not true, since these prefixes are reserved for UI features contributed by Monaco;
• it requires not only changing command constants, that is a breaking change itself, but their implementations.

I would rather have one place where VS Code types/commands are converted to LSP/Monaco, something like:

async $executeCommand<T>(id: string, ...args: any[]): Promise<T | undefined> {
        if (id === 'editor.action.showReferences') {
            // HERE convert args from VS Code to LSP/Monaco types
            const result = await this.delegate.executeCommand(EditorCommands.SHOW_REFERENCES, ...args);
            // HERE convert result from LSP/Monaco to VS Code types
            return result;
        }
        return this.delegate.executeCommand(id, ...args);
    }

It would be a minimal backward compatible change, not requiring changes of command constants, signatures or implementations.

[tsmaeder]

@akosyakov but command ids are used in many places, for example code actions, code lenses, etc, etc. We would have to filter all those locations and adjust parameters (positions, in particular) to fit the monaco commands.

[akosyakov]

It looks like we have to. We cannot use vscode types outside of the plugin extension. For example textEditor.commands.showReferences accepts LSP location with URI as a string, but in vscode types location has URI of a URI type.

I wonder what VS Code does, they should have exactly the same issue.

[akosyakov]

They apply type converters for exposed commands and language features.

Generically: microsoft/vscode#67649 (comment)
For vscode. commands: https://github.com/Microsoft/vscode/blob/f41ddf2bb65d552d7e7e54f66101afe2edbfb963/src/vs/workbench/api/node/extHostApiCommands.ts#L294

[tsmaeder]

Btw, I found out why the "editor.action.showReferences" command is not found: the plugin engine delegates the execution of commands to the theia command registry. However, this command is registered in the monaco command registry, so it's not found.

[akosyakov]

Btw, I found out why the "editor.action.showReferences" command is not found: the plugin engine delegates the execution of commands to the theia command registry. However, this command is registered in the monaco command registry, so it's not found.

Could you try to register it here https://github.com/theia-ide/theia/blob/f54fc5c67232592e71bd65d5514fdbab8e90c51f/packages/monaco/src/browser/monaco-command.ts#L60
with

{ id: 'editor.action.showReferences', delegate: 'editor.action.showReferences' }

If it does not work then you can call it directly on the editor:

const editor = MonacoEditor.getCurrent(editorManager);
if (editor) {
    editor.commandService.executeCommand('editor.action.showReferences', ...args);
}

[akosyakov]

I was able to register Monaco commands as is via by passing our command registry on the command invocation here: f482171#diff-9ddba23d7cd957a1a88b40636645c22bR365

I need it to fix VS Code api tests, they use commands like undo, redo. These commands luckily don't need type conversions.

I can extract a commit in a separate PR if you need it. For editor.action.showReferences the plugin system still should convert in all appropriate places from VS Code to Monaco types and backward.

[tsmaeder]

I can extract a commit in a separate PR if you need it. For editor.action.showReferences the plugin system still should convert in all appropriate places from VS Code to Monaco types and backward.

Are these changes part of a PR?

[akosyakov]

@tsmaeder can be closed?