Add support for dynamic updates in interpreter list - #17043
Merged
Kartik Raj (karrtikr) merged 19 commits intoSep 21, 2021
Conversation
Kartik Raj (karrtikr)
force-pushed
the
improveinterpreterdisplay
branch
3 times, most recently
from
August 23, 2021 21:45
07c95c5 to
d1c9d31
Compare
Kartik Raj (karrtikr)
force-pushed
the
improveinterpreterdisplay
branch
2 times, most recently
from
September 9, 2021 19:26
a8485cb to
c38f2b0
Compare
Kartik Raj (karrtikr)
force-pushed
the
improveinterpreterdisplay
branch
from
September 15, 2021 23:16
2093a8f to
0e5dece
Compare
Kartik Raj (karrtikr)
marked this pull request as ready for review
September 16, 2021 23:12
added 2 commits
September 16, 2021 16:19
| const invalidIndexes = areEnvsValid.map((isValid, index) => (isValid ? -1 : index)).filter((i) => i !== -1); | ||
| invalidIndexes.forEach((index) => { | ||
| const env = this.envs.splice(index, 1)[0]; | ||
| this.fire({ old: env, update: undefined }); |
Author
There was a problem hiding this comment.
This is to ensure we fire events for any envs removed from collection.
|
|
||
| public get refreshPromise(): Promise<void> { | ||
| return Promise.all(Array.from(this.refreshPromises.values())).then(); | ||
| public get refreshPromise(): Promise<void> | undefined { |
There was a problem hiding this comment.
We only need to set quickpick as busy if a refresh is going on, so returning undefined is helpful.
| await updateEnvUsingRegistry(resolvedEnv); | ||
| } | ||
| // Display name is not set here as we need version, arch etc. to build it. | ||
| resolvedEnv.display = getEnvDisplayString(resolvedEnv); |
There was a problem hiding this comment.
Allow to show partial display names with the partial info we have.
| */ | ||
| @cache(30_000, true, 10_000) | ||
| // eslint-disable-next-line class-methods-use-this | ||
| private async getInfoCached(command: string): Promise<CondaInfo> { |
There was a problem hiding this comment.
Noticed it was not cached and was being run twice.
| iconPath: getIcon(REFRESH_BUTTON_ICON), | ||
| tooltip: InterpreterQuickPickList.refreshInterpreterList(), | ||
| }, | ||
| callback: () => this.interpreterService.triggerRefresh(), |
There was a problem hiding this comment.
All we need to do is trigger the refresh, the list is automatically updated one by one via change events in the onChanged handler.
Kartik Raj (karrtikr)
requested review from
Karthik Nadig (karthiknadig) and
Kim-Adeline Miguel (kimadeline)
September 16, 2021 23:49
|
|
||
| private async setRecommendedItem(items: QuickPickType[], resource: Resource) { | ||
| const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource); | ||
| if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { |
There was a problem hiding this comment.
Notice we only set a recommended item only after refresh has finished, otherwise recommended item can change while the list is loading. Let me know if you disagree..
This was referenced Sep 17, 2021
Adopt new Python extensions non-blocking API for interpreter disocvery
microsoft/vscode-jupyter#7583
Closed
Kim-Adeline Miguel (kimadeline)
approved these changes
Sep 20, 2021
Author
Karthik Nadig (karthiknadig)
approved these changes
Sep 21, 2021
Karthik Nadig (karthiknadig)
left a comment
Karthik Nadig (karthiknadig)
left a comment
Member
There was a problem hiding this comment.
Nice work!!!
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes https://github.com/microsoft/vscode-python-internalbacklog/issues/380
Quickpick loads with the sorted list of known environments. If an env gets added to the known list, its added to the end of items list. A lot of effort here is done so that the visible items do not change when the list is loading.