#7295 Theia API is incompatible with VSCode

[JonasHelming]

fixed theia.commands.registerCommand to be compatible with VSCode

Signed-off-by: Jonas Helming jhelming@eclipsesource.com

What it does

Fixes issue #7295
Changes the theia plugin API to accept CommandDescription OR string at commands.registerCommand.
Moves special handling of this function from initializer to plugin context.

How to test

Link the adapted packages locally.
instanciate a theia hello world plugin (yo @theia/plugin)

Use the following example code in the plugin implementation:

import * as theia from '@theia/plugin';
import * as vscode from 'vscode';

export function start(context: theia.PluginContext) {
    const informationMessageTestCommand = {
        id: 'hello-world-example-generated2',
        label: "Hello World2"
    };
    context.subscriptions.push(theia.commands.registerCommand('hello-world-example-generated1', (...args: any[]) => {
        theia.window.showInformationMessage('Hello World1!');
    }));

    context.subscriptions.push(theia.commands.registerCommand(informationMessageTestCommand, (...args: any[]) => {
        theia.window.showInformationMessage('Hello World2!');
    }));

    context.subscriptions.push(vscode.commands.registerCommand('hello-world-example-generated3', (...args: any[]) => {
        theia.window.showInformationMessage('Hello World3!');
    }));

}

Register the two commands without a label in the package.json like so:

"contributes": {
		"commands": [{
			"command": "hello-world-example-generated1",
			"title": "Hello World1"
		},
        {
			"command": "hello-world-example-generated3",
			"title": "Hello World3"
		}]

Test that all three hello world commands work when launching the plugin.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[marcdumais-work]

@vince-fugnitto would you do another review pass?

[marcdumais-work]

@JonasHelming, please check the commit message - I think there was a mishap with it at some point.

[vince-fugnitto]

@JonasHelming, please check the commit message - I think there was a mishap with it at some point.

@JonasHelming in addition to Marc's comment, do you perhaps have an already created plugin for which to test the changes without reviewers having to each create the plugin themselves?

[JonasHelming]

@JonasHelming, please check the commit message - I think there was a mishap with it at some point.

@JonasHelming in addition to Marc's comment, do you perhaps have an already created plugin for which to test the changes without reviewers having to each create the plugin themselves?

Obviously I do, here you go :-)

testplugin.zip

[JonasHelming]

@JonasHelming, please check the commit message - I think there was a mishap with it at some point.

fixed

[vince-fugnitto]

@JonasHelming, please check the commit message - I think there was a mishap with it at some point.

@JonasHelming in addition to Marc's comment, do you perhaps have an already created plugin for which to test the changes without reviewers having to each create the plugin themselves?

Obviously I do, here you go :-)

testplugin.zip

I'm getting the following build error when attempting to build the plugin (ex: result of npm i):

src/newplugin-backend.ts:14:63 - error TS2345: Argument of type '"hello-world-example-generated1"' is not assignable to parameter of type 'CommandDescription'.

14     context.subscriptions.push(theia.commands.registerCommand('hello-world-example-generated1', (...args: any[]) => {
                                                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[JonasHelming]

@JonasHelming, please check the commit message - I think there was a mishap with it at some point.

@JonasHelming in addition to Marc's comment, do you perhaps have an already created plugin for which to test the changes without reviewers having to each create the plugin themselves?

Obviously I do, here you go :-)
testplugin.zip

I'm getting the following build error when attempting to build the plugin (ex: result of npm i):

src/newplugin-backend.ts:14:63 - error TS2345: Argument of type '"hello-world-example-generated1"' is not assignable to parameter of type 'CommandDescription'.

14     context.subscriptions.push(theia.commands.registerCommand('hello-world-example-generated1', (...args: any[]) => {
                                                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Hmm, that is weird. I can have a look again, but just to be sure: Did you link the fixed version of this package? If you look at packages/plugin/src/theia.d.ts, the commands.registerCommand function should accept both, a String or a CommandDescription

[vince-fugnitto]

@JonasHelming I haven't linked, did you mean run the command npm link and reference the package that way (local dependency)?

[JonasHelming]

@JonasHelming I haven't linked, did you mean run the command npm link and reference the package that way (local dependency)?

Yes, you have to make sure that the build of the test plugin uses the adapted package containing the fix. Otherwise it is expected that the build fails (that is kind of the reported issue)

[vince-fugnitto]

@JonasHelming I haven't linked, did you mean run the command npm link and reference the package that way (local dependency)?

Yes, you have to make sure that the build of the test plugin uses the adapted package containing the fix. Otherwise it is expected that the build fails (that is kind of the reported issue)

Okay no problem! I'll update the pull-request 'how to test' to be a bit clearer.

[vince-fugnitto]

I confirmed that I am able to run all three hello-world commands from the plugin 👍
I'll give others a chance to try out the pull-request as well.

[JonasHelming]

@benoitf and @azatsarynnyy Do you mind to have a look? If you do not want this change, please feel free to let me know.

[marcdumais-work]

@vince-fugnitto I think enough time has elapsed for all interested to have a look and comment? Time to merge?