Notify number of outdated extensions on Check for Extension updates action#56053
Conversation
ramya-rao-a
left a comment
There was a problem hiding this comment.
This will only show the extensions that have already been deemed outdated. You need to bring back the deleted code that checks for updates. When the promise returned by checkForUpdates is resolved, you can then use the viewlet service
|
Thanks for the patience 😄. Hope it works now as expected. |
|
I tried out the changes. Works very well when I run the But when the extensions view is open, and I then run the How about the below?
|
|
I agree with your suggestion. |
|
Also, with what you've said, we need to show the number of extension updates. In order to show the notification, I believe that we might need to add |
Yes definitely, my third point (
Yes, once
Use the |
|
@ramya-rao-a I've shown a different message when no extensions are outdated - |
| this.notificationService.notify( | ||
| { | ||
| severity: severity.Info, | ||
| message: localize('updatesAvailable', "{0} extensions are outdated.", outdatedCount) |
There was a problem hiding this comment.
extensions updates found sounds more positive than extensions are outdated :)
Also account for the case where only 1 extension is outdated. You will have to use 2 separate msgs.
| super(id, label, '', true); | ||
| } | ||
|
|
||
| private onUpdateNotAvailable(): void { |
There was a problem hiding this comment.
For brevity purposes, I would suggest to combine the onUpdateNotAvailable and onUpdateAvailable functions to a single function notifyUpdateAvailablility and make it take the outdated count as a parameter.
Then based on the count, you can first determine the msg to show and then use a single call to notificationService.notify
There was a problem hiding this comment.
Also, I just realized that it would be cool if we present an option to Update all extensions in this notification. Then we can skip the change of focus to the extension viewlet.
To do this, instead of notify you will need to use the prompt method. We already have the action UpdateAllAction, you will need to instantiate it using the instantiation service. For reference see https://github.com/Microsoft/vscode/blob/121bb10fd176fc29e790581449518554d53228b2/src/vs/workbench/parts/extensions/electron-browser/extensionsActions.ts#L57
There was a problem hiding this comment.
Also, I just realized that it would be cool if we present an option to Update all extensions in this notification. Then we can skip the change of focus to the extension viewlet.
I also thought of the same prompt while coding and redirecting to Update all extensions. Would be a nice to have option 👍
|
I got carried away with all the features we kept adding to this PR :)
I'll push a commit to remove the option. Apologies for the churn. |
|
@ParkourKarthik I've made some changes to the PR. Take a look. If you are fine by it as well, I'll merge the PR. |
|
Also, since you now know everything about outdated extensions, #22461 would be a good next item if you are interested in picking another item in VS Code :) |
|
I'm ok with your changes, you can merge it :) |
This fixes #55114