Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open VSX Registry integration #6655

Merged
merged 7 commits into from
Mar 19, 2020
Merged

Open VSX Registry integration #6655

merged 7 commits into from
Mar 19, 2020

Conversation

jbicker
Copy link
Contributor

@jbicker jbicker commented Nov 28, 2019

What it does

Widget where one can search and install VSCode Extensions from the Open VSX Registry.

Screenshot 2020-03-11 at 12 36 59

TODO:

  • keyboard navigation does not work (rewrite lists with trees)
  • installed extensions metadata should not be fetched from the registry, but should use local
  • vscode extension URI resolver should be installed, see Open VSX Registry integration #6655 (comment)
  • extensions should be installed under ${configDir}/extensions by default
  • extensions should be loaded from ${configDir}/extensions by default
  • focus is not properly given on the view activation
  • support built-in extensions
  • update view container layout based on query, preserve layout state for different modes
  • progress reporting in view container for search, install, uninstall
  • theming
  • preserve state properly, i.e. search query should be preserved only, everything else should be computed from it again
  • check TODOs in code

Out of scope:

  • finding a version compatible extension
  • detecting outdated extensions and upgrading them
  • supporting workspace extensions (only built-in and global (user))
  • supporting extension enablement
  • use webview to render remote content like README

How to test

  • Try to install/uninstall extensions. Make sure that they are preserved (or not after uninstall) on restart.
  • Look at the extension editor.
  • Test that the extension editor is opened for the vscode extension URIs.

Review checklist

Reminder for reviewers

@svenefftinge
Copy link
Contributor

The items should have an 'install' button when not installed, and an 'uninstall' button when installed.

@svenefftinge
Copy link
Contributor

The progress during installation should be shown with blue moving line above the search box, just as it does in other similar views. The button should turn to 'installing' during that.

@svenefftinge
Copy link
Contributor

I cannot install any extension. The button spins forever.

@svenefftinge
Copy link
Contributor

The search doesn't seem to work. If I search for 'Docker' I see many unrelated extensions.

@svenefftinge
Copy link
Contributor

Screenshot 2019-11-28 at 15 14 52

@jbicker
Copy link
Contributor Author

jbicker commented Nov 28, 2019

Regarding search: theia-ide/extension-registry#3

@akosyakov akosyakov added the vscode issues related to VSCode compatibility label Nov 29, 2019
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked through the code:

  • for the detail widget open handler should handle widget options properly and uri should be aligned with VS Code. We can test with webviews like Qarkus that clicking on java extension link opens the extension widget.
  • generally I find naming of a package and types confusing, it should talk about registry, but looks like implementation of vscode extensions
  • Also old code was copied without adjustment to our current code standards.

.vscode/launch.json Outdated Show resolved Hide resolved
packages/vscode-extensions/README.md Outdated Show resolved Hide resolved
packages/vscode-extensions/package.json Outdated Show resolved Hide resolved
@jbicker jbicker force-pushed the jb/extension-registry branch 2 times, most recently from 96b5ac3 to 42a7f27 Compare December 2, 2019 15:26
@akosyakov
Copy link
Member

akosyakov commented Dec 12, 2019

@akosyakov
Copy link
Member

Please clean up the history: https://github.com/eclipse-theia/theia/blob/master/doc/pull-requests.md#checklist-commit-history There are many commits which fixes introduced by this PR. If it is hard then squash it.

@akosyakov akosyakov force-pushed the jb/extension-registry branch 6 times, most recently from 09a6474 to 36b5544 Compare March 13, 2020 16:22
- Each plugin can be either of system or user type indicating that it was deployed by an embedding product or an end user.
- System plugins are considered to be built built-in plugins. By default they are collected from `THEIA_DEFAULT_PLUGINS`, `THEIA_PLUGINS` variables or can be provided via `--plugins` cli option.
- User plugins can are managed by an end user. They are stored either under user config plugins or extensions folder depending whether it is Theia plugin or VS Code extension. These plugins can survive the server restart.

Signed-off-by: Anton Kosyakov <[email protected]>
…ode Marketplace

Since it's violates MS TOS which allows only access only for products of the VS Code family. Theia based products has to provide own resolution.

Signed-off-by: Anton Kosyakov <[email protected]>
@akosyakov
Copy link
Member

I've rebased and addressed all comments from @vince-fugnitto and @spoenemann.

The current state:
Screenshot 2020-03-13 at 17 22 50

@marcdumais-work @svenefftinge please have a look

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did another review pass and concluded with the following findings:

  • verified that any new dependencies have approved licenses
  • verified superficially the source code and could not identify any major issues
  • verified that builtin extensions were correctly listed, and could not be removed

Past Issues:

  • previous issues with icons (ex: gulp) has been successfully addressed
  • the min height for the tree-view is successfully addressed

Issues:

  • the search does not work properly (ex: search for 'a' yields not results but 'alt' does)
  • when the progress bar is shown, there is a minor UI glitch where the rest of the view is pushed down
  • an extension's readme should be displayed in the center, similarly to vscode (tested using editorconfig)

Improvements (now or in the future):

  • better display of individual extensions in the tree (an extension's name should be bold like in vscode and less padding with the publisher):

    Screen Shot 2020-03-17 at 2 11 58 PMScreen Shot 2020-03-17 at 2 12 16 PM

@akosyakov
Copy link
Member

the search does not work properly (ex: search for 'a' yields not results but 'alt' does)

@vince-fugnitto I've filed an issue for the registry: eclipse/openvsx#34 cc @spoenemann

I'm not sure about other issues, they seem to be minor. I would better invest time into integration tests for typescript againt new Monaco. Someone else can improve styling after this PR is merged.

@akosyakov
Copy link
Member

I'm going to merge it tomorrow if no objection. Theia 1.0 is already next week.

@vince-fugnitto
Copy link
Member

the search does not work properly (ex: search for 'a' yields not results but 'alt' does)

@vince-fugnitto I've filed an issue for the registry: eclipse/openvsx#34 cc @spoenemann

👍

I'm not sure about other issues, they seem to be minor. I would better invest time into integration tests for typescript againt new Monaco. Someone else can improve styling after this PR is merged.

I'm fine with that, the improvements can be made iteratively at a later point.
I'll file issues for them once the pull-request is merged.

@vince-fugnitto
Copy link
Member

With the VSX (Extension) view now merged, is there still a reason to have the Plugins view?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants