Plugins: Two problems related to command parameters

Hi folks, when working on https://github.com/theia-ide/theia/issues/4084, I have come across two issues related to the execution of arbitrary commands. Commands, for example, are passed from plugins to theia when executing code actions.

In Theia, there are two command registries: the first is the “global” one, where you can register commands that are not editor related. The second one is the command registry of the embeded monaco editor. This command registry delegates to the global registry when executing commands that are not related to the current editor.

The first problem we’re facing, then is that the plugin API only delegates to the global registry. Editor-related commands like “editor.action.showReferences” are therefore not found. My proposed fix for this would be to delegate command execution to the current editor. If no editor is open, the command is passed to the global registry for execution.

The second problem relates to the parameters array that can be passed with the commands: For example, the “showReferences” command has a vscode.Position as a parameter. However, we are implementing Position with “_line” and “_character” private variables and property accessor methods. But class information does not survive the travel through the plugin API, so when we later interpret the passed parameter as a position, it will not be the class type “Position”, but just an object with none of the fields “line” nor “character” set.

VSCode works around this problem by converting a well known set of classes into pure javascript objects: Position and Range among them. They also convert from 0-based to 1-based positions (monaco convention vs. API convention). I feel this is cheating a bit, since it only works for well known classes.

The alternative to always converting those classes would be to only do it for certain, well known commands (see https://code.visualstudio.com/api/references/commands). I find that more “correct”, but I’d like to to hear opinions on this.

Thoughts?

[original thread by Thomas Mäder]

about 1st point: Could we delegate to global registry if not found in editor registry ?

about second point, there are some code allowing to pass objects: https://github.com/theia-ide/theia/blob/4572886d362f65c2e1857b37945a325d3bf09d8f/packages/plugin-ext/src/api/rpc-protocol.ts#L316 would that help (like adding Position and Range for example ? )

about 1st point: Could we delegate to global registry if not found in editor registry ?

We’re already doing that.

About the second one: I wasn’t aware that this code exists, but the question I’m raising is no a technical one: of course we can encode the classes end revive them. The problem is that it only works for a couple of classes (the ones we’re mangling in the code you showed).

Also, IMO the ObjectsTransferrer namespace has the layering wrong: it should live in a api-namespace-aware layer above the rpc layer.

I think it’s OK to only handle some classes based on Plugin’s API contract. When it was added on theia I asked if we could have something pluggable as well so some external code could plug additional converters.
You may discuss with Mykola who authored the code.

Plugin’s API contract

Do you mean “Plugin API’s contract”? I think it would be ok to say: "you can use classes from the the (vscode|theia) API, but no others, but then we’d have to actually convert all the classes.

Also, do we do “-1” on the positions? (https://spectrum.chat/theia/dev/base-of-position-index-in-main-ext-interface~b89018b0-d792-4a77-9e4b-2b243a2f8a85)

I would assume yes about converting classes that are handled by .d.ts.
Theia is using 0-based. Only when it deals with monaco it needs to update ranges