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

VSX: Add 'Install Another Version...' Command #9330

Closed
wants to merge 2 commits into from

Conversation

seantan22
Copy link
Contributor

What it does

  • Supports the ability to install any compatible version of an user-installed extension, provided that the extension is available in the Open VSX Registry.
install_another_version.mp4

How to test

  1. Open the extensions view and locate the Installed view of all user-installed extensions.
  2. Pick an extension --> Click the manage gear icon --> Click Install Another Version... command. Note that the command is disabled for any extension not available in the Open VSX Registry.
  3. Select a compatible version to install from the Quick Pick dropdown.
  4. Observe that the previous version is uninstalled and replaced with the newly selected version.

Review checklist

Reminder for reviewers

Signed-off-by: seantan22 [email protected]

@colin-grant-work colin-grant-work self-assigned this Oct 19, 2021
@colin-grant-work colin-grant-work force-pushed the st/install-version branch 4 times, most recently from 42a2fd6 to 6e02a59 Compare October 20, 2021 17:35
@vince-fugnitto vince-fugnitto added the vsx-registry Issues related to Open VSX Registry Integration label Oct 21, 2021
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

I've updated the coding guidelines regarding i18n. I'm currently working on tooling to simplify finding localization keys in vscode language packs.

@colin-grant-work colin-grant-work force-pushed the st/install-version branch 2 times, most recently from bdc4fac to a5c42d1 Compare November 4, 2021 22:20
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.

The changes overall look good to me, only had a minor comment about the quick-input.

Comment on lines 211 to 213
if (currentVersion === ext.version) {
publishedDate += ' (Current)';
}
Copy link
Member

@vince-fugnitto vince-fugnitto Nov 5, 2021

Choose a reason for hiding this comment

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

For some reason I do not see current added as a description to the current extension version:

current-version-ext.mp4

I also verified when it is not the first entry in the list. (not sure why I mentioned this)

What It Does

- Supports the ability to install any compatible version of an user-installed extension, provided that the extension is available in the Open VSX Registry.

How To Test

1. Open the extensions view and locate the `Installed` view of all user-installed extensions.
2. Pick an extension --> Click the `manage` gear icon --> Click `Install Another Version...` command. Note that the command is disabled fo any extension not available in the Open VSX Registry.
3. Select a compatible version to install from the Quick Pick dropdown.
4. Observe that the previous version is uninstalled and replaced with the newly selected version.

Signed-off-by: seantan22 <[email protected]>
@colin-grant-work colin-grant-work force-pushed the st/install-version branch 2 times, most recently from b30ed21 to 0d03186 Compare November 5, 2021 20:18
@colin-grant-work
Copy link
Contributor

colin-grant-work commented Nov 5, 2021

@vince-fugnitto, thanks for pointing out the (Current) issue. It got knocked out by some changes from Rob Moran's tooltip commit. You mentioned that the current version wasn't first - I don't think it should be, necessarily. Here's a screenshot from VSCode.

image

Earlier, you also expressed some misgivings about the mid-stream undeployment and redeployment. It seems as though VSCode pretty much always prompts you to reload if you want to change plugin versions. Should we just do the same, adding a button to reload if the plugin is awaiting deployment? In Electron that's fairly straightforward - I'm not quite sure what we should do in the Browser, since it would be expecting a full shutdown and restart of the back end.

@vince-fugnitto
Copy link
Member

You mentioned that the current version wasn't first - I don't think it should be, necessarily.

That's right, not sure why I mentioned it should be the first in the list, I believe I just wanted to express it should be added to the currently deployed version 👍

Earlier, you also expressed some misgivings about the mid-stream undeployment and redeployment. It seems as though VSCode pretty much always prompts you to reload if you want to change plugin versions. Should we just do the same, adding a button to reload if the plugin is awaiting deployment? In Electron that's fairly straightforward - I'm not quite sure what we should do in the Browser, since it would be expecting a full shutdown and restart of the back end.

I'm not sure either. Are there cases where a plugin will not be updated until the app is reloaded?

@colin-grant-work
Copy link
Contributor

@tsmaeder, would you be willing to weigh in here, since y'all at Red Hat probably have the best understanding of the plugin system. Is it / should it be safe to uninstall one version of a plugin and start a different version in the same session, or would it be preferable to restart the whole application when swapping out versions of plugins?

@tsmaeder
Copy link
Contributor

@colin-grant-work I have two observations:

  1. I didn't see any code that adds a plugin to a running plugin host process, so we'll have to at least restart the plugin host, which is usually done by reloading the browser window.
  2. Adding a plugin should not be a problem (we do it in "plugin dev mode"), but removing one may leave code running in existing plugin hosts that relies on resources that might not be there anymore. Think of an icon that has been removed in a newer version of the plugin. So I think replacing a plugin requires the back end to be restarted, because we don't have a separate set of deployed plugins per plugin host.

@paul-marechal
Copy link
Member

paul-marechal commented Dec 6, 2021

So I think replacing a plugin requires the back end to be restarted, because we don't have a separate set of deployed plugins per plugin host.

@tsmaeder Unless the backend stores plugin-specific data, I don't see why we would need to restart it. An extension's executable code is loaded into the plugin host process only and resources such as icons should be loaded in the frontend, so I'd expect to only need to restart frontend and plugin host?

@colin-grant-work
Copy link
Contributor

Planning to pick this back up, but ran into an issue with the queries to Open VSX. I'll try to work around it - I'm not sure we need the query that's failing.

@colin-grant-work
Copy link
Contributor

@JonasHelming, this one has been waiting on a solution for issues with plugin uninstallation: #11084. Once that's merged, I can finish this up straightforwardly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decision_needed issues that require decisions on ways forward (ex: if they should be updated or closed) vsx-registry Issues related to Open VSX Registry Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants