-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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-registry: display incompatible extensions #10424
Conversation
The commit adds the functionality to display incompatible extensions when performing a search. The changes will now display incompatible extensions but mark them as disabled, and non-installable. The behavior is also controllable by a new preference so users can decide if they want the potential extra noise when searching. Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior looks good to me:
- Incompatible version (tested this with API version 1.53.2 and 1.30.0) are shown but not installable.
- Switching the preference on/off correctly starts/stops displaying extensions with incompatible versions
- The visual changes work well with different themes.
I have a few suggestions regarding i18n though.
Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments:
Functionality
- If you search for plugins and then change the preference, the current search result set doesn't change (the search isn't rerun). Particularly when moving from not showing -> showing, I think it would be better if the display did update.
- It seems that if a plugin is incompatible, you can't open an extension view widget for it to read its readme. That was unexpected, and seems undesirable.
Experience
Displaying a plugin that you can't interact with at all seems odd. Certainly it should be possible to open the extension view for it to see the readme, but I think it might also make sense to allow the user to install an incompatible plugin after a warning dialog that points out that the plugin may not work. Perhaps beyond the scope of this PR, but an excellent addition to it, if you're willing, would be to take the logic from #9330 to give the user the chance to select the oldest available version to install, in case that works better than the latest.
Converted the pull-request to a draft in favor of completing #9330 first to get a better behavior in this pull-request 👍 |
Moved the draft to my fork in the meantime vince-fugnitto#9. |
What it does
Fixes: #9676.
The commit adds the functionality to display incompatible extensions when performing a search. The changes will now display incompatible extensions but mark them as disabled, and non-installable. The behavior is also controllable by a new preference so users can decide if they want the potential extra noise when searching.
Example: using an api-version of 1.30.0:
How to test
3.1. hovering over the disabled extension should correctly display the tooltip
3.2 the plugin should not resolve, the readme won't be displayed
'extensions.displayIncompatible'
and re-searching should not yield incompatible resultsyarn start --vscode-api-version=1.30.0
for additional incompatible resultsReview checklist
Reminder for reviewers
Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com