Plugin management #162

vitaliy-guliy · 2019-04-08T07:49:23Z

What does this PR do?

Displays a dedicated view with list of available plugins
Installs / uninstalls a plugin by simple mouse click
Displays installed plugins in separate list
Allows the user to install a plugin from custom registry
Installs VS Code extension using a command or drag and drop

Video demonstrating the feature
https://www.youtube.com/watch?v=2_GPZb1_Hew

What issues does this PR fix or reference?

eclipse-che/che#12932

Signed-off-by: Vitaliy Gulyy vgulyy@redhat.com

AndrienkoAleksandr · 2019-04-08T08:14:50Z

extensions/eclipse-che-theia-plugin-ext/package.json

@@ -42,4 +45,4 @@
            "backend": "lib/node/che-backend-module"
        }
    ]
-}
+}


Fix line ending please.

AndrienkoAleksandr · 2019-04-08T08:47:01Z

extensions/eclipse-che-theia-plugin-ext/src/browser/che-frontend-module.ts

+
+    bindViewContribution(bind, ChePluginViewContribution);
+
+    bind(ChePluginWidget).toSelf();


Maybe it should be singleton?

Fixed. Thanks.

AndrienkoAleksandr · 2019-04-08T09:03:29Z

extensions/eclipse-che-theia-plugin-ext/src/node/che-plugin-service.ts

+// const { yaml } = require('js-yaml');
+const yaml = require('js-yaml');
+
+export interface ChePluginMetadataInternal {


Maybe ChePluginMetadataInternal extends ChePluginMetadata?

There is another ChePluginMetadata defined in common/che-protocol

AndrienkoAleksandr · 2019-04-08T09:12:18Z

extensions/eclipse-che-theia-plugin-ext/src/node/che-plugin-service.ts

+            return getPluginsRequest.data;
+        }
+
+        return [];


Maybe throw an error instead of empty array?

Has been reworked

AndrienkoAleksandr · 2019-04-08T09:27:11Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-widget.tsx

+    }
+
+    private installedPlugins: string[] = [
+        'org.eclipse.che.editor.theia:1.0.0',


I think we should get this information from workspace config attributes.

Yep, it's only draft for now

olexii4 · 2019-04-08T13:36:35Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-widget.tsx

+        this.title.closable = true;
+        this.addClass('theia-plugins');
+
+        this.node.tabIndex = -1;


Why do we use '-1' for the tabIndex(not 0)?

Fixed. Thanks!

slemeur · 2019-04-09T10:01:12Z

Very cool @vitaliy-guliy !

Can we also take the opportunity to improve a little bit the formatting of each "plugin row" ?
Few feedbacks:

Use a single font size, for title and description (the one from description). Difference will be applied with the bold style.
Use the same font size in the install/installed buttons that for the author of the plugin
Reduce the space between title and description, description and author
Left alignement for all fields
Display "Install" and "Installed" buttons always at the same position. I think where you have the "Install" button works better.

vitaliy-guliy · 2019-04-09T13:09:26Z

@slemeur Yes, sure. I will add a screenshot here with updates

slemeur · 2019-04-10T06:41:31Z

Thanks @vitaliy-guliy

vitaliy-guliy · 2019-04-12T07:41:19Z

@slemeur WDYT?

The space between title and description is reduced a bit. But I reserved space for description to be able to show 3 lines of text.
I'm thinking about displaying plugin type somewhere as a label. Where do you prefer to show it?
I have two options: above or under the description.

benoitf · 2019-05-06T01:23:24Z

hello, what os the status of this PR ?

benoitf · 2019-05-06T01:23:40Z

ci-build is failing and ECA check as well

monaka · 2019-05-06T05:18:10Z

@benoitf Looks same as I reported in my comment.
CI build server may be broken.

monaka · 2019-05-07T07:42:06Z

FYI

I tried to build this PR branch on my CI server.
It looks a version mismatch happened.

https://dev.azure.com/pizzafactory/camino/_build/results?buildId=670

Step 17/40 : RUN yarn
 ---> Running in f87813b46e5d
yarn install v1.12.3
[1/5] Validating package.json...
error parent@0.0.0: The engine &quot;node&quot; is incompatible with this module. Expected version &quot;&gt;=10.2.0&quot;. Got &quot;8.15.0&quot;‌
error Found incompatible module‌
info Visit https:&#x2F;&#x2F;yarnpkg.com&#x2F;en&#x2F;docs&#x2F;cli&#x2F;install for documentation about this command.‌
The command '/bin/sh -c yarn' returned a non-zero code: 1

monaka · 2019-05-09T06:10:09Z

FYI

Looks ECA rejection is caused commit-id 7e6cf42 250bcc2.

monaka · 2019-05-09T07:25:37Z

FYI

Build failures I repoted above will be fixed after rebasing or merging from the master branch. (I checked by hand)

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

vitaliy-guliy · 2019-05-29T13:33:16Z

Too much things has been reworked. PR is ready to review.

AndrienkoAleksandr · 2019-05-29T14:03:37Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-command-contribution.ts

@@ -0,0 +1,177 @@
+/********************************************************************************
+ * Copyright (C) 2018 Red Hat, Inc. and others.


Fix year, please, for all files...

Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com>

AndrienkoAleksandr · 2019-05-30T09:00:20Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-frontend-service.ts

+    // @builtin
+    // @enabled
+    // @disabled
+    hasType(filter: string, type: string) {


Minor: apply, please, return type :boolean. But up to you.

Done. Thanks

AndrienkoAleksandr · 2019-05-30T09:28:04Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-manager.ts

+
+                const registry = this.registryList.find(r => r.uri === uri);
+                if (registry === undefined) {
+                    instance.registryList.push({


In the two lines upper you used this.registryList:
const registry = this.registryList.find(r => r.uri === uri);
Could you do the same this.registryList.push({ and remove const instance = this;?

AndrienkoAleksandr · 2019-05-30T09:41:17Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-manager.ts

+    private getIdPublisher(input: string): string {
+        if (input.startsWith('ext install ')) {
+            // check for 'ext install rebornix.Ruby'
+            return input.substring('ext install '.length);


It would be nice to trim result.

This is intentional decision. Space is a part of a prefix of the command.
The first command has a space at the end, the second command has a slash.

if (input.startsWith('ext install ')) { // check for 'ext install rebornix.Ruby' return input.substring('ext install '.length); } else if (input.startsWith('vscode:extension/')) { // check for 'vscode:extension/rebornix.Ruby' return input.substring('vscode:extension/'.length); }

mmorhun

If you need to merge it ASAP you may ignore my comments for now and handle them in the following PR

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-command-contribution.ts

mmorhun · 2019-05-30T09:42:43Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-frontend-service.ts

+        return plugins;
+    }
+
+    private determinePluginType(meta: PluginMetadata): string {


Maybe use literals type or enum as return value?

It could be any string

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-frontend-service.ts

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-manager.ts

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-menu.ts

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-widget.tsx

extensions/eclipse-che-theia-plugin-ext/src/node/che-plugin-service.ts

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-frontend-service.ts

akurinnoy · 2019-05-30T11:53:50Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-frontend-service.ts

+        const metadata = await this.hostedPluginServer.getDeployedMetadata();
+
+        const plugins: ChePluginMetadata[] = await Promise.all(
+            metadata.map(async (meta: PluginMetadata) => {


Here is a synchronous function in map method. I seem you don't need promises here.

Has been reworked. Thanks!

akurinnoy · 2019-05-30T11:54:39Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-frontend-service.ts

+                    category: 'string',
+                    latestUpdateDate: 'string',
+
+                    // Plugin KEY. Used to set in workpsace configuration


typo: workpsace -> workspace

Fixed. Thanks!

akurinnoy · 2019-05-30T12:06:31Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-manager.ts

+
+        const prefs = this.chePluginPreferences['chePlugins.repositories'];
+        if (prefs) {
+            const instance = this;


You don't need to save this as instance because an arrow function doesn't have its own this, and this of surrounding scope is used.

Yes, it's already fixed. Thanks!

akurinnoy · 2019-05-30T12:13:24Z

extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-manager.ts

+        try {
+            // add the plugin to workspace configuration
+            await this.chePluginService.addPlugin(plugin.key);
+            await this.delay(1000);


Could you explain why do you need a delay here?

It's already removed. No delay anymore :=))

akurinnoy · 2019-05-30T12:44:34Z

extensions/eclipse-che-theia-plugin-ext/src/node/che-plugin-service.ts

+        }
+
+        try {
+            const workpsaceSettings: WorkspaceSettings = await this.cheApiService.getWorkspaceSettings();


typo: workpsaceSettings -> workspaceSettings

Fixed. Thanks!

vitaliy-guliy · 2019-05-30T13:03:31Z

If you need to merge it ASAP you may ignore my comments for now and handle them in the following PR

I have to fix everything. No options here.

Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com>

* Makes it possible to install a plugin from Theia Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com> * Update year in license Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com> * PR feebbacks Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com> * Solving PR feedbacks Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com>

…ins (#162) * Use eclipse/che-remote-plugin-runner-java11:next image for java based plugins Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com> * Revert changes to use Java8 for sonarlint plugin Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com> * Set up memory limit for apache-camel plugin Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com> * Set up memory limit for sonarlint plugin Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

vitaliy-guliy requested review from benoitf, evidolob, olexii4, AndrienkoAleksandr, mmorhun and akurinnoy April 8, 2019 07:49

AndrienkoAleksandr reviewed Apr 8, 2019

View reviewed changes

olexii4 reviewed Apr 8, 2019

View reviewed changes

vitaliy-guliy closed this Apr 12, 2019

vitaliy-guliy reopened this Apr 12, 2019

ashumilova mentioned this pull request Apr 25, 2019

IDE2-2019-05-21 (Sprint: 166) eclipse-che/che#13212

Closed

25 tasks

evidolob mentioned this pull request May 15, 2019

IDE2-2019-06-12 (Sprint: 167) eclipse-che/che#13335

Closed

30 tasks

vitaliy-guliy changed the title ~~Che Plugins View~~ Plugin management May 28, 2019

Makes it possible to install a plugin from Theia

c02aa4f

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

vitaliy-guliy force-pushed the plugins-view branch from 77e3e08 to c02aa4f Compare May 29, 2019 13:29

vitaliy-guliy marked this pull request as ready for review May 29, 2019 13:30

vitaliy-guliy requested a review from tolusha as a code owner May 29, 2019 13:30

vitaliy-guliy removed the request for review from tolusha May 29, 2019 13:30

AndrienkoAleksandr reviewed May 29, 2019

View reviewed changes

Update year in license

7e459ff

Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com>

AndrienkoAleksandr reviewed May 30, 2019

View reviewed changes

mmorhun approved these changes May 30, 2019

View reviewed changes

AndrienkoAleksandr approved these changes May 30, 2019

View reviewed changes

akurinnoy reviewed May 30, 2019

View reviewed changes

vitaliy-guliy added 2 commits May 30, 2019 15:42

PR feebbacks

119d7e8

Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com>

Solving PR feedbacks

8a547f7

Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com>

akurinnoy approved these changes May 31, 2019

View reviewed changes

vitaliy-guliy merged commit cd45d3d into master May 31, 2019

vitaliy-guliy deleted the plugins-view branch May 31, 2019 13:15


		bindViewContribution(bind, ChePluginViewContribution);

		bind(ChePluginWidget).toSelf();

		@@ -0,0 +1,177 @@
		/********************************************************************************
		* Copyright (C) 2018 Red Hat, Inc. and others.

Plugin management #162

Plugin management #162

Conversation

vitaliy-guliy commented Apr 8, 2019 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slemeur commented Apr 9, 2019

vitaliy-guliy commented Apr 9, 2019

slemeur commented Apr 10, 2019

vitaliy-guliy commented Apr 12, 2019 • edited Loading

benoitf commented May 6, 2019

benoitf commented May 6, 2019

monaka commented May 6, 2019

monaka commented May 7, 2019 • edited Loading

monaka commented May 9, 2019

monaka commented May 9, 2019

vitaliy-guliy commented May 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmorhun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaliy-guliy May 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaliy-guliy commented May 30, 2019

vitaliy-guliy commented Apr 8, 2019 •

edited

Loading

vitaliy-guliy commented Apr 12, 2019 •

edited

Loading

monaka commented May 7, 2019 •

edited

Loading

vitaliy-guliy May 31, 2019 •

edited

Loading