From 463824ab4414256d500b37620d11228faee86342 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 23 Aug 2021 14:44:13 -0700 Subject: [PATCH 01/19] Add support for dynamic updates in interpreter list --- src/client/common/utils/multiStepInput.ts | 11 ++++++- .../commands/setInterpreter.ts | 26 ++++++++------- .../interpreterSelector.ts | 33 +++++++++++++++++-- src/client/interpreter/configuration/types.ts | 9 ++++- src/client/jupyter/jupyterIntegration.ts | 2 +- .../commands/setInterpreter.unit.test.ts | 26 ++++++++++----- .../interpreterSelector.unit.test.ts | 4 +-- 7 files changed, 83 insertions(+), 28 deletions(-) diff --git a/src/client/common/utils/multiStepInput.ts b/src/client/common/utils/multiStepInput.ts index f327b8eb97ff..ee5bfc586fcd 100644 --- a/src/client/common/utils/multiStepInput.ts +++ b/src/client/common/utils/multiStepInput.ts @@ -6,7 +6,7 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { Disposable, QuickInput, QuickInputButton, QuickInputButtons, QuickPick, QuickPickItem } from 'vscode'; +import { Disposable, QuickInput, QuickInputButton, QuickInputButtons, QuickPick, QuickPickItem, Event } from 'vscode'; import { IApplicationShell } from '../application/types'; // Borrowed from https://github.com/Microsoft/vscode-extension-samples/blob/master/quickinput-sample/src/multiStepInput.ts @@ -51,6 +51,7 @@ export interface IQuickPickParameters { matchOnDescription?: boolean; matchOnDetail?: boolean; acceptFilterBoxTextAsSelection?: boolean; + onChangeItem?: { getItems: () => Promise; event: Event }; } interface InputBoxParameters { @@ -110,6 +111,7 @@ export class MultiStepInput implements IMultiStepInput { matchOnDescription, matchOnDetail, acceptFilterBoxTextAsSelection, + onChangeItem, }: P): Promise> { const disposables: Disposable[] = []; try { @@ -160,6 +162,13 @@ export class MultiStepInput implements IMultiStepInput { this.current.dispose(); } this.current = input; + if (onChangeItem) { + disposables.push( + onChangeItem.event(async () => { + input.items = await onChangeItem.getItems(); + }), + ); + } this.current.show(); }); } finally { diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index d692f92137ed..8da1f4bf6f5f 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -22,6 +22,7 @@ import { import { REFRESH_BUTTON_ICON } from '../../../../debugger/extension/attachQuickPick/types'; import { captureTelemetry, sendTelemetryEvent } from '../../../../telemetry'; import { EventName } from '../../../../telemetry/constants'; +import { IInterpreterService } from '../../../contracts'; import { IInterpreterQuickPickItem, IInterpreterSelector, @@ -48,6 +49,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { @inject(IPlatformService) private readonly platformService: IPlatformService, @inject(IInterpreterSelector) private readonly interpreterSelector: IInterpreterSelector, @inject(IWorkspaceService) workspaceService: IWorkspaceService, + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, ) { super(pythonPathUpdaterService, commandManager, applicationShell, workspaceService); } @@ -104,7 +106,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { state.workspace ? state.workspace.fsPath : undefined, ); - let activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); + const activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); state.path = undefined; const refreshButton = { @@ -117,11 +119,9 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { activeItem: activeInterpreter.length > 0 ? activeInterpreter[0] : interpreterSuggestions[0], matchOnDetail: true, matchOnDescription: true, - customButtonSetup: { - button: refreshButton, - callback: async (quickPick) => { - quickPick.busy = true; - + onChangeItem: { + event: this.interpreterSelector.onChanged, + getItems: async () => { interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace, true); if (interpreterSuggestions.length > 0) { const suggested = interpreterSuggestions.shift(); @@ -133,15 +133,17 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } } - const newSuggestions = defaultInterpreterPathSuggestion + return defaultInterpreterPathSuggestion ? [manualEntrySuggestion, defaultInterpreterPathSuggestion, ...interpreterSuggestions] : [manualEntrySuggestion, ...interpreterSuggestions]; + }, + }, + customButtonSetup: { + button: refreshButton, + callback: async (quickPick) => { + quickPick.busy = true; - activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); - - quickPick.items = newSuggestions; - quickPick.activeItems = - activeInterpreter.length > 0 ? [activeInterpreter[0]] : [interpreterSuggestions[0]]; + await this.interpreterService.triggerRefresh(); quickPick.busy = false; }, diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index 3b60e25169fc..dafea2cd6ddf 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -4,27 +4,54 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { Disposable, Uri } from 'vscode'; +import { Disposable, EventEmitter, Uri, Event } from 'vscode'; import { IPathUtils, Resource } from '../../../common/types'; import { PythonEnvironment } from '../../../pythonEnvironments/info'; import { IInterpreterService } from '../../contracts'; -import { IInterpreterComparer, IInterpreterQuickPickItem, IInterpreterSelector } from '../types'; +import { + IInterpreterComparer, + IInterpreterQuickPickItem, + IInterpreterSelector, + PythonEnvSuggestionChangedEvent, +} from '../types'; @injectable() export class InterpreterSelector implements IInterpreterSelector { private disposables: Disposable[] = []; + private readonly changed = new EventEmitter(); + constructor( @inject(IInterpreterService) private readonly interpreterManager: IInterpreterService, @inject(IInterpreterComparer) private readonly envTypeComparer: IInterpreterComparer, @inject(IPathUtils) private readonly pathUtils: IPathUtils, - ) {} + ) { + this.interpreterManager.onDidChangeInterpreters(async (event) => { + this.changed.fire({ + old: event.old ? await this.suggestionToQuickPickItem(event.old) : event.old, + update: event.update ? await this.suggestionToQuickPickItem(event.update) : event.update, + }); + }); + } + + public get onChanged(): Event { + return this.changed.event; + } public dispose(): void { this.disposables.forEach((disposable) => disposable.dispose()); } public async getSuggestions(resource: Resource, ignoreCache?: boolean): Promise { + const interpreters = await this.interpreterManager.getInterpreters(resource, { + onSuggestion: true, + ignoreCache, + }); + + return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource))); + } + + public async getAllSuggestions(resource: Resource, ignoreCache?: boolean): Promise { const interpreters = await this.interpreterManager.getAllInterpreters(resource, { onSuggestion: true, ignoreCache, diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 90384d248912..0b2f62865432 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -1,4 +1,4 @@ -import { ConfigurationTarget, Disposable, QuickPickItem, Uri } from 'vscode'; +import { ConfigurationTarget, Disposable, QuickPickItem, Uri, Event } from 'vscode'; import { Resource } from '../../common/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -23,8 +23,15 @@ export interface IPythonPathUpdaterServiceManager { ): Promise; } +export type PythonEnvSuggestionChangedEvent = { + old?: IInterpreterQuickPickItem; + update?: IInterpreterQuickPickItem | undefined; +}; + export const IInterpreterSelector = Symbol('IInterpreterSelector'); export interface IInterpreterSelector extends Disposable { + readonly onChanged: Event; + getAllSuggestions(resource: Resource, ignoreCache?: boolean): Promise; getSuggestions(resource: Resource, ignoreCache?: boolean): Promise; } diff --git a/src/client/jupyter/jupyterIntegration.ts b/src/client/jupyter/jupyterIntegration.ts index 1974270bdc88..9b512f122664 100644 --- a/src/client/jupyter/jupyterIntegration.ts +++ b/src/client/jupyter/jupyterIntegration.ts @@ -183,7 +183,7 @@ export class JupyterExtensionIntegration { return isWindowsStoreInterpreter(pythonPath); }, getSuggestions: async (resource: Resource): Promise => - this.interpreterSelector.getSuggestions(resource), + this.interpreterSelector.getAllSuggestions(resource), install: async ( product: JupyterProductToInstall, resource?: InterpreterUri, diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 6d8ff947542f..c0d2a109a8ec 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -133,10 +133,10 @@ suite('Set Interpreter Command', () => { }; }); interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) .returns(() => Promise.resolve([item])); interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) + .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([refreshedItem])); pythonSettings.setup((p) => p.pythonPath).returns(() => currentPythonPath); pythonSettings.setup((p) => p.defaultInterpreterPath).returns(() => defaultInterpreterPath); @@ -521,7 +521,9 @@ suite('Set Interpreter Command', () => { workspace.setup((w) => w.workspaceFolders).returns(() => undefined); - interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); + interpreterSelector + .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .returns(() => Promise.resolve([])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { state.path = selectedItem.path; @@ -561,7 +563,9 @@ suite('Set Interpreter Command', () => { const folder = { name: 'one', uri: Uri.parse('one'), index: 0 }; workspace.setup((w) => w.workspaceFolders).returns(() => [folder]); - interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); + interpreterSelector + .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .returns(() => Promise.resolve([])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { @@ -617,7 +621,9 @@ suite('Set Interpreter Command', () => { }, ]; - interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); + interpreterSelector + .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .returns(() => Promise.resolve([])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { @@ -684,7 +690,7 @@ suite('Set Interpreter Command', () => { ]; interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) .returns(() => Promise.resolve([selectedItem])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { @@ -723,7 +729,9 @@ suite('Set Interpreter Command', () => { test('Do not update anything when user does not select a workspace folder and there is more than one workspace folder', async () => { workspace.setup((w) => w.workspaceFolders).returns(() => [folder1, folder2]); - interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); + interpreterSelector + .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .returns(() => Promise.resolve([])); multiStepInputFactory.setup((f) => f.create()).verifiable(TypeMoq.Times.never()); const expectedItems = [ @@ -788,7 +796,9 @@ suite('Set Interpreter Command', () => { workspace.setup((w) => w.workspaceFolders).returns(() => undefined); - interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); + interpreterSelector + .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .returns(() => Promise.resolve([])); const multiStepInput = { run: (inputStepArg: InputStepType, state: InterpreterStateArgs) => { inputStep = inputStepArg; diff --git a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts index 4d01bb5bd9e0..ca53272e2a95 100644 --- a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts @@ -94,7 +94,7 @@ suite('Interpreters - selector', () => { .setup((x) => x.getAllInterpreters(TypeMoq.It.isAny(), { onSuggestion: true, ignoreCache })) .returns(() => new Promise((resolve) => resolve(initial))); - const actual = await selector.getSuggestions(undefined, ignoreCache); + const actual = await selector.getAllSuggestions(undefined, ignoreCache); const expected: InterpreterQuickPickItem[] = [ new InterpreterQuickPickItem('1', 'c:/path1/path1'), @@ -169,7 +169,7 @@ suite('Interpreters - selector', () => { new PathUtils(getOSType() === OSType.Windows), ); - const result = await selector.getSuggestions(undefined, ignoreCache); + const result = await selector.getAllSuggestions(undefined, ignoreCache); const expected: InterpreterQuickPickItem[] = [ new InterpreterQuickPickItem('two', path.join(workspacePath, '.venv', 'bin', 'python')), From 018a0e5777bf343b66cdcac77097588e0d7f61a2 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 23 Aug 2021 14:47:22 -0700 Subject: [PATCH 02/19] News entry --- news/1 Enhancements/17043.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1 Enhancements/17043.md diff --git a/news/1 Enhancements/17043.md b/news/1 Enhancements/17043.md new file mode 100644 index 000000000000..35c4cc0e3c19 --- /dev/null +++ b/news/1 Enhancements/17043.md @@ -0,0 +1 @@ +Add support for dynamic updates in interpreter list. From 65e6a6880c16d061f5f7a1311142ce2a8ac2b78a Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 23 Aug 2021 17:13:27 -0700 Subject: [PATCH 03/19] Cleanup --- src/client/common/utils/multiStepInput.ts | 14 +++++------ .../commands/setInterpreter.ts | 23 +++++++++++++++---- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/client/common/utils/multiStepInput.ts b/src/client/common/utils/multiStepInput.ts index ee5bfc586fcd..04b1976c3cd7 100644 --- a/src/client/common/utils/multiStepInput.ts +++ b/src/client/common/utils/multiStepInput.ts @@ -39,7 +39,8 @@ type QuickInputButtonSetup = { */ callback: buttonCallbackType; }; -export interface IQuickPickParameters { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export interface IQuickPickParameters { title?: string; step?: number; totalSteps?: number; @@ -51,7 +52,10 @@ export interface IQuickPickParameters { matchOnDescription?: boolean; matchOnDetail?: boolean; acceptFilterBoxTextAsSelection?: boolean; - onChangeItem?: { getItems: () => Promise; event: Event }; + onChangeItem?: { + callback: (event: E, quickPick: QuickPick) => Promise; + event: Event; + }; } interface InputBoxParameters { @@ -163,11 +167,7 @@ export class MultiStepInput implements IMultiStepInput { } this.current = input; if (onChangeItem) { - disposables.push( - onChangeItem.event(async () => { - input.items = await onChangeItem.getItems(); - }), - ); + disposables.push(onChangeItem.event((e) => onChangeItem.callback(e, input))); } this.current.show(); }); diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 8da1f4bf6f5f..b646290a66ad 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -28,6 +28,7 @@ import { IInterpreterSelector, IPythonPathUpdaterServiceManager, ISpecialQuickPickItem, + PythonEnvSuggestionChangedEvent, } from '../../types'; import { BaseInterpreterSelectorCommand } from './base'; @@ -106,14 +107,17 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { state.workspace ? state.workspace.fsPath : undefined, ); - const activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); + let activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); state.path = undefined; const refreshButton = { iconPath: getIcon(REFRESH_BUTTON_ICON), tooltip: InterpreterQuickPickList.refreshInterpreterList(), }; - const selection = await input.showQuickPick>({ + const selection = await input.showQuickPick< + QuickPickType, + IQuickPickParameters + >({ placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), items: suggestions, activeItem: activeInterpreter.length > 0 ? activeInterpreter[0] : interpreterSuggestions[0], @@ -121,7 +125,13 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { matchOnDescription: true, onChangeItem: { event: this.interpreterSelector.onChanged, - getItems: async () => { + callback: async (_event, quickPick) => { + quickPick.busy = true; + this.interpreterService.refreshPromise.then(() => { + this.interpreterService.refreshPromise.then(() => { + quickPick.busy = false; + }); + }); interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace, true); if (interpreterSuggestions.length > 0) { const suggested = interpreterSuggestions.shift(); @@ -133,9 +143,14 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } } - return defaultInterpreterPathSuggestion + const newSuggestions = defaultInterpreterPathSuggestion ? [manualEntrySuggestion, defaultInterpreterPathSuggestion, ...interpreterSuggestions] : [manualEntrySuggestion, ...interpreterSuggestions]; + activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); + + quickPick.items = newSuggestions; + quickPick.activeItems = + activeInterpreter.length > 0 ? [activeInterpreter[0]] : [interpreterSuggestions[0]]; }, }, customButtonSetup: { From c68781afb196559de0e753b3407f68eaa5e62bbb Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 8 Sep 2021 20:14:21 -0700 Subject: [PATCH 04/19] Add implementation to preserve scroll position --- src/client/common/utils/multiStepInput.ts | 6 ++ .../commands/setInterpreter.ts | 71 +++++++++---------- src/client/interpreter/contracts.ts | 4 +- src/client/interpreter/interpreterService.ts | 2 +- src/client/pythonEnvironments/base/locator.ts | 4 +- .../composite/envsCollectionService.ts | 6 +- types/vscode.proposed.d.ts | 12 ++++ 7 files changed, 59 insertions(+), 46 deletions(-) diff --git a/src/client/common/utils/multiStepInput.ts b/src/client/common/utils/multiStepInput.ts index 04b1976c3cd7..472a703830c2 100644 --- a/src/client/common/utils/multiStepInput.ts +++ b/src/client/common/utils/multiStepInput.ts @@ -51,6 +51,8 @@ export interface IQuickPickParameters { customButtonSetup?: QuickInputButtonSetup; matchOnDescription?: boolean; matchOnDetail?: boolean; + keepScrollPosition?: boolean; + sortByLabel?: boolean; acceptFilterBoxTextAsSelection?: boolean; onChangeItem?: { callback: (event: E, quickPick: QuickPick) => Promise; @@ -116,6 +118,8 @@ export class MultiStepInput implements IMultiStepInput { matchOnDetail, acceptFilterBoxTextAsSelection, onChangeItem, + keepScrollPosition, + sortByLabel }: P): Promise> { const disposables: Disposable[] = []; try { @@ -123,6 +127,8 @@ export class MultiStepInput implements IMultiStepInput { const input = this.shell.createQuickPick(); input.title = title; input.step = step; + input.keepScrollPosition = keepScrollPosition; + input.sortByLabel = sortByLabel || false; input.totalSteps = totalSteps; input.placeholder = placeholder; input.ignoreFocusOut = true; diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index b646290a66ad..48f8f06d19f3 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -9,6 +9,7 @@ import * as path from 'path'; import { QuickPickItem } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types'; import { Commands, Octicons } from '../../../../common/constants'; +import { traceWarning } from '../../../../common/logger'; import { IPlatformService } from '../../../../common/platform/types'; import { IConfigurationService, IPathUtils, Resource } from '../../../../common/types'; import { getIcon } from '../../../../common/utils/icons'; @@ -28,7 +29,6 @@ import { IInterpreterSelector, IPythonPathUpdaterServiceManager, ISpecialQuickPickItem, - PythonEnvSuggestionChangedEvent, } from '../../types'; import { BaseInterpreterSelectorCommand } from './base'; @@ -89,15 +89,21 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { suggestions.push(defaultInterpreterPathSuggestion); } - let interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace); - - if (interpreterSuggestions.length > 0) { - const suggested = interpreterSuggestions.shift(); - if (suggested) { - const starred = cloneDeep(suggested); - starred.label = `${Octicons.Star} ${starred.label}`; - starred.description = Common.recommended(); - interpreterSuggestions.unshift(starred); + let isRefreshing = false; + let interpreterSuggestions: IInterpreterQuickPickItem[]; + if (this.interpreterService.refreshPromise) { + isRefreshing = true; + interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace); + } else { + interpreterSuggestions = await this.interpreterSelector.getAllSuggestions(state.workspace); + if (interpreterSuggestions.length > 0) { + const suggested = interpreterSuggestions.shift(); + if (suggested) { + const starred = cloneDeep(suggested); + starred.label = `${Octicons.Star} ${starred.label}`; + starred.description = Common.recommended(); + interpreterSuggestions.unshift(starred); + } } } suggestions.push(...interpreterSuggestions); @@ -114,54 +120,41 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { iconPath: getIcon(REFRESH_BUTTON_ICON), tooltip: InterpreterQuickPickList.refreshInterpreterList(), }; - const selection = await input.showQuickPick< - QuickPickType, - IQuickPickParameters - >({ + const selection = await input.showQuickPick>({ placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), items: suggestions, - activeItem: activeInterpreter.length > 0 ? activeInterpreter[0] : interpreterSuggestions[0], + // If the list is refreshing, adding elements in the end is only + // way to preserve scroll position, so we don't need sorting. + sortByLabel: !isRefreshing, + keepScrollPosition: true, + activeItem: undefined, matchOnDetail: true, matchOnDescription: true, onChangeItem: { - event: this.interpreterSelector.onChanged, + event: this.interpreterService.onDidChangeInterpreters, callback: async (_event, quickPick) => { - quickPick.busy = true; - this.interpreterService.refreshPromise.then(() => { - this.interpreterService.refreshPromise.then(() => { + if (this.interpreterService.refreshPromise) { + quickPick.busy = true; + this.interpreterService.refreshPromise.then(async () => { + // TODO: Suggested a recommended interpreter now that refresh has finished quickPick.busy = false; }); - }); - interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace, true); - if (interpreterSuggestions.length > 0) { - const suggested = interpreterSuggestions.shift(); - if (suggested) { - const starred = cloneDeep(suggested); - starred.label = `${Octicons.Star} ${starred.label}`; - starred.description = Common.recommended(); - interpreterSuggestions.unshift(starred); - } + } else { + traceWarning('An ongoing refresh is expected if interpreter quickpick list is changing'); } - const newSuggestions = defaultInterpreterPathSuggestion + quickPick.items = defaultInterpreterPathSuggestion ? [manualEntrySuggestion, defaultInterpreterPathSuggestion, ...interpreterSuggestions] : [manualEntrySuggestion, ...interpreterSuggestions]; - activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); - quickPick.items = newSuggestions; + activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); quickPick.activeItems = activeInterpreter.length > 0 ? [activeInterpreter[0]] : [interpreterSuggestions[0]]; }, }, customButtonSetup: { button: refreshButton, - callback: async (quickPick) => { - quickPick.busy = true; - - await this.interpreterService.triggerRefresh(); - - quickPick.busy = false; - }, + callback: () => this.interpreterService.triggerRefresh(), }, title: InterpreterQuickPickList.browsePath.openButtonLabel(), }); diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts index 3853fe59632d..363e7c7e7416 100644 --- a/src/client/interpreter/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -43,7 +43,7 @@ export const IComponentAdapter = Symbol('IComponentAdapter'); export interface IComponentAdapter { readonly onRefreshStart: Event; triggerRefresh(query?: PythonLocatorQuery): Promise; - readonly refreshPromise: Promise; + readonly refreshPromise: Promise | undefined; readonly onChanged: Event; // VirtualEnvPrompt onDidCreate(resource: Resource, callback: () => void): Disposable; @@ -111,7 +111,7 @@ export interface ICondaLocatorService { export const IInterpreterService = Symbol('IInterpreterService'); export interface IInterpreterService { triggerRefresh(query?: PythonLocatorQuery): Promise; - readonly refreshPromise: Promise; + readonly refreshPromise: Promise | undefined; readonly onDidChangeInterpreters: Event; onDidChangeInterpreterConfiguration: Event; onDidChangeInterpreter: Event; diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index f48eacff3511..32293a3c61ca 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -63,7 +63,7 @@ export class InterpreterService implements Disposable, IInterpreterService { : Promise.resolve(); } - public get refreshPromise(): Promise { + public get refreshPromise(): Promise | undefined { return inDiscoveryExperimentSync(this.experimentService) ? this.pyenvs.refreshPromise : Promise.resolve(); } diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts index b24ac553c5b6..59f61b5adc78 100644 --- a/src/client/pythonEnvironments/base/locator.ts +++ b/src/client/pythonEnvironments/base/locator.ts @@ -176,9 +176,9 @@ export interface IDiscoveryAPI { readonly onChanged: Event; /** * Resolves once environment list has finished refreshing, i.e all environments are - * discovered. + * discovered. Carries `undefined` if there is no refresh currently going on. */ - readonly refreshPromise: Promise; + readonly refreshPromise: Promise | undefined; /** * Triggers a new refresh for query if there isn't any already running. */ diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts index 71f76b6e9ee3..3f88fd0994f4 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts @@ -30,8 +30,10 @@ export class EnvsCollectionService extends PythonEnvsWatcher { - return Promise.all(Array.from(this.refreshPromises.values())).then(); + public get refreshPromise(): Promise | undefined { + return this.refreshPromises.size > 0 + ? Promise.all(Array.from(this.refreshPromises.values())).then() + : undefined; } constructor(private readonly cache: IEnvsCollectionCache, private readonly locator: IResolvingLocator) { diff --git a/types/vscode.proposed.d.ts b/types/vscode.proposed.d.ts index 89a2d2f12185..3550b5c667e9 100644 --- a/types/vscode.proposed.d.ts +++ b/types/vscode.proposed.d.ts @@ -750,6 +750,18 @@ declare module 'vscode' { replaceOutputItems(items: NotebookCellOutputItem | NotebookCellOutputItem[], outputId: string): Thenable; } + export interface QuickPick extends QuickInput { + /** + * An optional flag to sort the final results by index of first query match in label. Defaults to true. + */ + sortByLabel: boolean; + + /* + * An optional flag that can be set to true to maintain the scroll position of the quick pick when the quick pick items are updated. Defaults to false. + */ + keepScrollPosition?: boolean; + } + export enum NotebookCellExecutionState { Idle = 1, Pending = 2, From bed8350981bf3605fa872989470bc2cc195f5b1c Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 9 Sep 2021 15:29:36 -0700 Subject: [PATCH 05/19] Fix --- .../configuration/interpreterSelector/commands/setInterpreter.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 48f8f06d19f3..16254fd5c2c7 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -143,6 +143,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { traceWarning('An ongoing refresh is expected if interpreter quickpick list is changing'); } + interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace); quickPick.items = defaultInterpreterPathSuggestion ? [manualEntrySuggestion, defaultInterpreterPathSuggestion, ...interpreterSuggestions] : [manualEntrySuggestion, ...interpreterSuggestions]; From 467c874ff854d9934c928023da2dfa4abbeb513d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 14 Sep 2021 18:22:34 -0700 Subject: [PATCH 06/19] Refactor code nicely --- .../commands/setInterpreter.ts | 165 +++++++++--------- .../interpreterSelector.ts | 10 +- src/client/interpreter/configuration/types.ts | 4 +- .../common/environmentManagers/conda.ts | 11 +- .../common/environmentManagers/poetry.ts | 10 +- .../environmentManagers/poetry.unit.test.ts | 12 +- 6 files changed, 114 insertions(+), 98 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 16254fd5c2c7..446dd8451ad6 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -9,7 +9,6 @@ import * as path from 'path'; import { QuickPickItem } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types'; import { Commands, Octicons } from '../../../../common/constants'; -import { traceWarning } from '../../../../common/logger'; import { IPlatformService } from '../../../../common/platform/types'; import { IConfigurationService, IPathUtils, Resource } from '../../../../common/types'; import { getIcon } from '../../../../common/utils/icons'; @@ -39,6 +38,10 @@ type QuickPickType = IInterpreterQuickPickItem | ISpecialQuickPickItem; @injectable() export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { + private readonly manualEntrySuggestion: ISpecialQuickPickItem = { + label: `${Octicons.Add} ${InterpreterQuickPickList.enterPath.label()}`, + alwaysShow: true, + }; constructor( @inject(IApplicationShell) applicationShell: IApplicationShell, @inject(IPathUtils) private readonly pathUtils: IPathUtils, @@ -65,106 +68,65 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { input: IMultiStepInput, state: InterpreterStateArgs, ): Promise> { - const suggestions: QuickPickType[] = []; - - const manualEntrySuggestion: ISpecialQuickPickItem = { - label: `${Octicons.Add} ${InterpreterQuickPickList.enterPath.label()}`, - alwaysShow: true, - }; - suggestions.push(manualEntrySuggestion); - - const config = this.workspaceService.getConfiguration('python', state.workspace); - const defaultInterpreterPathValue = config.get('defaultInterpreterPath'); - let defaultInterpreterPathSuggestion: ISpecialQuickPickItem | undefined; - if (defaultInterpreterPathValue && defaultInterpreterPathValue !== 'python') { - defaultInterpreterPathSuggestion = { - label: `${Octicons.Gear} ${InterpreterQuickPickList.defaultInterpreterPath.label()}`, - detail: this.pathUtils.getDisplayName( - defaultInterpreterPathValue, - state.workspace ? state.workspace.fsPath : undefined, - ), - path: defaultInterpreterPathValue, - alwaysShow: true, - }; - suggestions.push(defaultInterpreterPathSuggestion); - } - - let isRefreshing = false; - let interpreterSuggestions: IInterpreterQuickPickItem[]; - if (this.interpreterService.refreshPromise) { - isRefreshing = true; - interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace); - } else { - interpreterSuggestions = await this.interpreterSelector.getAllSuggestions(state.workspace); - if (interpreterSuggestions.length > 0) { - const suggested = interpreterSuggestions.shift(); - if (suggested) { - const starred = cloneDeep(suggested); - starred.label = `${Octicons.Star} ${starred.label}`; - starred.description = Common.recommended(); - interpreterSuggestions.unshift(starred); - } - } - } - suggestions.push(...interpreterSuggestions); - - const currentPythonPath = this.pathUtils.getDisplayName( + // If the list is refreshing, adding elements in the end is only + // way to preserve scroll position, so we don't need sorting. + const sortList = !this.interpreterService.refreshPromise; + const suggestions = await this.getItems(state.workspace, sortList); + state.path = undefined; + const currentInterpreterPathDisplay = this.pathUtils.getDisplayName( this.configurationService.getSettings(state.workspace).pythonPath, state.workspace ? state.workspace.fsPath : undefined, ); - - let activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); - - state.path = undefined; - const refreshButton = { - iconPath: getIcon(REFRESH_BUTTON_ICON), - tooltip: InterpreterQuickPickList.refreshInterpreterList(), - }; const selection = await input.showQuickPick>({ - placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), + placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentInterpreterPathDisplay), items: suggestions, - // If the list is refreshing, adding elements in the end is only - // way to preserve scroll position, so we don't need sorting. - sortByLabel: !isRefreshing, + sortByLabel: sortList, keepScrollPosition: true, - activeItem: undefined, + activeItem: this.getActiveItem(state.workspace, suggestions), matchOnDetail: true, matchOnDescription: true, + title: InterpreterQuickPickList.browsePath.openButtonLabel(), + customButtonSetup: { + button: { + iconPath: getIcon(REFRESH_BUTTON_ICON), + tooltip: InterpreterQuickPickList.refreshInterpreterList(), + }, + callback: () => this.interpreterService.triggerRefresh(), + }, onChangeItem: { event: this.interpreterService.onDidChangeInterpreters, callback: async (_event, quickPick) => { if (this.interpreterService.refreshPromise) { quickPick.busy = true; this.interpreterService.refreshPromise.then(async () => { - // TODO: Suggested a recommended interpreter now that refresh has finished + // TODO: Suggested a recommended interpreter now that refresh has finished? quickPick.busy = false; + const interpreterSuggestions = await this.getItems(state.workspace, false); + if (quickPick.activeItems.length === 0) { + // Changing active items if one is already set is not a good idea as user might be using it. + quickPick.activeItems = [this.getActiveItem(state.workspace, interpreterSuggestions)]; + } }); - } else { - traceWarning('An ongoing refresh is expected if interpreter quickpick list is changing'); } - interpreterSuggestions = await this.interpreterSelector.getSuggestions(state.workspace); - quickPick.items = defaultInterpreterPathSuggestion - ? [manualEntrySuggestion, defaultInterpreterPathSuggestion, ...interpreterSuggestions] - : [manualEntrySuggestion, ...interpreterSuggestions]; - - activeInterpreter = interpreterSuggestions.filter((i) => i.detail === currentPythonPath); - quickPick.activeItems = - activeInterpreter.length > 0 ? [activeInterpreter[0]] : [interpreterSuggestions[0]]; + const interpreterSuggestions = await this.getItems( + state.workspace, + !this.interpreterService.refreshPromise, + ); + quickPick.items = interpreterSuggestions; + if (quickPick.activeItems.length === 0) { + // Changing active items if one is already set is not a good idea as user might be using it. + quickPick.activeItems = [this.getActiveItem(state.workspace, interpreterSuggestions)]; + } }, }, - customButtonSetup: { - button: refreshButton, - callback: () => this.interpreterService.triggerRefresh(), - }, - title: InterpreterQuickPickList.browsePath.openButtonLabel(), }); if (selection === undefined) { sendTelemetryEvent(EventName.SELECT_INTERPRETER_SELECTED, undefined, { action: 'escape' }); - } else if (selection.label === manualEntrySuggestion.label) { + } else if (selection.label === this.manualEntrySuggestion.label) { sendTelemetryEvent(EventName.SELECT_INTERPRETER_ENTER_OR_FIND); - return this._enterOrBrowseInterpreterPath(input, state, interpreterSuggestions); + return this._enterOrBrowseInterpreterPath(input, state, suggestions); } else { sendTelemetryEvent(EventName.SELECT_INTERPRETER_SELECTED, undefined, { action: 'selected' }); state.path = (selection as IInterpreterQuickPickItem).path; @@ -177,7 +139,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { public async _enterOrBrowseInterpreterPath( input: IMultiStepInput, state: InterpreterStateArgs, - suggestions: IInterpreterQuickPickItem[], + suggestions: QuickPickType[], ): Promise> { const items: QuickPickItem[] = [ { @@ -236,6 +198,48 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } } + private async getItems(resource: Resource, sortList: boolean) { + const suggestions: QuickPickType[] = [this.manualEntrySuggestion]; + const defaultInterpreterPathSuggestion = this.getDefaultInterpreterPathSuggestion(resource); + if (defaultInterpreterPathSuggestion) { + suggestions.push(defaultInterpreterPathSuggestion); + } + let interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource, sortList); + if (sortList && interpreterSuggestions.length > 0) { + // If list is already sorted, the first item is the recommended one. + const suggested = interpreterSuggestions.shift(); + if (suggested) { + const starred = cloneDeep(suggested); + starred.label = `${Octicons.Star} ${starred.label}`; + starred.description = Common.recommended(); + interpreterSuggestions.unshift(starred); + } + } + return suggestions; + } + + private getActiveItem(resource: Resource, interpreterSuggestions: QuickPickType[]) { + const currentPythonPath = this.configurationService.getSettings(resource).pythonPath; + const activeInterpreter = interpreterSuggestions.filter((i) => i.path === currentPythonPath); + return activeInterpreter.length > 0 ? activeInterpreter[0] : interpreterSuggestions[0]; + } + + private getDefaultInterpreterPathSuggestion(resource: Resource): ISpecialQuickPickItem | undefined { + const config = this.workspaceService.getConfiguration('python', resource); + const defaultInterpreterPathValue = config.get('defaultInterpreterPath'); + if (defaultInterpreterPathValue && defaultInterpreterPathValue !== 'python') { + return { + label: `${Octicons.Gear} ${InterpreterQuickPickList.defaultInterpreterPath.label()}`, + detail: this.pathUtils.getDisplayName( + defaultInterpreterPathValue, + resource ? resource.fsPath : undefined, + ), + path: defaultInterpreterPathValue, + alwaysShow: true, + }; + } + } + /** * Check if the interpreter that was entered exists in the list of suggestions. * If it does, it means that it had already been discovered, @@ -247,7 +251,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { private async sendInterpreterEntryTelemetry( selection: string, workspace: Resource, - suggestions: IInterpreterQuickPickItem[], + suggestions: QuickPickType[], ): Promise { let interpreterPath = path.normalize(untildify(selection)); @@ -256,7 +260,10 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } const expandedPaths = suggestions.map((s) => { - const suggestionPath = s.interpreter.path; + const suggestionPath = s.path; + if (!suggestionPath) { + return undefined; + } let expandedPath = path.normalize(untildify(suggestionPath)); if (!path.isAbsolute(suggestionPath)) { diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index dafea2cd6ddf..155b4023bf3c 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -42,21 +42,21 @@ export class InterpreterSelector implements IInterpreterSelector { this.disposables.forEach((disposable) => disposable.dispose()); } - public async getSuggestions(resource: Resource, ignoreCache?: boolean): Promise { + public async getSuggestions(resource: Resource, sortSuggestions: boolean): Promise { const interpreters = await this.interpreterManager.getInterpreters(resource, { onSuggestion: true, - ignoreCache, }); + if (sortSuggestions) { + interpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer)); + } return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource))); } - public async getAllSuggestions(resource: Resource, ignoreCache?: boolean): Promise { + public async getAllSuggestions(resource: Resource): Promise { const interpreters = await this.interpreterManager.getAllInterpreters(resource, { onSuggestion: true, - ignoreCache, }); - interpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer)); return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource))); } diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 0b2f62865432..43cb4a55af5e 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -31,8 +31,8 @@ export type PythonEnvSuggestionChangedEvent = { export const IInterpreterSelector = Symbol('IInterpreterSelector'); export interface IInterpreterSelector extends Disposable { readonly onChanged: Event; - getAllSuggestions(resource: Resource, ignoreCache?: boolean): Promise; - getSuggestions(resource: Resource, ignoreCache?: boolean): Promise; + getAllSuggestions(resource: Resource): Promise; + getSuggestions(resource: Resource, sortSuggestions: boolean): Promise; } export interface IInterpreterQuickPickItem extends QuickPickItem { diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index 540325eae5c9..025cb7fc75b5 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -344,8 +344,17 @@ export class Conda { * Corresponds to "conda info --json". */ public async getInfo(): Promise { + return this.getInfoCached(this.command); + } + + /** + * Cache result for this particular command. + */ + @cache(30_000, true, 10_000) + // eslint-disable-next-line class-methods-use-this + private async getInfoCached(command: string): Promise { const disposables = new Set(); - const result = await exec(this.command, ['info', '--json'], {}, disposables); + const result = await exec(command, ['info', '--json'], {}, disposables); traceVerbose(`conda info --json: ${result.stdout}`); // Ensure the process we started is cleaned up. diff --git a/src/client/pythonEnvironments/common/environmentManagers/poetry.ts b/src/client/pythonEnvironments/common/environmentManagers/poetry.ts index eb52c5b653d5..cafa58835072 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/poetry.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/poetry.ts @@ -99,11 +99,11 @@ export class Poetry { /** * Creates a Poetry service corresponding to the corresponding "poetry" command. * - * @param _command - Command used to run poetry. This has the same meaning as the + * @param command - Command used to run poetry. This has the same meaning as the * first argument of spawn() - i.e. it can be a full path, or just a binary name. * @param cwd - The working directory to use as cwd when running poetry. */ - constructor(public readonly _command: string, private cwd: string) { + constructor(public readonly command: string, private cwd: string) { this.fixCwd(); } @@ -184,7 +184,7 @@ export class Poetry { */ @cache(30_000, true, 10_000) private async getEnvListCached(_cwd: string): Promise { - const result = await this.safeShellExecute(`${this._command} env list --full-path`); + const result = await this.safeShellExecute(`${this.command} env list --full-path`); if (!result) { return undefined; } @@ -220,7 +220,7 @@ export class Poetry { */ @cache(20_000, true, 10_000) private async getActiveEnvPathCached(_cwd: string): Promise { - const result = await this.safeShellExecute(`${this._command} env info -p`, true); + const result = await this.safeShellExecute(`${this.command} env info -p`, true); if (!result) { return undefined; } @@ -232,7 +232,7 @@ export class Poetry { * environments are created for the directory. Corresponds to "poetry config virtualenvs.path". Swallows errors if any. */ public async getVirtualenvsPathSetting(): Promise { - const result = await this.safeShellExecute(`${this._command} config virtualenvs.path`); + const result = await this.safeShellExecute(`${this.command} config virtualenvs.path`); if (!result) { return undefined; } diff --git a/src/test/pythonEnvironments/common/environmentManagers/poetry.unit.test.ts b/src/test/pythonEnvironments/common/environmentManagers/poetry.unit.test.ts index 355a1251d118..166b388a11c0 100644 --- a/src/test/pythonEnvironments/common/environmentManagers/poetry.unit.test.ts +++ b/src/test/pythonEnvironments/common/environmentManagers/poetry.unit.test.ts @@ -106,7 +106,7 @@ suite('Poetry binary is located correctly', async () => { const poetry = await Poetry.getPoetry(testPoetryDir); - expect(poetry?._command).to.equal(undefined); + expect(poetry?.command).to.equal(undefined); }); test('Return undefined if cwd contains pyproject.toml which does not contain a poetry section', async () => { @@ -117,7 +117,7 @@ suite('Poetry binary is located correctly', async () => { const poetry = await Poetry.getPoetry(project3); - expect(poetry?._command).to.equal(undefined); + expect(poetry?.command).to.equal(undefined); }); test('When user has specified a valid poetry path, use it', async () => { @@ -135,7 +135,7 @@ suite('Poetry binary is located correctly', async () => { const poetry = await Poetry.getPoetry(project1); - expect(poetry?._command).to.equal('poetryPath'); + expect(poetry?.command).to.equal('poetryPath'); }); test("When user hasn't specified a path, use poetry on PATH if available", async () => { @@ -153,7 +153,7 @@ suite('Poetry binary is located correctly', async () => { const poetry = await Poetry.getPoetry(project1); - expect(poetry?._command).to.equal('poetry'); + expect(poetry?.command).to.equal('poetry'); }); test('When poetry is not available on PATH, try using the default poetry location if valid', async () => { @@ -180,7 +180,7 @@ suite('Poetry binary is located correctly', async () => { const poetry = await Poetry.getPoetry(project1); - expect(poetry?._command).to.equal(defaultPoetry); + expect(poetry?.command).to.equal(defaultPoetry); }); test('Return undefined otherwise', async () => { @@ -191,6 +191,6 @@ suite('Poetry binary is located correctly', async () => { const poetry = await Poetry.getPoetry(project1); - expect(poetry?._command).to.equal(undefined); + expect(poetry?.command).to.equal(undefined); }); }); From 0374b7d8e1c72c57f0c74b157593502880da34e9 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 15 Sep 2021 10:24:39 -0700 Subject: [PATCH 07/19] Cleanup --- .../interpreterSelector.ts | 24 +++---------------- src/client/interpreter/configuration/types.ts | 6 ----- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index 155b4023bf3c..53403348d37e 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -4,39 +4,21 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { Disposable, EventEmitter, Uri, Event } from 'vscode'; +import { Disposable, Uri } from 'vscode'; import { IPathUtils, Resource } from '../../../common/types'; import { PythonEnvironment } from '../../../pythonEnvironments/info'; import { IInterpreterService } from '../../contracts'; -import { - IInterpreterComparer, - IInterpreterQuickPickItem, - IInterpreterSelector, - PythonEnvSuggestionChangedEvent, -} from '../types'; +import { IInterpreterComparer, IInterpreterQuickPickItem, IInterpreterSelector } from '../types'; @injectable() export class InterpreterSelector implements IInterpreterSelector { private disposables: Disposable[] = []; - private readonly changed = new EventEmitter(); - constructor( @inject(IInterpreterService) private readonly interpreterManager: IInterpreterService, @inject(IInterpreterComparer) private readonly envTypeComparer: IInterpreterComparer, @inject(IPathUtils) private readonly pathUtils: IPathUtils, - ) { - this.interpreterManager.onDidChangeInterpreters(async (event) => { - this.changed.fire({ - old: event.old ? await this.suggestionToQuickPickItem(event.old) : event.old, - update: event.update ? await this.suggestionToQuickPickItem(event.update) : event.update, - }); - }); - } - - public get onChanged(): Event { - return this.changed.event; - } + ) {} public dispose(): void { this.disposables.forEach((disposable) => disposable.dispose()); diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 43cb4a55af5e..f5e8a3184bc6 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -23,14 +23,8 @@ export interface IPythonPathUpdaterServiceManager { ): Promise; } -export type PythonEnvSuggestionChangedEvent = { - old?: IInterpreterQuickPickItem; - update?: IInterpreterQuickPickItem | undefined; -}; - export const IInterpreterSelector = Symbol('IInterpreterSelector'); export interface IInterpreterSelector extends Disposable { - readonly onChanged: Event; getAllSuggestions(resource: Resource): Promise; getSuggestions(resource: Resource, sortSuggestions: boolean): Promise; } From 0e5dece51f69fc253b6ebfeb632fd50501abd7d6 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 15 Sep 2021 16:16:04 -0700 Subject: [PATCH 08/19] Fix some bugs and tests --- src/client/common/utils/async.ts | 2 +- src/client/common/utils/multiStepInput.ts | 2 +- .../commands/setInterpreter.ts | 25 ++-- .../interpreterSelector.ts | 1 + src/client/interpreter/configuration/types.ts | 2 +- .../commands/setInterpreter.unit.test.ts | 110 ++++++++++++++---- .../interpreterSelector.unit.test.ts | 9 +- 7 files changed, 108 insertions(+), 43 deletions(-) diff --git a/src/client/common/utils/async.ts b/src/client/common/utils/async.ts index 788baf87a5e8..76f15ec511c4 100644 --- a/src/client/common/utils/async.ts +++ b/src/client/common/utils/async.ts @@ -77,7 +77,7 @@ class DeferredImpl implements Deferred { } // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types -export function createDeferred(scope: any = null): Deferred { +export function createDeferred(scope: any = null): Deferred { return new DeferredImpl(scope); } diff --git a/src/client/common/utils/multiStepInput.ts b/src/client/common/utils/multiStepInput.ts index 472a703830c2..2a2b25fbb0e6 100644 --- a/src/client/common/utils/multiStepInput.ts +++ b/src/client/common/utils/multiStepInput.ts @@ -119,7 +119,7 @@ export class MultiStepInput implements IMultiStepInput { acceptFilterBoxTextAsSelection, onChangeItem, keepScrollPosition, - sortByLabel + sortByLabel, }: P): Promise> { const disposables: Disposable[] = []; try { diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 446dd8451ad6..a3d4c9b4ad3c 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -69,7 +69,8 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { state: InterpreterStateArgs, ): Promise> { // If the list is refreshing, adding elements in the end is only - // way to preserve scroll position, so we don't need sorting. + // way to preserve scroll position. If we try to maintain a + // sorted list, that is not guaranteed, so disable sorting. const sortList = !this.interpreterService.refreshPromise; const suggestions = await this.getItems(state.workspace, sortList); state.path = undefined; @@ -98,14 +99,8 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { callback: async (_event, quickPick) => { if (this.interpreterService.refreshPromise) { quickPick.busy = true; - this.interpreterService.refreshPromise.then(async () => { - // TODO: Suggested a recommended interpreter now that refresh has finished? + this.interpreterService.refreshPromise.then(() => { quickPick.busy = false; - const interpreterSuggestions = await this.getItems(state.workspace, false); - if (quickPick.activeItems.length === 0) { - // Changing active items if one is already set is not a good idea as user might be using it. - quickPick.activeItems = [this.getActiveItem(state.workspace, interpreterSuggestions)]; - } }); } @@ -215,13 +210,21 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { interpreterSuggestions.unshift(starred); } } + suggestions.push(...interpreterSuggestions); return suggestions; } - private getActiveItem(resource: Resource, interpreterSuggestions: QuickPickType[]) { + private getActiveItem(resource: Resource, suggestions: QuickPickType[]) { const currentPythonPath = this.configurationService.getSettings(resource).pythonPath; - const activeInterpreter = interpreterSuggestions.filter((i) => i.path === currentPythonPath); - return activeInterpreter.length > 0 ? activeInterpreter[0] : interpreterSuggestions[0]; + const activeInterpreter = suggestions.filter((i) => i.path === currentPythonPath); + if (activeInterpreter.length > 0) { + return activeInterpreter[0]; + } + const firstInterpreterSuggestion = suggestions.find((s) => 'interpreter' in s && s.interpreter); + if (firstInterpreterSuggestion) { + return firstInterpreterSuggestion; + } + return suggestions[0]; } private getDefaultInterpreterPathSuggestion(resource: Resource): ISpecialQuickPickItem | undefined { diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index 53403348d37e..b8d8ecd6b0e8 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -39,6 +39,7 @@ export class InterpreterSelector implements IInterpreterSelector { const interpreters = await this.interpreterManager.getAllInterpreters(resource, { onSuggestion: true, }); + interpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer)); return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource))); } diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index f5e8a3184bc6..5fd43f1b05ac 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -1,4 +1,4 @@ -import { ConfigurationTarget, Disposable, QuickPickItem, Uri, Event } from 'vscode'; +import { ConfigurationTarget, Disposable, QuickPickItem, Uri } from 'vscode'; import { Resource } from '../../common/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index c0d2a109a8ec..b5b0b6f71cf0 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -33,12 +33,15 @@ import { EventName } from '../../../../client/telemetry/constants'; import * as Telemetry from '../../../../client/telemetry'; import { MockWorkspaceConfiguration } from '../../../mocks/mockWorkspaceConfig'; import { Octicons } from '../../../../client/common/constants'; +import { IInterpreterService } from '../../../../client/interpreter/contracts'; +import { createDeferred, sleep } from '../../../../client/common/utils/async'; +import { instance, mock, verify, when } from 'ts-mockito'; const untildify = require('untildify'); -type TelemetryEventType = { eventName: EventName; properties: Record }; +type TelemetryEventType = { eventName: EventName; properties: unknown }; -suite('Set Interpreter Command', () => { +suite('xSet Interpreter Command', () => { let workspace: TypeMoq.IMock; let interpreterSelector: TypeMoq.IMock; let appShell: TypeMoq.IMock; @@ -48,6 +51,7 @@ suite('Set Interpreter Command', () => { let pythonSettings: TypeMoq.IMock; let platformService: TypeMoq.IMock; let multiStepInputFactory: TypeMoq.IMock; + let interpreterService: IInterpreterService; const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; @@ -64,6 +68,8 @@ suite('Set Interpreter Command', () => { pythonSettings = TypeMoq.Mock.ofType(); workspace = TypeMoq.Mock.ofType(); + interpreterService = mock(); + when(interpreterService.refreshPromise).thenReturn(undefined); workspace.setup((w) => w.rootPath).returns(() => 'rootPath'); configurationService.setup((x) => x.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); @@ -78,6 +84,7 @@ suite('Set Interpreter Command', () => { platformService.object, interpreterSelector.object, workspace.object, + instance(interpreterService), ); }); @@ -126,18 +133,15 @@ suite('Set Interpreter Command', () => { _enterOrBrowseInterpreterPath.resolves(); sendTelemetryStub = sinon .stub(Telemetry, 'sendTelemetryEvent') - .callsFake((eventName: EventName, _, properties: Record) => { + .callsFake((eventName: EventName, _, properties: unknown) => { telemetryEvent = { eventName, properties, }; }); interpreterSelector - .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([item])); - interpreterSelector - .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny(), true)) - .returns(() => Promise.resolve([refreshedItem])); pythonSettings.setup((p) => p.pythonPath).returns(() => currentPythonPath); pythonSettings.setup((p) => p.defaultInterpreterPath).returns(() => defaultInterpreterPath); @@ -160,6 +164,7 @@ suite('Set Interpreter Command', () => { platformService.object, interpreterSelector.object, workspace.object, + instance(interpreterService), ); }); teardown(() => { @@ -187,13 +192,15 @@ suite('Set Interpreter Command', () => { recommended.label = `${Octicons.Star} ${item.label}`; recommended.description = Common.recommended(); const suggestions = [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, recommended]; - const expectedParameters = { + const expectedParameters: IQuickPickParameters = { placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentPythonPath), items: suggestions, activeItem: recommended, matchOnDetail: true, matchOnDescription: true, title: InterpreterQuickPickList.browsePath.openButtonLabel(), + sortByLabel: true, + keepScrollPosition: true, }; let actualParameters: IQuickPickParameters | undefined; multiStepInput @@ -201,8 +208,7 @@ suite('Set Interpreter Command', () => { .callback((options) => { actualParameters = options; }) - .returns(() => Promise.resolve((undefined as unknown) as QuickPickItem)) - .verifiable(TypeMoq.Times.once()); + .returns(() => Promise.resolve((undefined as unknown) as QuickPickItem)); await setInterpreterCommand._pickInterpreter(multiStepInput.object, state); @@ -210,21 +216,76 @@ suite('Set Interpreter Command', () => { const refreshButtonCallback = actualParameters!.customButtonSetup?.callback; expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set'); delete actualParameters!.customButtonSetup; + delete actualParameters!.onChangeItem; assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal'); + }); + + test('Ensure a refresh is triggered if refresh button is clicked', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + let actualParameters: IQuickPickParameters | undefined; + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + .callback((options) => { + actualParameters = options; + }) + .returns(() => Promise.resolve((undefined as unknown) as QuickPickItem)); + + await setInterpreterCommand._pickInterpreter(multiStepInput.object, state); + + expect(actualParameters).to.not.equal(undefined, 'Parameters not set'); + const refreshButtonCallback = actualParameters!.customButtonSetup?.callback; + expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set'); + + when(interpreterService.triggerRefresh()).thenResolve(); + await refreshButtonCallback!({} as any); // Invoke callback, meaning that the refresh button is clicked. + verify(interpreterService.triggerRefresh()).once(); + }); + + test('If an event to update quickpick is received, the quickpick is updated accordingly', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + let actualParameters: IQuickPickParameters | undefined; + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + .callback((options) => { + actualParameters = options; + }) + .returns(() => Promise.resolve((undefined as unknown) as QuickPickItem)); + + await setInterpreterCommand._pickInterpreter(multiStepInput.object, state); + + expect(actualParameters).to.not.equal(undefined, 'Parameters not set'); + const onChangedCallback = actualParameters!.onChangeItem?.callback; + expect(onChangedCallback).to.not.equal(undefined, 'Callback not set'); multiStepInput.verifyAll(); - const quickPick = { items: [] }; + const quickPick = { items: [], activeItems: [], busy: false }; + interpreterSelector.reset(); + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), false)) + .returns(() => Promise.resolve([refreshedItem])); + const refreshPromiseDeferred = createDeferred(); + // Assume a refresh is currently going on... + when(interpreterService.refreshPromise).thenReturn(refreshPromiseDeferred.promise); + // eslint-disable-next-line @typescript-eslint/no-explicit-any - await refreshButtonCallback!(quickPick as any); // Invoke callback, meaning that the refresh button is clicked. + await onChangedCallback!({} as any, quickPick as any); // Invoke callback, meaning that the items are supposed to change. - const recommendedRefreshedItem = cloneDeep(refreshedItem); - recommendedRefreshedItem.label = `${Octicons.Star} ${refreshedItem.label}`; - recommendedRefreshedItem.description = Common.recommended(); assert.deepStrictEqual( - quickPick.items, - [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, recommendedRefreshedItem], + quickPick, + { + items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, refreshedItem], + activeItems: [refreshedItem], + busy: true, + }, 'Quickpick not updated correctly', ); + + refreshPromiseDeferred.resolve(); + await sleep(1); + // Refresh finishes, so quickpick busy indicator should go away + assert.deepStrictEqual(quickPick.busy, false, 'Quickpick status not updated to ideal'); }); test('If an item is selected, update state and return', async () => { @@ -383,7 +444,7 @@ suite('Set Interpreter Command', () => { setup(() => { sendTelemetryStub = sinon .stub(Telemetry, 'sendTelemetryEvent') - .callsFake((eventName: EventName, _, properties: Record) => { + .callsFake((eventName: EventName, _, properties: unknown) => { telemetryEvents.push({ eventName, properties, @@ -522,7 +583,7 @@ suite('Set Interpreter Command', () => { workspace.setup((w) => w.workspaceFolders).returns(() => undefined); interpreterSelector - .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { @@ -564,7 +625,7 @@ suite('Set Interpreter Command', () => { workspace.setup((w) => w.workspaceFolders).returns(() => [folder]); interpreterSelector - .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([])); const multiStepInput = { @@ -622,7 +683,7 @@ suite('Set Interpreter Command', () => { ]; interpreterSelector - .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([])); const multiStepInput = { @@ -690,7 +751,7 @@ suite('Set Interpreter Command', () => { ]; interpreterSelector - .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([selectedItem])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { @@ -730,7 +791,7 @@ suite('Set Interpreter Command', () => { workspace.setup((w) => w.workspaceFolders).returns(() => [folder1, folder2]); interpreterSelector - .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([])); multiStepInputFactory.setup((f) => f.create()).verifiable(TypeMoq.Times.never()); @@ -781,6 +842,7 @@ suite('Set Interpreter Command', () => { platformService.object, interpreterSelector.object, workspace.object, + instance(interpreterService), ); type InputStepType = () => Promise | void>; let inputStep!: InputStepType; @@ -797,7 +859,7 @@ suite('Set Interpreter Command', () => { workspace.setup((w) => w.workspaceFolders).returns(() => undefined); interpreterSelector - .setup((i) => i.getAllSuggestions(TypeMoq.It.isAny())) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve([])); const multiStepInput = { run: (inputStepArg: InputStepType, state: InterpreterStateArgs) => { diff --git a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts index ca53272e2a95..d322864bf168 100644 --- a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts @@ -50,7 +50,6 @@ suite('Interpreters - selector', () => { let interpreterService: TypeMoq.IMock; let fileSystem: TypeMoq.IMock; let newComparer: TypeMoq.IMock; - const ignoreCache = false; class TestInterpreterSelector extends InterpreterSelector { public async suggestionToQuickPickItem( suggestion: PythonEnvironment, @@ -91,10 +90,10 @@ suite('Interpreters - selector', () => { { displayName: '4', path: 'c:/path4/path4', envType: EnvironmentType.Conda }, ].map((item) => ({ ...info, ...item })); interpreterService - .setup((x) => x.getAllInterpreters(TypeMoq.It.isAny(), { onSuggestion: true, ignoreCache })) + .setup((x) => x.getAllInterpreters(TypeMoq.It.isAny(), { onSuggestion: true })) .returns(() => new Promise((resolve) => resolve(initial))); - const actual = await selector.getAllSuggestions(undefined, ignoreCache); + const actual = await selector.getAllSuggestions(undefined); const expected: InterpreterQuickPickItem[] = [ new InterpreterQuickPickItem('1', 'c:/path1/path1'), @@ -153,7 +152,7 @@ suite('Interpreters - selector', () => { ].map((item) => ({ ...info, ...item })); interpreterService - .setup((x) => x.getAllInterpreters(TypeMoq.It.isAny(), { onSuggestion: true, ignoreCache })) + .setup((x) => x.getAllInterpreters(TypeMoq.It.isAny(), { onSuggestion: true })) .returns(() => new Promise((resolve) => resolve(environments))); const interpreterHelper = TypeMoq.Mock.ofType(); @@ -169,7 +168,7 @@ suite('Interpreters - selector', () => { new PathUtils(getOSType() === OSType.Windows), ); - const result = await selector.getAllSuggestions(undefined, ignoreCache); + const result = await selector.getAllSuggestions(undefined); const expected: InterpreterQuickPickItem[] = [ new InterpreterQuickPickItem('two', path.join(workspacePath, '.venv', 'bin', 'python')), From d2ded4573d6e1039c7dc7ed81fd23e53e64a96bc Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 15 Sep 2021 16:28:18 -0700 Subject: [PATCH 09/19] Fix more tests and bugs --- .../interpreterSelector/commands/setInterpreter.ts | 5 +---- .../interpreterSelector/commands/setInterpreter.unit.test.ts | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index a3d4c9b4ad3c..66c9b88c32f8 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -263,10 +263,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } const expandedPaths = suggestions.map((s) => { - const suggestionPath = s.path; - if (!suggestionPath) { - return undefined; - } + const suggestionPath = 'interpreter' in s ? s.interpreter.path : ''; let expandedPath = path.normalize(untildify(suggestionPath)); if (!path.isAbsolute(suggestionPath)) { diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index b5b0b6f71cf0..cd1c87e6ca04 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -41,7 +41,7 @@ const untildify = require('untildify'); type TelemetryEventType = { eventName: EventName; properties: unknown }; -suite('xSet Interpreter Command', () => { +suite('Set Interpreter Command', () => { let workspace: TypeMoq.IMock; let interpreterSelector: TypeMoq.IMock; let appShell: TypeMoq.IMock; From c2a40ca3e7561a2361003a7044eb8a78625bb40b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 15 Sep 2021 16:32:07 -0700 Subject: [PATCH 10/19] Fix linting --- .../interpreterSelector/commands/setInterpreter.ts | 4 +++- .../interpreterSelector/commands/setInterpreter.unit.test.ts | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 66c9b88c32f8..be8af508c662 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -42,6 +42,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { label: `${Octicons.Add} ${InterpreterQuickPickList.enterPath.label()}`, alwaysShow: true, }; + constructor( @inject(IApplicationShell) applicationShell: IApplicationShell, @inject(IPathUtils) private readonly pathUtils: IPathUtils, @@ -199,7 +200,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if (defaultInterpreterPathSuggestion) { suggestions.push(defaultInterpreterPathSuggestion); } - let interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource, sortList); + const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource, sortList); if (sortList && interpreterSuggestions.length > 0) { // If list is already sorted, the first item is the recommended one. const suggested = interpreterSuggestions.shift(); @@ -241,6 +242,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { alwaysShow: true, }; } + return undefined; } /** diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index cd1c87e6ca04..57a14d2f47a0 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -8,6 +8,7 @@ import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; import { ConfigurationTarget, OpenDialogOptions, QuickPickItem, Uri } from 'vscode'; import { cloneDeep } from 'lodash'; +import { instance, mock, verify, when } from 'ts-mockito'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../client/common/application/types'; import { PathUtils } from '../../../../client/common/platform/pathUtils'; import { IPlatformService } from '../../../../client/common/platform/types'; @@ -35,7 +36,6 @@ import { MockWorkspaceConfiguration } from '../../../mocks/mockWorkspaceConfig'; import { Octicons } from '../../../../client/common/constants'; import { IInterpreterService } from '../../../../client/interpreter/contracts'; import { createDeferred, sleep } from '../../../../client/common/utils/async'; -import { instance, mock, verify, when } from 'ts-mockito'; const untildify = require('untildify'); @@ -238,6 +238,7 @@ suite('Set Interpreter Command', () => { expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set'); when(interpreterService.triggerRefresh()).thenResolve(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any await refreshButtonCallback!({} as any); // Invoke callback, meaning that the refresh button is clicked. verify(interpreterService.triggerRefresh()).once(); }); From 91510ee4067bc68fdfbad55d74b462c47ce9e74a Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 15 Sep 2021 16:42:58 -0700 Subject: [PATCH 11/19] Move private methods to where they are used --- .../commands/setInterpreter.ts | 102 +++++++++--------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index be8af508c662..d51d67bcc9d6 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -131,6 +131,57 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { return undefined; } + private async getItems(resource: Resource, sortList: boolean) { + const suggestions: QuickPickType[] = [this.manualEntrySuggestion]; + const defaultInterpreterPathSuggestion = this.getDefaultInterpreterPathSuggestion(resource); + if (defaultInterpreterPathSuggestion) { + suggestions.push(defaultInterpreterPathSuggestion); + } + const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource, sortList); + if (sortList && interpreterSuggestions.length > 0) { + // If list is already sorted, the first item is the recommended one. + const suggested = interpreterSuggestions.shift(); + if (suggested) { + const starred = cloneDeep(suggested); + starred.label = `${Octicons.Star} ${starred.label}`; + starred.description = Common.recommended(); + interpreterSuggestions.unshift(starred); + } + } + suggestions.push(...interpreterSuggestions); + return suggestions; + } + + private getActiveItem(resource: Resource, suggestions: QuickPickType[]) { + const currentPythonPath = this.configurationService.getSettings(resource).pythonPath; + const activeInterpreter = suggestions.filter((i) => i.path === currentPythonPath); + if (activeInterpreter.length > 0) { + return activeInterpreter[0]; + } + const firstInterpreterSuggestion = suggestions.find((s) => 'interpreter' in s && s.interpreter); + if (firstInterpreterSuggestion) { + return firstInterpreterSuggestion; + } + return suggestions[0]; + } + + private getDefaultInterpreterPathSuggestion(resource: Resource): ISpecialQuickPickItem | undefined { + const config = this.workspaceService.getConfiguration('python', resource); + const defaultInterpreterPathValue = config.get('defaultInterpreterPath'); + if (defaultInterpreterPathValue && defaultInterpreterPathValue !== 'python') { + return { + label: `${Octicons.Gear} ${InterpreterQuickPickList.defaultInterpreterPath.label()}`, + detail: this.pathUtils.getDisplayName( + defaultInterpreterPathValue, + resource ? resource.fsPath : undefined, + ), + path: defaultInterpreterPathValue, + alwaysShow: true, + }; + } + return undefined; + } + @captureTelemetry(EventName.SELECT_INTERPRETER_ENTER_BUTTON) public async _enterOrBrowseInterpreterPath( input: IMultiStepInput, @@ -194,57 +245,6 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } } - private async getItems(resource: Resource, sortList: boolean) { - const suggestions: QuickPickType[] = [this.manualEntrySuggestion]; - const defaultInterpreterPathSuggestion = this.getDefaultInterpreterPathSuggestion(resource); - if (defaultInterpreterPathSuggestion) { - suggestions.push(defaultInterpreterPathSuggestion); - } - const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource, sortList); - if (sortList && interpreterSuggestions.length > 0) { - // If list is already sorted, the first item is the recommended one. - const suggested = interpreterSuggestions.shift(); - if (suggested) { - const starred = cloneDeep(suggested); - starred.label = `${Octicons.Star} ${starred.label}`; - starred.description = Common.recommended(); - interpreterSuggestions.unshift(starred); - } - } - suggestions.push(...interpreterSuggestions); - return suggestions; - } - - private getActiveItem(resource: Resource, suggestions: QuickPickType[]) { - const currentPythonPath = this.configurationService.getSettings(resource).pythonPath; - const activeInterpreter = suggestions.filter((i) => i.path === currentPythonPath); - if (activeInterpreter.length > 0) { - return activeInterpreter[0]; - } - const firstInterpreterSuggestion = suggestions.find((s) => 'interpreter' in s && s.interpreter); - if (firstInterpreterSuggestion) { - return firstInterpreterSuggestion; - } - return suggestions[0]; - } - - private getDefaultInterpreterPathSuggestion(resource: Resource): ISpecialQuickPickItem | undefined { - const config = this.workspaceService.getConfiguration('python', resource); - const defaultInterpreterPathValue = config.get('defaultInterpreterPath'); - if (defaultInterpreterPathValue && defaultInterpreterPathValue !== 'python') { - return { - label: `${Octicons.Gear} ${InterpreterQuickPickList.defaultInterpreterPath.label()}`, - detail: this.pathUtils.getDisplayName( - defaultInterpreterPathValue, - resource ? resource.fsPath : undefined, - ), - path: defaultInterpreterPathValue, - alwaysShow: true, - }; - } - return undefined; - } - /** * Check if the interpreter that was entered exists in the list of suggestions. * If it does, it means that it had already been discovered, From 0269eec9f46bfdd8de368786e81e106aec95074b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 16 Sep 2021 15:56:24 -0700 Subject: [PATCH 12/19] Ensure we set partial display names --- .vscode/launch.json | 2 +- package.nls.json | 2 +- package.nls.nl.json | 1 - package.nls.zh-cn.json | 1 - package.nls.zh-tw.json | 1 - src/client/common/utils/localize.ts | 2 +- .../commands/setInterpreter.ts | 90 +++++++++++++------ .../interpreterSelector.ts | 13 +-- src/client/interpreter/configuration/types.ts | 3 +- .../interpreter/display/progressDisplay.ts | 4 +- .../pythonEnvironments/base/info/env.ts | 5 +- .../locators/composite/envsCollectionCache.ts | 8 +- .../base/locators/composite/resolverUtils.ts | 4 +- .../commands/setInterpreter.unit.test.ts | 56 ++++++------ .../interpreterSelector.unit.test.ts | 5 +- .../display/progressDisplay.unit.test.ts | 8 +- .../base/info/env.unit.test.ts | 84 +++++++---------- .../composite/resolverUtils.unit.test.ts | 25 ++++-- 18 files changed, 163 insertions(+), 151 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index c2fc063109b2..107f75b919d8 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -158,7 +158,7 @@ "--ui=tdd", "--recursive", "--colors", - //"--grep", "", + "--grep", "xpyenvs", "--timeout=300000" ], "outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"], diff --git a/package.nls.json b/package.nls.json index edb7e74d2dce..d3c178e624ac 100644 --- a/package.nls.json +++ b/package.nls.json @@ -52,7 +52,7 @@ "Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters", "Interpreters.entireWorkspace": "Entire workspace", "Interpreters.pythonInterpreterPath": "Python interpreter path: {0}", - "Interpreters.LoadingInterpreters": "Loading Python Interpreters", + "Interpreters.DiscoveringInterpreters": "Discovering Python Interpreters", "Interpreters.condaInheritEnvMessage": "We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change \"terminal.integrated.inheritEnv\" to false in your user settings.", "Logging.CurrentWorkingDirectory": "cwd:", "InterpreterQuickPickList.quickPickListPlaceholder": "Current: {0}", diff --git a/package.nls.nl.json b/package.nls.nl.json index 6463e22b4644..8a28c7d1a816 100644 --- a/package.nls.nl.json +++ b/package.nls.nl.json @@ -26,7 +26,6 @@ "LanguageService.lsFailedToDownload": "We zijn een probleem tegengekomen bij het downloaden van de language server. Aan het terugschakelen naar het alternatief, Jedi. Bekijk het weergavepaneel voor details.", "LanguageService.lsFailedToExtract": "We zijn een probleem tegengekomen bij het uitpakken van de language server. Aan het terugschakelen naar het alternatief, Jedi. Bekijk het weergavepaneel voor details.", "Interpreters.RefreshingInterpreters": "Python-Interpreters verversen", - "Interpreters.LoadingInterpreters": "Python-Interpreters laden", "Linter.InstalledButNotEnabled": "Linter {0} is geinstalleerd maar niet ingeschakeld.", "Linter.replaceWithSelectedLinter": "Meerdere linters zijn ingeschakeld in de instellingen. Vervangen met '{0}'?", "diagnostics.warnSourceMaps": "Bronkaartondersteuning is ingeschakeld in de Python-extensie, dit zal een ongunstige impact hebben op de uitvoering van de extensie.", diff --git a/package.nls.zh-cn.json b/package.nls.zh-cn.json index 1d2864a2469e..e5a143452f92 100644 --- a/package.nls.zh-cn.json +++ b/package.nls.zh-cn.json @@ -51,7 +51,6 @@ "Interpreters.RefreshingInterpreters": "正在刷新 Python 解释器", "Interpreters.entireWorkspace": "完整工作区", "Interpreters.pythonInterpreterPath": "Python 解释器路径: {0}", - "Interpreters.LoadingInterpreters": "正在加载 Python 解释器", "Interpreters.condaInheritEnvMessage": "您正在使用 conda 环境，如果您在集成终端中遇到相关问题，建议您允许 Python 扩展将用户设置中的 \"terminal.integrated.inheritEnv\" 改为 false。", "Logging.CurrentWorkingDirectory": "cwd:", "InterpreterQuickPickList.quickPickListPlaceholder": "当前: {0}", diff --git a/package.nls.zh-tw.json b/package.nls.zh-tw.json index 03529ff4a997..936edb6fdff2 100644 --- a/package.nls.zh-tw.json +++ b/package.nls.zh-tw.json @@ -34,7 +34,6 @@ "LanguageService.lsFailedToExtract": "擷取語言伺服器時遇到問題。改回使用替代方案 \"Jedi\"。請檢查 Python 輸出面板以取得更多資訊。", "Experiments.inGroup": "使用者屬於 \"{0}\" 實驗性群組", "Interpreters.RefreshingInterpreters": "正在重新整理 Python 解譯器", - "Interpreters.LoadingInterpreters": "正在載入 Python 解譯器", "Interpreters.condaInheritEnvMessage": "我們發覺到您在使用 conda 環境。如果你在整合式終端器中使用這個環境時遇到問題，建議您讓 Python 延伸模組變更使用者設定中的 \"terminal.integrated.inheritEnv\" 為 false。", "Logging.CurrentWorkingDirectory": "cwd:", "Common.doNotShowAgain": "不再顯示", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index e600eaee0c22..37d2e0408b6a 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -261,7 +261,7 @@ export namespace Experiments { export const optedOutOf = localize('Experiments.optedOutOf', "User opted out of experiment group '{0}'"); } export namespace Interpreters { - export const loading = localize('Interpreters.LoadingInterpreters', 'Loading Python Interpreters'); + export const discovering = localize('Interpreters.DiscoveringInterpreters', 'Discovering Python Interpreters'); export const refreshing = localize('Interpreters.RefreshingInterpreters', 'Refreshing Python Interpreters'); export const condaInheritEnvMessage = localize( 'Interpreters.condaInheritEnvMessage', diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index d51d67bcc9d6..80ff4a2b8619 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -4,11 +4,11 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { cloneDeep } from 'lodash'; import * as path from 'path'; import { QuickPickItem } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types'; import { Commands, Octicons } from '../../../../common/constants'; +import { arePathsSame } from '../../../../common/platform/fs-paths'; import { IPlatformService } from '../../../../common/platform/types'; import { IConfigurationService, IPathUtils, Resource } from '../../../../common/types'; import { getIcon } from '../../../../common/utils/icons'; @@ -22,7 +22,7 @@ import { import { REFRESH_BUTTON_ICON } from '../../../../debugger/extension/attachQuickPick/types'; import { captureTelemetry, sendTelemetryEvent } from '../../../../telemetry'; import { EventName } from '../../../../telemetry/constants'; -import { IInterpreterService } from '../../../contracts'; +import { IInterpreterService, PythonEnvironmentsChangedEvent } from '../../../contracts'; import { IInterpreterQuickPickItem, IInterpreterSelector, @@ -69,11 +69,10 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { input: IMultiStepInput, state: InterpreterStateArgs, ): Promise> { - // If the list is refreshing, adding elements in the end is only - // way to preserve scroll position. If we try to maintain a - // sorted list, that is not guaranteed, so disable sorting. - const sortList = !this.interpreterService.refreshPromise; - const suggestions = await this.getItems(state.workspace, sortList); + // If the list is refreshing, it's crucial to maintain sorting order at all + // times, so the visible items do not change. + const preserveOrderWhenFiltering = !!this.interpreterService.refreshPromise; + const suggestions = await this.getItems(state.workspace); state.path = undefined; const currentInterpreterPathDisplay = this.pathUtils.getDisplayName( this.configurationService.getSettings(state.workspace).pythonPath, @@ -82,7 +81,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { const selection = await input.showQuickPick>({ placeholder: InterpreterQuickPickList.quickPickListPlaceholder().format(currentInterpreterPathDisplay), items: suggestions, - sortByLabel: sortList, + sortByLabel: !preserveOrderWhenFiltering, keepScrollPosition: true, activeItem: this.getActiveItem(state.workspace, suggestions), matchOnDetail: true, @@ -97,23 +96,27 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { }, onChangeItem: { event: this.interpreterService.onDidChangeInterpreters, - callback: async (_event, quickPick) => { + callback: async (event: PythonEnvironmentsChangedEvent, quickPick) => { if (this.interpreterService.refreshPromise) { quickPick.busy = true; this.interpreterService.refreshPromise.then(() => { quickPick.busy = false; }); } - - const interpreterSuggestions = await this.getItems( - state.workspace, - !this.interpreterService.refreshPromise, - ); - quickPick.items = interpreterSuggestions; - if (quickPick.activeItems.length === 0) { - // Changing active items if one is already set is not a good idea as user might be using it. - quickPick.activeItems = [this.getActiveItem(state.workspace, interpreterSuggestions)]; - } + // Active items are reset once we replace the current list with updated items, so save it. + const activeItemBeforeUpdate = + quickPick.activeItems.length > 0 ? quickPick.activeItems[0] : undefined; + quickPick.items = this.getUpdatedItems(quickPick.items, event, state.workspace); + // Ensure we maintain the same active item as before. + const activeItem = activeItemBeforeUpdate + ? quickPick.items.find((item) => { + if ('interpreter' in item && 'interpreter' in activeItemBeforeUpdate) { + return arePathsSame(item.interpreter.path, activeItemBeforeUpdate.interpreter.path); + } + return false; + }) + : undefined; + quickPick.activeItems = activeItem ? [activeItem] : []; }, }, }); @@ -131,21 +134,19 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { return undefined; } - private async getItems(resource: Resource, sortList: boolean) { + private async getItems(resource: Resource) { const suggestions: QuickPickType[] = [this.manualEntrySuggestion]; const defaultInterpreterPathSuggestion = this.getDefaultInterpreterPathSuggestion(resource); if (defaultInterpreterPathSuggestion) { suggestions.push(defaultInterpreterPathSuggestion); } - const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource, sortList); - if (sortList && interpreterSuggestions.length > 0) { - // If list is already sorted, the first item is the recommended one. - const suggested = interpreterSuggestions.shift(); + const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource); + if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { + // If list is not refreshing, the first item is the recommended one. + const suggested = interpreterSuggestions[0]; if (suggested) { - const starred = cloneDeep(suggested); - starred.label = `${Octicons.Star} ${starred.label}`; - starred.description = Common.recommended(); - interpreterSuggestions.unshift(starred); + suggested.label = `${Octicons.Star} ${suggested.label}`; + suggested.description = Common.recommended(); } } suggestions.push(...interpreterSuggestions); @@ -182,6 +183,39 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { return undefined; } + private getUpdatedItems( + items: readonly QuickPickType[], + event: PythonEnvironmentsChangedEvent, + resource: Resource, + ): QuickPickType[] { + const updatedItems = [...items.values()]; + const env = event.old ?? event.update; + let envIndex = -1; + if (env) { + envIndex = updatedItems.findIndex((item) => { + if ('interpreter' in item) { + return arePathsSame(item.interpreter.path, env.path); + } + return false; + }); + } + if (event.update) { + const newSuggestion: QuickPickType = this.interpreterSelector.suggestionToQuickPickItem( + event.update, + resource, + ); + if (envIndex === -1) { + updatedItems.push(newSuggestion); + } else { + updatedItems[envIndex] = newSuggestion; + } + } + if (envIndex !== -1 && event.update === undefined) { + updatedItems.splice(envIndex, 1); + } + return updatedItems; + } + @captureTelemetry(EventName.SELECT_INTERPRETER_ENTER_BUTTON) public async _enterOrBrowseInterpreterPath( input: IMultiStepInput, diff --git a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts index b8d8ecd6b0e8..08a3c3ee3def 100644 --- a/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts +++ b/src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts @@ -24,13 +24,11 @@ export class InterpreterSelector implements IInterpreterSelector { this.disposables.forEach((disposable) => disposable.dispose()); } - public async getSuggestions(resource: Resource, sortSuggestions: boolean): Promise { + public async getSuggestions(resource: Resource): Promise { const interpreters = await this.interpreterManager.getInterpreters(resource, { onSuggestion: true, }); - if (sortSuggestions) { - interpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer)); - } + interpreters.sort(this.envTypeComparer.compare.bind(this.envTypeComparer)); return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource))); } @@ -44,14 +42,11 @@ export class InterpreterSelector implements IInterpreterSelector { return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource))); } - protected async suggestionToQuickPickItem( - suggestion: PythonEnvironment, - workspaceUri?: Uri, - ): Promise { + public suggestionToQuickPickItem(suggestion: PythonEnvironment, workspaceUri?: Uri): IInterpreterQuickPickItem { const detail = this.pathUtils.getDisplayName(suggestion.path, workspaceUri ? workspaceUri.fsPath : undefined); const cachedPrefix = suggestion.cachedEntry ? '(cached) ' : ''; return { - label: suggestion.displayName!, + label: suggestion.displayName || 'Python', detail: `${cachedPrefix}${detail}`, path: suggestion.path, interpreter: suggestion, diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 5fd43f1b05ac..001fd3d545f8 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -26,7 +26,8 @@ export interface IPythonPathUpdaterServiceManager { export const IInterpreterSelector = Symbol('IInterpreterSelector'); export interface IInterpreterSelector extends Disposable { getAllSuggestions(resource: Resource): Promise; - getSuggestions(resource: Resource, sortSuggestions: boolean): Promise; + getSuggestions(resource: Resource): Promise; + suggestionToQuickPickItem(suggestion: PythonEnvironment, workspaceUri?: Uri | undefined): IInterpreterQuickPickItem; } export interface IInterpreterQuickPickItem extends QuickPickItem { diff --git a/src/client/interpreter/display/progressDisplay.ts b/src/client/interpreter/display/progressDisplay.ts index ef1c374491e3..862434f13404 100644 --- a/src/client/interpreter/display/progressDisplay.ts +++ b/src/client/interpreter/display/progressDisplay.ts @@ -11,7 +11,7 @@ import { inDiscoveryExperiment } from '../../common/experiments/helpers'; import { traceDecorators } from '../../common/logger'; import { IDisposableRegistry, IExperimentService } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; -import { Common, Interpreters } from '../../common/utils/localize'; +import { Interpreters } from '../../common/utils/localize'; import { IServiceContainer } from '../../ioc/types'; import { IComponentAdapter, IInterpreterLocatorProgressService } from '../contracts'; @@ -70,7 +70,7 @@ export class InterpreterLocatorProgressStatubarHandler implements IExtensionSing private createProgress() { const progressOptions: ProgressOptions = { location: ProgressLocation.Window, - title: this.isFirstTimeLoadingInterpreters ? Common.loadingExtension() : Interpreters.refreshing(), + title: this.isFirstTimeLoadingInterpreters ? Interpreters.discovering() : Interpreters.refreshing(), }; this.isFirstTimeLoadingInterpreters = false; this.shell.withProgress(progressOptions, () => { diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index 2810c92b276f..a082742a1713 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -113,10 +113,7 @@ function updateEnv( * E.g. `Python 3.5.1 32-bit (myenv2: virtualenv)` */ export function getEnvDisplayString(env: PythonEnvInfo): string { - if (env.display === undefined || env.display === '') { - env.display = buildEnvDisplayString(env); - } - return env.display; + return buildEnvDisplayString(env); } function buildEnvDisplayString(env: PythonEnvInfo): string { diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts index c6d116792ea1..cddbcfa477cc 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts @@ -3,7 +3,6 @@ import { Event } from 'vscode'; import { traceInfo } from '../../../../common/logger'; -import { asyncFilter } from '../../../../common/utils/arrayUtils'; import { pathExists } from '../../../common/externalDependencies'; import { PythonEnvInfo } from '../../info'; import { areSameEnv } from '../../info/env'; @@ -76,7 +75,12 @@ export class PythonEnvInfoCache extends PythonEnvsWatcher pathExists(e.executable.filename)); + const areEnvsValid = await Promise.all(this.envs.map((e) => pathExists(e.executable.filename))); + 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 }); + }); } public getAllEnvs(): PythonEnvInfo[] { diff --git a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts index 9251404f4e54..0ecd54b26591 100644 --- a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts +++ b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts @@ -6,7 +6,7 @@ import { Uri } from 'vscode'; import { uniq } from 'lodash'; import { traceError, traceWarning } from '../../../../common/logger'; import { PythonEnvInfo, PythonEnvKind, PythonEnvSource, UNKNOWN_PYTHON_VERSION, virtualEnvKinds } from '../../info'; -import { buildEnvInfo, comparePythonVersionSpecificity, getEnvMatcher } from '../../info/env'; +import { buildEnvInfo, comparePythonVersionSpecificity, getEnvDisplayString, getEnvMatcher } from '../../info/env'; import { getEnvironmentDirFromPath, getInterpreterPathFromDir, @@ -52,7 +52,7 @@ export async function resolveBasicEnv({ kind, executablePath, source }: BasicEnv // We can update env further using information we can get from the Windows registry. await updateEnvUsingRegistry(resolvedEnv); } - // Display name is not set here as we need version, arch etc. to build it. + resolvedEnv.display = getEnvDisplayString(resolvedEnv); return resolvedEnv; } diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 57a14d2f47a0..29125b7d95ad 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -34,7 +34,7 @@ import { EventName } from '../../../../client/telemetry/constants'; import * as Telemetry from '../../../../client/telemetry'; import { MockWorkspaceConfiguration } from '../../../mocks/mockWorkspaceConfig'; import { Octicons } from '../../../../client/common/constants'; -import { IInterpreterService } from '../../../../client/interpreter/contracts'; +import { IInterpreterService, PythonEnvironmentsChangedEvent } from '../../../../client/interpreter/contracts'; import { createDeferred, sleep } from '../../../../client/common/utils/async'; const untildify = require('untildify'); @@ -97,12 +97,13 @@ suite('Set Interpreter Command', () => { let sendTelemetryStub: sinon.SinonStub; let telemetryEvent: TelemetryEventType | undefined; + const interpreterPath = 'path/to/interpreter'; const item: IInterpreterQuickPickItem = { description: '', detail: '', - label: '', - path: 'This is the selected Python path', - interpreter: {} as PythonEnvironment, + label: 'This is the selected Python path', + path: interpreterPath, + interpreter: { path: interpreterPath } as PythonEnvironment, }; const defaultInterpreterPath = 'defaultInterpreterPath'; const defaultInterpreterPathSuggestion = { @@ -115,9 +116,9 @@ suite('Set Interpreter Command', () => { const refreshedItem: IInterpreterQuickPickItem = { description: '', detail: '', - label: '', - path: 'Refreshed path', - interpreter: {} as PythonEnvironment, + label: 'Refreshed path', + path: interpreterPath, + interpreter: { path: interpreterPath } as PythonEnvironment, }; const expectedEnterInterpreterPathSuggestion = { label: `${Octicons.Add} ${InterpreterQuickPickList.enterPath.label()}`, @@ -140,7 +141,7 @@ suite('Set Interpreter Command', () => { }; }); interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny())) .returns(() => Promise.resolve([item])); pythonSettings.setup((p) => p.pythonPath).returns(() => currentPythonPath); pythonSettings.setup((p) => p.defaultInterpreterPath).returns(() => defaultInterpreterPath); @@ -261,17 +262,24 @@ suite('Set Interpreter Command', () => { expect(onChangedCallback).to.not.equal(undefined, 'Callback not set'); multiStepInput.verifyAll(); - const quickPick = { items: [], activeItems: [], busy: false }; - interpreterSelector.reset(); + const quickPick = { + items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, refreshedItem], + activeItems: [item], + busy: false, + }; interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), false)) - .returns(() => Promise.resolve([refreshedItem])); + .setup((i) => i.suggestionToQuickPickItem(TypeMoq.It.isAny(), undefined)) + .returns(() => refreshedItem); const refreshPromiseDeferred = createDeferred(); // Assume a refresh is currently going on... when(interpreterService.refreshPromise).thenReturn(refreshPromiseDeferred.promise); + const changeEvent: PythonEnvironmentsChangedEvent = { + old: item.interpreter, + update: refreshedItem.interpreter, + }; // eslint-disable-next-line @typescript-eslint/no-explicit-any - await onChangedCallback!({} as any, quickPick as any); // Invoke callback, meaning that the items are supposed to change. + await onChangedCallback!(changeEvent, quickPick as any); // Invoke callback, meaning that the items are supposed to change. assert.deepStrictEqual( quickPick, @@ -583,9 +591,7 @@ suite('Set Interpreter Command', () => { workspace.setup((w) => w.workspaceFolders).returns(() => undefined); - interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) - .returns(() => Promise.resolve([])); + interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { state.path = selectedItem.path; @@ -625,9 +631,7 @@ suite('Set Interpreter Command', () => { const folder = { name: 'one', uri: Uri.parse('one'), index: 0 }; workspace.setup((w) => w.workspaceFolders).returns(() => [folder]); - interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) - .returns(() => Promise.resolve([])); + interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { @@ -683,9 +687,7 @@ suite('Set Interpreter Command', () => { }, ]; - interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) - .returns(() => Promise.resolve([])); + interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { @@ -752,7 +754,7 @@ suite('Set Interpreter Command', () => { ]; interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) + .setup((i) => i.getSuggestions(TypeMoq.It.isAny())) .returns(() => Promise.resolve([selectedItem])); const multiStepInput = { run: (_: unknown, state: InterpreterStateArgs) => { @@ -791,9 +793,7 @@ suite('Set Interpreter Command', () => { test('Do not update anything when user does not select a workspace folder and there is more than one workspace folder', async () => { workspace.setup((w) => w.workspaceFolders).returns(() => [folder1, folder2]); - interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) - .returns(() => Promise.resolve([])); + interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); multiStepInputFactory.setup((f) => f.create()).verifiable(TypeMoq.Times.never()); const expectedItems = [ @@ -859,9 +859,7 @@ suite('Set Interpreter Command', () => { workspace.setup((w) => w.workspaceFolders).returns(() => undefined); - interpreterSelector - .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), true)) - .returns(() => Promise.resolve([])); + interpreterSelector.setup((i) => i.getSuggestions(TypeMoq.It.isAny())).returns(() => Promise.resolve([])); const multiStepInput = { run: (inputStepArg: InputStepType, state: InterpreterStateArgs) => { inputStep = inputStepArg; diff --git a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts index d322864bf168..81137acb52cf 100644 --- a/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts @@ -51,10 +51,7 @@ suite('Interpreters - selector', () => { let fileSystem: TypeMoq.IMock; let newComparer: TypeMoq.IMock; class TestInterpreterSelector extends InterpreterSelector { - public async suggestionToQuickPickItem( - suggestion: PythonEnvironment, - workspaceUri?: Uri, - ): Promise { + public suggestionToQuickPickItem(suggestion: PythonEnvironment, workspaceUri?: Uri): IInterpreterQuickPickItem { return super.suggestionToQuickPickItem(suggestion, workspaceUri); } } diff --git a/src/test/interpreters/display/progressDisplay.unit.test.ts b/src/test/interpreters/display/progressDisplay.unit.test.ts index 9fc2c5451738..db5b5cb0a52b 100644 --- a/src/test/interpreters/display/progressDisplay.unit.test.ts +++ b/src/test/interpreters/display/progressDisplay.unit.test.ts @@ -8,7 +8,7 @@ import { anything, capture, instance, mock, when } from 'ts-mockito'; import { CancellationToken, Disposable, Progress, ProgressOptions } from 'vscode'; import { ApplicationShell } from '../../../client/common/application/applicationShell'; import { ExperimentService } from '../../../client/common/experiments/service'; -import { Common, Interpreters } from '../../../client/common/utils/localize'; +import { Interpreters } from '../../../client/common/utils/localize'; import { noop } from '../../../client/common/utils/misc'; import { IComponentAdapter, IInterpreterLocatorProgressService } from '../../../client/interpreter/contracts'; import { InterpreterLocatorProgressStatubarHandler } from '../../../client/interpreter/display/progressDisplay'; @@ -60,7 +60,7 @@ suite('Interpreters - Display Progress', () => { refreshingCallback(undefined); const options = capture(shell.withProgress as never).last()[0] as ProgressOptions; - expect(options.title).to.be.equal(Common.loadingExtension()); + expect(options.title).to.be.equal(Interpreters.discovering()); }); test('Display refreshing message when refreshing interpreters for the second time', async () => { @@ -78,7 +78,7 @@ suite('Interpreters - Display Progress', () => { refreshingCallback(undefined); let options = capture(shell.withProgress as never).last()[0] as ProgressOptions; - expect(options.title).to.be.equal(Common.loadingExtension()); + expect(options.title).to.be.equal(Interpreters.discovering()); refreshingCallback(undefined); @@ -104,7 +104,7 @@ suite('Interpreters - Display Progress', () => { const callback = capture(shell.withProgress as never).last()[1] as ProgressTask; const promise = callback(undefined as never, undefined as never); - expect(options.title).to.be.equal(Common.loadingExtension()); + expect(options.title).to.be.equal(Interpreters.discovering()); refreshedCallback(undefined); // Promise must resolve when refreshed callback is invoked. diff --git a/src/test/pythonEnvironments/base/info/env.unit.test.ts b/src/test/pythonEnvironments/base/info/env.unit.test.ts index 54593afd584b..6ca0e167b94d 100644 --- a/src/test/pythonEnvironments/base/info/env.unit.test.ts +++ b/src/test/pythonEnvironments/base/info/env.unit.test.ts @@ -9,6 +9,17 @@ import { getEnvDisplayString } from '../../../../client/pythonEnvironments/base/ import { createLocatedEnv } from '../common'; suite('pyenvs info - getEnvDisplayString()', () => { + const name = 'my-env'; + const location = 'x/y/z/spam/'; + const arch = Architecture.x64; + const version = '3.8.1'; + const kind = PythonEnvKind.Venv; + const distro: PythonDistroInfo = { + org: 'Distro X', + defaultDisplayName: 'distroX 1.2', + version: parseVersionInfo('1.2.3')?.version, + binDir: 'distroX/bin', + }; function getEnv(info: { version?: string; arch?: Architecture; @@ -30,61 +41,26 @@ suite('pyenvs info - getEnvDisplayString()', () => { env.display = info.display; return env; } + const tests: [PythonEnvInfo, string][] = [ + [getEnv({}), 'Python'], + [getEnv({ version, arch, name, kind, distro }), "Python 3.8.1 64-bit ('my-env': venv)"], + // without "suffix" info + [getEnv({ version }), 'Python 3.8.1'], + [getEnv({ arch }), 'Python 64-bit'], + [getEnv({ version, arch }), 'Python 3.8.1 64-bit'], + // with "suffix" info + [getEnv({ name }), "Python ('my-env')"], + [getEnv({ kind }), 'Python (venv)'], + [getEnv({ name, kind }), "Python ('my-env': venv)"], + // env.location is ignored. + [getEnv({ location }), 'Python'], + [getEnv({ name, location }), "Python ('my-env')"], + ]; + tests.forEach(([env, expected]) => { + test(`"${expected}"`, () => { + const result = getEnvDisplayString(env); - suite('already set', () => { - [ - 'Python', // built: absolute minimal - 'Python 3.7.x x64 (my-env: venv)', // built: full - 'spam', - 'some env', - // corner cases - '---', - ' ', - ].forEach((display: string) => { - test(`"${display}"`, () => { - const expected = display; - const env = getEnv({ display }); - - const result = getEnvDisplayString(env); - - assert.equal(result, expected); - }); - }); - }); - - suite('built', () => { - const name = 'my-env'; - const location = 'x/y/z/spam/'; - const arch = Architecture.x64; - const version = '3.8.1'; - const kind = PythonEnvKind.Venv; - const distro: PythonDistroInfo = { - org: 'Distro X', - defaultDisplayName: 'distroX 1.2', - version: parseVersionInfo('1.2.3')?.version, - binDir: 'distroX/bin', - }; - const tests: [PythonEnvInfo, string][] = [ - [getEnv({}), 'Python'], - [getEnv({ version, arch, name, kind, distro }), "Python 3.8.1 64-bit ('my-env': venv)"], - // without "suffix" info - [getEnv({ version }), 'Python 3.8.1'], - [getEnv({ arch }), 'Python 64-bit'], - [getEnv({ version, arch }), 'Python 3.8.1 64-bit'], - // with "suffix" info - [getEnv({ name }), "Python ('my-env')"], - [getEnv({ kind }), 'Python (venv)'], - [getEnv({ name, kind }), "Python ('my-env': venv)"], - // env.location is ignored. - [getEnv({ location }), 'Python'], - [getEnv({ name, location }), "Python ('my-env')"], - ]; - tests.forEach(([env, expected]) => { - test(`"${expected}"`, () => { - const result = getEnvDisplayString(env); - - assert.equal(result, expected); - }); + assert.equal(result, expected); }); }); }); diff --git a/src/test/pythonEnvironments/base/locators/composite/resolverUtils.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/resolverUtils.unit.test.ts index c5de00c3072a..556072dd0804 100644 --- a/src/test/pythonEnvironments/base/locators/composite/resolverUtils.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/resolverUtils.unit.test.ts @@ -14,7 +14,7 @@ import { PythonVersion, UNKNOWN_PYTHON_VERSION, } from '../../../../../client/pythonEnvironments/base/info'; -import { buildEnvInfo } from '../../../../../client/pythonEnvironments/base/info/env'; +import { buildEnvInfo, getEnvDisplayString } from '../../../../../client/pythonEnvironments/base/info/env'; import { InterpreterInformation } from '../../../../../client/pythonEnvironments/base/info/interpreter'; import { parseVersion } from '../../../../../client/pythonEnvironments/base/info/pythonVersion'; import { TEST_LAYOUT_ROOT } from '../../../common/commonTestConstants'; @@ -61,6 +61,7 @@ suite('Resolver Utils', () => { }); envInfo.location = path.join(testPyenvVersionsDir, '3.9.0'); envInfo.name = '3.9.0'; + envInfo.display = getEnvDisplayString(envInfo); return envInfo; } @@ -114,7 +115,7 @@ suite('Resolver Utils', () => { test('resolveEnv', async () => { const python38path = path.join(testStoreAppRoot, 'python3.8.exe'); - const expected = { + const expected: PythonEnvInfo = { display: undefined, searchLocation: undefined, name: '', @@ -124,6 +125,7 @@ suite('Resolver Utils', () => { source: [PythonEnvSource.PathEnvVar], ...createExpectedInterpreterInfo(python38path), }; + expected.display = getEnvDisplayString(expected); const actual = await resolveBasicEnv({ executablePath: python38path, @@ -135,7 +137,7 @@ suite('Resolver Utils', () => { test('resolveEnv(string): forbidden path', async () => { const python38path = path.join(testLocalAppData, 'Program Files', 'WindowsApps', 'python3.8.exe'); - const expected = { + const expected: PythonEnvInfo = { display: undefined, searchLocation: undefined, name: '', @@ -145,6 +147,7 @@ suite('Resolver Utils', () => { source: [PythonEnvSource.PathEnvVar], ...createExpectedInterpreterInfo(python38path), }; + expected.display = getEnvDisplayString(expected); const actual = await resolveBasicEnv({ executablePath: python38path, @@ -180,6 +183,7 @@ suite('Resolver Utils', () => { fileInfo: undefined, name: 'base', }); + info.display = getEnvDisplayString(info); return info; } function createSimpleEnvInfo( @@ -189,7 +193,7 @@ suite('Resolver Utils', () => { name = '', location = '', ): PythonEnvInfo { - return { + const info: PythonEnvInfo = { name, location, kind, @@ -206,6 +210,8 @@ suite('Resolver Utils', () => { searchLocation: undefined, source: [], }; + info.display = getEnvDisplayString(info); + return info; } teardown(() => { @@ -284,7 +290,7 @@ suite('Resolver Utils', () => { name = '', location = '', ): PythonEnvInfo { - return { + const info: PythonEnvInfo = { name, location, kind, @@ -301,6 +307,8 @@ suite('Resolver Utils', () => { searchLocation: Uri.file(path.dirname(location)), source: [], }; + info.display = getEnvDisplayString(info); + return info; } test('resolveEnv', async () => { @@ -337,7 +345,7 @@ suite('Resolver Utils', () => { name = '', location = '', ): PythonEnvInfo { - return { + const info: PythonEnvInfo = { name, location, kind, @@ -354,6 +362,8 @@ suite('Resolver Utils', () => { searchLocation: undefined, source: [], }; + info.display = getEnvDisplayString(info); + return info; } test('resolveEnv', async () => { @@ -540,6 +550,7 @@ suite('Resolver Utils', () => { org: 'PythonCore', source: [PythonEnvSource.WindowsRegistry], }); + expected.display = getEnvDisplayString(expected); expected.distro.defaultDisplayName = 'Python 3.9 (64-bit)'; assertEnvEqual(actual, expected); }); @@ -559,6 +570,7 @@ suite('Resolver Utils', () => { org: 'PythonCodingPack', // Provided by registry source: [PythonEnvSource.WindowsRegistry, PythonEnvSource.PathEnvVar], }); + expected.display = getEnvDisplayString(expected); expected.distro.defaultDisplayName = 'Python 3.8 (32-bit)'; assertEnvEqual(actual, expected); }); @@ -585,6 +597,7 @@ suite('Resolver Utils', () => { name: 'conda3', source: [PythonEnvSource.WindowsRegistry], }); + expected.display = getEnvDisplayString(expected); expected.distro.defaultDisplayName = 'Anaconda py38_4.8.3'; assertEnvEqual(actual, expected); }); From 8c9ea8c9f4531da8c4379231aff94ebc7e5d4aac Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 16 Sep 2021 16:19:10 -0700 Subject: [PATCH 13/19] Fix unit tests --- .vscode/launch.json | 2 +- .../base/locators/composite/envsResolver.unit.test.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 107f75b919d8..c2fc063109b2 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -158,7 +158,7 @@ "--ui=tdd", "--recursive", "--colors", - "--grep", "xpyenvs", + //"--grep", "", "--timeout=300000" ], "outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"], diff --git a/src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts index 6694d77957ee..0038070076a6 100644 --- a/src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts @@ -66,6 +66,7 @@ suite('Python envs locator - Environments Resolver', () => { version: PythonVersion = UNKNOWN_PYTHON_VERSION, name = '', location = '', + display: string | undefined = undefined, ): PythonEnvInfo { return { name, @@ -77,7 +78,7 @@ suite('Python envs locator - Environments Resolver', () => { ctime: -1, mtime: -1, }, - display: undefined, + display, version, arch: Architecture.Unknown, distro: { org: '' }, @@ -117,6 +118,7 @@ suite('Python envs locator - Environments Resolver', () => { undefined, 'win1', path.join(testVirtualHomeDir, '.venvs', 'win1'), + "Python ('win1': venv)", ); const envsReturnedByParentLocator = [env1]; const parentLocator = new SimpleLocator(envsReturnedByParentLocator); From 2c1ffca5005d315ecada00c6ddfe799eb31bc8ef Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 16 Sep 2021 16:26:52 -0700 Subject: [PATCH 14/19] Fix comment --- .../interpreterSelector/commands/setInterpreter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 80ff4a2b8619..fd117f8ace21 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -70,7 +70,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { state: InterpreterStateArgs, ): Promise> { // If the list is refreshing, it's crucial to maintain sorting order at all - // times, so the visible items do not change. + // times so that the visible items do not change. const preserveOrderWhenFiltering = !!this.interpreterService.refreshPromise; const suggestions = await this.getItems(state.workspace); state.path = undefined; From 0cdc3349e6bd979c80948b66cffecaa2b3208950 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 16 Sep 2021 16:43:50 -0700 Subject: [PATCH 15/19] Ensure special active items are also maintained --- .../interpreterSelector/commands/setInterpreter.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index fd117f8ace21..6d94e9273157 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -113,6 +113,10 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if ('interpreter' in item && 'interpreter' in activeItemBeforeUpdate) { return arePathsSame(item.interpreter.path, activeItemBeforeUpdate.interpreter.path); } + if ('alwaysShow' in item && 'alwaysShow' in activeItemBeforeUpdate) { + // It's a special quickpick item, 'label' is a constant here instead of 'path'. + return item.label === activeItemBeforeUpdate.label; + } return false; }) : undefined; From d22021fb030adf5ecba29d212b09b2fb613da972 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 16 Sep 2021 23:41:24 -0700 Subject: [PATCH 16/19] Ensure we set recommended item after refresh finishes --- .../commands/setInterpreter.ts | 83 ++++++++++++------- .../composite/envsCollectionService.ts | 5 +- .../commands/setInterpreter.unit.test.ts | 33 ++++++-- 3 files changed, 82 insertions(+), 39 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 6d94e9273157..c44cfb72957b 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -4,8 +4,9 @@ 'use strict'; import { inject, injectable } from 'inversify'; +import { cloneDeep } from 'lodash'; import * as path from 'path'; -import { QuickPickItem } from 'vscode'; +import { QuickPick, QuickPickItem } from 'vscode'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../common/application/types'; import { Commands, Octicons } from '../../../../common/constants'; import { arePathsSame } from '../../../../common/platform/fs-paths'; @@ -99,28 +100,13 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { callback: async (event: PythonEnvironmentsChangedEvent, quickPick) => { if (this.interpreterService.refreshPromise) { quickPick.busy = true; - this.interpreterService.refreshPromise.then(() => { + this.interpreterService.refreshPromise.then(async () => { quickPick.busy = false; + // Ensure we set a recommended item after refresh has finished. + await this.updateQuickPickItems(quickPick, {}, state.workspace); }); } - // Active items are reset once we replace the current list with updated items, so save it. - const activeItemBeforeUpdate = - quickPick.activeItems.length > 0 ? quickPick.activeItems[0] : undefined; - quickPick.items = this.getUpdatedItems(quickPick.items, event, state.workspace); - // Ensure we maintain the same active item as before. - const activeItem = activeItemBeforeUpdate - ? quickPick.items.find((item) => { - if ('interpreter' in item && 'interpreter' in activeItemBeforeUpdate) { - return arePathsSame(item.interpreter.path, activeItemBeforeUpdate.interpreter.path); - } - if ('alwaysShow' in item && 'alwaysShow' in activeItemBeforeUpdate) { - // It's a special quickpick item, 'label' is a constant here instead of 'path'. - return item.label === activeItemBeforeUpdate.label; - } - return false; - }) - : undefined; - quickPick.activeItems = activeItem ? [activeItem] : []; + await this.updateQuickPickItems(quickPick, event, state.workspace); }, }, }); @@ -145,14 +131,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { suggestions.push(defaultInterpreterPathSuggestion); } const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource); - if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { - // If list is not refreshing, the first item is the recommended one. - const suggested = interpreterSuggestions[0]; - if (suggested) { - suggested.label = `${Octicons.Star} ${suggested.label}`; - suggested.description = Common.recommended(); - } - } + await this.setRecommendedItem(interpreterSuggestions, resource); suggestions.push(...interpreterSuggestions); return suggestions; } @@ -187,11 +166,35 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { return undefined; } - private getUpdatedItems( + private async updateQuickPickItems( + quickPick: QuickPick, + event: PythonEnvironmentsChangedEvent, + resource: Resource, + ) { + // Active items are reset once we replace the current list with updated items, so save it. + const activeItemBeforeUpdate = quickPick.activeItems.length > 0 ? quickPick.activeItems[0] : undefined; + quickPick.items = await this.getUpdatedItems(quickPick.items, event, resource); + // Ensure we maintain the same active item as before. + const activeItem = activeItemBeforeUpdate + ? quickPick.items.find((item) => { + if ('interpreter' in item && 'interpreter' in activeItemBeforeUpdate) { + return arePathsSame(item.interpreter.path, activeItemBeforeUpdate.interpreter.path); + } + if ('alwaysShow' in item && 'alwaysShow' in activeItemBeforeUpdate) { + // It's of special quickpick item type, 'label' is a constant here instead of 'path'. + return item.label === activeItemBeforeUpdate.label; + } + return false; + }) + : undefined; + quickPick.activeItems = activeItem ? [activeItem] : []; + } + + private async getUpdatedItems( items: readonly QuickPickType[], event: PythonEnvironmentsChangedEvent, resource: Resource, - ): QuickPickType[] { + ): Promise { const updatedItems = [...items.values()]; const env = event.old ?? event.update; let envIndex = -1; @@ -217,9 +220,29 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if (envIndex !== -1 && event.update === undefined) { updatedItems.splice(envIndex, 1); } + await this.setRecommendedItem(updatedItems, resource); return updatedItems; } + private async setRecommendedItem(items: QuickPickType[], resource: Resource) { + const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource); + if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { + // If list is in the final state, first suggestion is the recommended one. + const recommended = cloneDeep(interpreterSuggestions[0]); + recommended.label = `${Octicons.Star} ${recommended.label}`; + recommended.description = Common.recommended(); + const index = items.findIndex((item) => { + if ('interpreter' in item) { + return arePathsSame(item.interpreter.path, recommended.interpreter.path); + } + return false; + }); + if (index !== -1) { + items[index] = recommended; + } + } + } + @captureTelemetry(EventName.SELECT_INTERPRETER_ENTER_BUTTON) public async _enterOrBrowseInterpreterPath( input: IMultiStepInput, diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts index 3f88fd0994f4..f0785301691d 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts @@ -89,15 +89,16 @@ export class EnvsCollectionService extends PythonEnvsWatcher { const stopWatch = new StopWatch(); const deferred = createDeferred(); - // Ensure we set this before we trigger the promise to correctly track when a refresh has started. + // Ensure we set this before we trigger the promise to accurately track when a refresh has started. this.refreshPromises.set(query, deferred.promise); this.refreshStarted.fire(); const iterator = this.locator.iterEnvs(query); const promise = this.addEnvsToCacheFromIterator(iterator); return promise .then(async () => { - deferred.resolve(); + // Ensure we delete this before we resolve the promise to accurately track when a refresh finishes. this.refreshPromises.delete(query); + deferred.resolve(); sendTelemetryEvent(EventName.PYTHON_INTERPRETER_DISCOVERY, stopWatch.elapsedTime, { interpreters: this.cache.getAllEnvs().length, }); diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 29125b7d95ad..181ac3a955f6 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -41,7 +41,7 @@ const untildify = require('untildify'); type TelemetryEventType = { eventName: EventName; properties: unknown }; -suite('Set Interpreter Command', () => { +suite('xSet Interpreter Command', () => { let workspace: TypeMoq.IMock; let interpreterSelector: TypeMoq.IMock; let appShell: TypeMoq.IMock; @@ -254,6 +254,9 @@ suite('Set Interpreter Command', () => { actualParameters = options; }) .returns(() => Promise.resolve((undefined as unknown) as QuickPickItem)); + const refreshPromiseDeferred = createDeferred(); + // Assume a refresh is currently going on... + when(interpreterService.refreshPromise).thenReturn(refreshPromiseDeferred.promise); await setInterpreterCommand._pickInterpreter(multiStepInput.object, state); @@ -263,16 +266,13 @@ suite('Set Interpreter Command', () => { multiStepInput.verifyAll(); const quickPick = { - items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, refreshedItem], + items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, item], activeItems: [item], busy: false, }; interpreterSelector .setup((i) => i.suggestionToQuickPickItem(TypeMoq.It.isAny(), undefined)) .returns(() => refreshedItem); - const refreshPromiseDeferred = createDeferred(); - // Assume a refresh is currently going on... - when(interpreterService.refreshPromise).thenReturn(refreshPromiseDeferred.promise); const changeEvent: PythonEnvironmentsChangedEvent = { old: item.interpreter, @@ -291,10 +291,29 @@ suite('Set Interpreter Command', () => { 'Quickpick not updated correctly', ); + // Refresh is over; set the final states accordingly + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny())) + .returns(() => Promise.resolve([refreshedItem])); + when(interpreterService.refreshPromise).thenReturn(undefined); + refreshPromiseDeferred.resolve(); await sleep(1); - // Refresh finishes, so quickpick busy indicator should go away - assert.deepStrictEqual(quickPick.busy, false, 'Quickpick status not updated to ideal'); + + const recommended = cloneDeep(refreshedItem); + recommended.label = `${Octicons.Star} ${refreshedItem.label}`; + recommended.description = Common.recommended(); + assert.deepStrictEqual( + quickPick, + { + // Refresh has finished, so recommend an interpreter + items: [expectedEnterInterpreterPathSuggestion, defaultInterpreterPathSuggestion, recommended], + activeItems: [recommended], + // Refresh has finished, so quickpick busy indicator should go away + busy: false, + }, + 'Quickpick not updated correctly after refresh has finished', + ); }); test('If an item is selected, update state and return', async () => { From 37aeb2949f038c1874660a15cbb72566e07e5802 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 17 Sep 2021 09:43:05 -0700 Subject: [PATCH 17/19] Add doc comment --- .../interpreterSelector/commands/setInterpreter.ts | 6 ++++++ .../commands/setInterpreter.unit.test.ts | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index c44cfb72957b..2e9323da328b 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -166,6 +166,9 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { return undefined; } + /** + * Updates quickpick using the change event received. + */ private async updateQuickPickItems( quickPick: QuickPick, event: PythonEnvironmentsChangedEvent, @@ -190,6 +193,9 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { quickPick.activeItems = activeItem ? [activeItem] : []; } + /** + * Prepare updated items to replace the quickpick list with. + */ private async getUpdatedItems( items: readonly QuickPickType[], event: PythonEnvironmentsChangedEvent, diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 181ac3a955f6..5c895696b73f 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -41,7 +41,7 @@ const untildify = require('untildify'); type TelemetryEventType = { eventName: EventName; properties: unknown }; -suite('xSet Interpreter Command', () => { +suite('Set Interpreter Command', () => { let workspace: TypeMoq.IMock; let interpreterSelector: TypeMoq.IMock; let appShell: TypeMoq.IMock; From a7611369fa857f382e6836dece9efe83d47601fd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 20 Sep 2021 12:49:02 -0700 Subject: [PATCH 18/19] Code reviews --- .../commands/setInterpreter.ts | 30 +++++++++++-------- .../locators/composite/envsCollectionCache.ts | 1 + 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 2e9323da328b..58611f412c34 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -37,6 +37,13 @@ const untildify = require('untildify'); export type InterpreterStateArgs = { path?: string; workspace: Resource }; type QuickPickType = IInterpreterQuickPickItem | ISpecialQuickPickItem; +function isInterpreterQuickPickItem(item: QuickPickType): item is IInterpreterQuickPickItem { + return 'interpreter' in item; +} + +function isSpecialQuickPickItem(item: QuickPickType): item is ISpecialQuickPickItem { + return 'alwaysShow' in item; +} @injectable() export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { private readonly manualEntrySuggestion: ISpecialQuickPickItem = { @@ -142,7 +149,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if (activeInterpreter.length > 0) { return activeInterpreter[0]; } - const firstInterpreterSuggestion = suggestions.find((s) => 'interpreter' in s && s.interpreter); + const firstInterpreterSuggestion = suggestions.find((s) => isInterpreterQuickPickItem(s)); if (firstInterpreterSuggestion) { return firstInterpreterSuggestion; } @@ -180,11 +187,11 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { // Ensure we maintain the same active item as before. const activeItem = activeItemBeforeUpdate ? quickPick.items.find((item) => { - if ('interpreter' in item && 'interpreter' in activeItemBeforeUpdate) { + if (isInterpreterQuickPickItem(item) && isInterpreterQuickPickItem(activeItemBeforeUpdate)) { return arePathsSame(item.interpreter.path, activeItemBeforeUpdate.interpreter.path); } - if ('alwaysShow' in item && 'alwaysShow' in activeItemBeforeUpdate) { - // It's of special quickpick item type, 'label' is a constant here instead of 'path'. + if (isSpecialQuickPickItem(item) && isSpecialQuickPickItem(activeItemBeforeUpdate)) { + // 'label' is a constant here instead of 'path'. return item.label === activeItemBeforeUpdate.label; } return false; @@ -206,7 +213,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { let envIndex = -1; if (env) { envIndex = updatedItems.findIndex((item) => { - if ('interpreter' in item) { + if (isInterpreterQuickPickItem(item)) { return arePathsSame(item.interpreter.path, env.path); } return false; @@ -237,12 +244,11 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { const recommended = cloneDeep(interpreterSuggestions[0]); recommended.label = `${Octicons.Star} ${recommended.label}`; recommended.description = Common.recommended(); - const index = items.findIndex((item) => { - if ('interpreter' in item) { - return arePathsSame(item.interpreter.path, recommended.interpreter.path); - } - return false; - }); + const index = items.findIndex( + (item) => + isInterpreterQuickPickItem(item) && + arePathsSame(item.interpreter.path, recommended.interpreter.path), + ); if (index !== -1) { items[index] = recommended; } @@ -332,7 +338,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } const expandedPaths = suggestions.map((s) => { - const suggestionPath = 'interpreter' in s ? s.interpreter.path : ''; + const suggestionPath = isInterpreterQuickPickItem(s) ? s.interpreter.path : ''; let expandedPath = path.normalize(untildify(suggestionPath)); if (!path.isAbsolute(suggestionPath)) { diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts index cddbcfa477cc..0b3bc2e2237f 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts @@ -79,6 +79,7 @@ export class PythonEnvInfoCache extends PythonEnvsWatcher (isValid ? -1 : index)).filter((i) => i !== -1); invalidIndexes.forEach((index) => { const env = this.envs.splice(index, 1)[0]; + // Ensure we fire events for any envs removed from collection. this.fire({ old: env, update: undefined }); }); } From 1d682e156338f77288752010d46580927fdf1853 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Sep 2021 00:07:37 -0700 Subject: [PATCH 19/19] Code reviews II --- .../interpreterSelector/commands/setInterpreter.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 58611f412c34..f11708b5019f 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -44,6 +44,7 @@ function isInterpreterQuickPickItem(item: QuickPickType): item is IInterpreterQu function isSpecialQuickPickItem(item: QuickPickType): item is ISpecialQuickPickItem { return 'alwaysShow' in item; } + @injectable() export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { private readonly manualEntrySuggestion: ISpecialQuickPickItem = { @@ -212,12 +213,9 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { const env = event.old ?? event.update; let envIndex = -1; if (env) { - envIndex = updatedItems.findIndex((item) => { - if (isInterpreterQuickPickItem(item)) { - return arePathsSame(item.interpreter.path, env.path); - } - return false; - }); + envIndex = updatedItems.findIndex( + (item) => isInterpreterQuickPickItem(item) && arePathsSame(item.interpreter.path, env.path), + ); } if (event.update) { const newSuggestion: QuickPickType = this.interpreterSelector.suggestionToQuickPickItem( @@ -240,7 +238,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { private async setRecommendedItem(items: QuickPickType[], resource: Resource) { const interpreterSuggestions = await this.interpreterSelector.getSuggestions(resource); if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { - // If list is in the final state, first suggestion is the recommended one. + // List is in the final state, so first suggestion is the recommended one. const recommended = cloneDeep(interpreterSuggestions[0]); recommended.label = `${Octicons.Star} ${recommended.label}`; recommended.description = Common.recommended();