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

Use targetPlatform when installing plugin from open-vsx #13825

Merged
merged 4 commits into from
Jun 25, 2024

Conversation

msujew
Copy link
Member

@msujew msujew commented Jun 18, 2024

What it does

Closes #11291
Closes #13558

For a while now open-vsx has offered to query plugins using the targetPlatform filter. This adjusts Theia's open-vsx integration to respect the backend's target platform. This is related to #12410, which implemented this feature for the CLI.

When using the filter, only plugin entries with the specified target platform are actually returned. That's why we have to also query universal after querying the platform specific plugins to ensure that we actually install the newest available extension.

Also closes #11291 by using the new v2 API endpoint which returns more compact results.

How to test

Testing this works best when you have two different operating systems available. I used Windows (PC) and Ubuntu (Gitpod) to test this.

  1. Install a plugin with native dependencies such as the rust-analyzer
  2. Assert that the plugin starts up when opening a *.rs file (it currently crashes on master if you aren't accidentally on the correct OS)
  3. Assert that all the other plugin related features work as expected

Follow-ups

I'm not sure this is how the open-vsx API is intended to be used. Querying the API multiple times for different target platforms (i.e. the backend platform + universal) seems a bit counterintuitive. We can maybe improve on this.

Review checklist

Reminder for reviewers

Copy link
Contributor

@jfaltermeier jfaltermeier left a comment

Choose a reason for hiding this comment

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

Looks good to me. I've tested it on Ubuntu and in a Windows VM, and the expected extensions were installed correctly. I've added two comments/questions inline for clarification.

dev-packages/ovsx-client/src/ovsx-api-filter.ts Outdated Show resolved Hide resolved
dev-packages/ovsx-client/src/ovsx-api-filter.ts Outdated Show resolved Hide resolved
@msujew msujew requested a review from jfaltermeier June 24, 2024 09:43
Copy link
Contributor

@jfaltermeier jfaltermeier left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm!

@msujew msujew merged commit 1a5d8d8 into master Jun 25, 2024
14 checks passed
@msujew msujew deleted the msujew/ovsx-api-v2 branch June 25, 2024 09:39
@github-actions github-actions bot added this to the 1.51.0 milestone Jun 25, 2024
@msujew msujew mentioned this pull request Jun 25, 2024
1 task
@msujew msujew mentioned this pull request Aug 9, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-vsx issues related to the open-vsx registry
Projects
Archived in project
2 participants