From 8b5ab97585167d6403c4372cf782b65c3e6b2e33 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Sep 2022 17:41:23 -0700 Subject: [PATCH 1/5] Expose interpreter quickpick with filtering --- .../commands/setInterpreter.ts | 50 ++++++++++++++----- src/client/interpreter/configuration/types.ts | 8 +++ src/client/interpreter/serviceRegistry.ts | 3 ++ 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index aad89c58959b..04e3e4d2419b 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -23,12 +23,13 @@ import { IQuickPickParameters, } from '../../../../common/utils/multiStepInput'; import { SystemVariables } from '../../../../common/variables/systemVariables'; -import { EnvironmentType } from '../../../../pythonEnvironments/info'; +import { EnvironmentType, PythonEnvironment } from '../../../../pythonEnvironments/info'; import { captureTelemetry, sendTelemetryEvent } from '../../../../telemetry'; import { EventName } from '../../../../telemetry/constants'; import { IInterpreterService, PythonEnvironmentsChangedEvent } from '../../../contracts'; import { isProblematicCondaEnvironment } from '../../environmentTypeComparer'; import { + IInterpreterQuickPick, IInterpreterQuickPickItem, IInterpreterSelector, IPythonPathUpdaterServiceManager, @@ -69,7 +70,7 @@ export namespace EnvGroups { } @injectable() -export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { +export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implements IInterpreterQuickPick { private readonly manualEntrySuggestion: ISpecialQuickPickItem = { label: `${Octicons.Add} ${InterpreterQuickPickList.enterPath.label}`, alwaysShow: true, @@ -126,11 +127,12 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { public async _pickInterpreter( input: IMultiStepInput, state: InterpreterStateArgs, + filter?: (i: PythonEnvironment) => boolean, ): Promise> { // If the list is refreshing, it's crucial to maintain sorting order at all // times so that the visible items do not change. const preserveOrderWhenFiltering = !!this.interpreterService.refreshPromise; - const suggestions = this._getItems(state.workspace); + const suggestions = this._getItems(state.workspace, filter); state.path = undefined; const currentInterpreterPathDisplay = this.pathUtils.getDisplayName( this.configurationService.getSettings(state.workspace).pythonPath, @@ -179,10 +181,10 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { // Items are in the final state as all previous callbacks have finished executing. quickPick.busy = false; // Ensure we set a recommended item after refresh has finished. - this.updateQuickPickItems(quickPick, {}, state.workspace); + this.updateQuickPickItems(quickPick, {}, state.workspace, filter); }); } - this.updateQuickPickItems(quickPick, event, state.workspace); + this.updateQuickPickItems(quickPick, event, state.workspace, filter); }, }, }); @@ -204,26 +206,33 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { return undefined; } - public _getItems(resource: Resource): QuickPickType[] { + public _getItems(resource: Resource, filter: ((i: PythonEnvironment) => boolean) | undefined): QuickPickType[] { const suggestions: QuickPickType[] = [this.manualEntrySuggestion]; const defaultInterpreterPathSuggestion = this.getDefaultInterpreterPathSuggestion(resource); if (defaultInterpreterPathSuggestion) { suggestions.push(defaultInterpreterPathSuggestion); } - const interpreterSuggestions = this.getSuggestions(resource); + const interpreterSuggestions = this.getSuggestions(resource, filter); this.finalizeItems(interpreterSuggestions, resource); suggestions.push(...interpreterSuggestions); return suggestions; } - private getSuggestions(resource: Resource): QuickPickType[] { + private getSuggestions( + resource: Resource, + filter: ((i: PythonEnvironment) => boolean) | undefined, + ): QuickPickType[] { const workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); - const items = this.interpreterSelector.getSuggestions(resource, !!this.interpreterService.refreshPromise); + const items = this.interpreterSelector + .getSuggestions(resource, !!this.interpreterService.refreshPromise) + .filter((i) => !filter || filter(i.interpreter)); if (this.interpreterService.refreshPromise) { // We cannot put items in groups while the list is loading as group of an item can change. return items; } - const itemsWithFullName = this.interpreterSelector.getSuggestions(resource, true); + const itemsWithFullName = this.interpreterSelector + .getSuggestions(resource, true) + .filter((i) => !filter || filter(i.interpreter)); const recommended = this.interpreterSelector.getRecommendedSuggestion( itemsWithFullName, this.workspaceService.getWorkspaceFolder(resource)?.uri, @@ -277,10 +286,11 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { quickPick: QuickPick, event: PythonEnvironmentsChangedEvent, resource: Resource, + filter: ((i: PythonEnvironment) => boolean) | undefined, ) { // 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, resource); + quickPick.items = this.getUpdatedItems(quickPick.items, event, resource, filter); // Ensure we maintain the same active item as before. const activeItem = activeItemBeforeUpdate ? quickPick.items.find((item) => { @@ -304,10 +314,14 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { items: readonly QuickPickType[], event: PythonEnvironmentsChangedEvent, resource: Resource, + filter: ((i: PythonEnvironment) => boolean) | undefined, ): QuickPickType[] { const updatedItems = [...items.values()]; const areItemsGrouped = items.find((item) => isSeparatorItem(item)); const env = event.old ?? event.new; + if (filter && event.new && !filter(event.new)) { + event.new = undefined; // Ignore events not related to relevant envs. + } let envIndex = -1; if (env) { envIndex = updatedItems.findIndex( @@ -476,7 +490,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { const wkspace = targetConfig[0].folderUri; const interpreterState: InterpreterStateArgs = { path: undefined, workspace: wkspace }; const multiStep = this.multiStepFactory.create(); - await multiStep.run((input, s) => this._pickInterpreter(input, s), interpreterState); + await multiStep.run((input, s) => this._pickInterpreter(input, s, undefined), interpreterState); if (interpreterState.path !== undefined) { // User may choose to have an empty string stored, so variable `interpreterState.path` may be @@ -486,6 +500,16 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } } + public async getInterpreterViaQuickPick( + workspace: Resource, + filter: ((i: PythonEnvironment) => boolean) | undefined, + ): Promise { + const interpreterState: InterpreterStateArgs = { path: undefined, workspace }; + const multiStep = this.multiStepFactory.create(); + await multiStep.run((input, s) => this._pickInterpreter(input, s, filter), interpreterState); + return interpreterState.path; + } + /** * Check if the interpreter that was entered exists in the list of suggestions. * If it does, it means that it had already been discovered, @@ -495,7 +519,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { */ // eslint-disable-next-line class-methods-use-this private sendInterpreterEntryTelemetry(selection: string, workspace: Resource): void { - const suggestions = this._getItems(workspace); + const suggestions = this._getItems(workspace, undefined); let interpreterPath = path.normalize(untildify(selection)); if (!path.isAbsolute(interpreterPath)) { diff --git a/src/client/interpreter/configuration/types.ts b/src/client/interpreter/configuration/types.ts index 2621a297defe..1e57f4ffcfec 100644 --- a/src/client/interpreter/configuration/types.ts +++ b/src/client/interpreter/configuration/types.ts @@ -65,3 +65,11 @@ export interface IInterpreterComparer { compare(a: PythonEnvironment, b: PythonEnvironment): number; getRecommended(interpreters: PythonEnvironment[], resource: Resource): PythonEnvironment | undefined; } + +export const IInterpreterQuickPick = Symbol('IInterpreterQuickPick'); +export interface IInterpreterQuickPick { + getInterpreterViaQuickPick( + workspace: Resource, + filter?: (i: PythonEnvironment) => boolean, + ): Promise; +} diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 6d36a4b9277d..8b5b12148e8f 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -21,6 +21,7 @@ import { PythonPathUpdaterService } from './configuration/pythonPathUpdaterServi import { PythonPathUpdaterServiceFactory } from './configuration/pythonPathUpdaterServiceFactory'; import { IInterpreterComparer, + IInterpreterQuickPick, IInterpreterSelector, IPythonPathUpdaterServiceFactory, IPythonPathUpdaterServiceManager, @@ -62,6 +63,8 @@ export function registerInterpreterTypes(serviceManager: IServiceManager): void IExtensionSingleActivationService, SetShebangInterpreterCommand, ); + serviceManager.addSingleton(IInterpreterQuickPick, SetInterpreterCommand); + serviceManager.addBinding(IInterpreterQuickPick, IExtensionSingleActivationService); serviceManager.addSingleton(IExtensionActivationService, VirtualEnvironmentPrompt); From 14574ec9ff908ebf7842072ffbf056de615386c8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 14 Sep 2022 00:05:46 -0700 Subject: [PATCH 2/5] Add test --- .../commands/setInterpreter.ts | 2 +- .../commands/setInterpreter.unit.test.ts | 115 ++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 04e3e4d2419b..0c04da6e3b3f 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -320,7 +320,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implem const areItemsGrouped = items.find((item) => isSeparatorItem(item)); const env = event.old ?? event.new; if (filter && event.new && !filter(event.new)) { - event.new = undefined; // Ignore events not related to relevant envs. + event.new = undefined; // Remove envs we're not looking for from the list. } let envIndex = -1; if (env) { diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 1bd27b5d2cbb..b42b1aaa94e7 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -471,6 +471,121 @@ suite('Set Interpreter Command', () => { assert.deepStrictEqual(actualParameters?.items, expectedParameters.items, 'Params not equal'); }); + test('Items displayed should be filtered out if a filter is provided', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + const interpreterItems: IInterpreterQuickPickItem[] = [ + { + description: `${workspacePath}/interpreterPath1`, + detail: '', + label: 'This is the selected Python path', + path: `${workspacePath}/interpreterPath1`, + interpreter: { + id: `${workspacePath}/interpreterPath1`, + path: `${workspacePath}/interpreterPath1`, + envType: EnvironmentType.Venv, + } as PythonEnvironment, + }, + { + description: 'interpreterPath2', + detail: '', + label: 'This is the selected Python path', + path: 'interpreterPath2', + interpreter: { + id: 'interpreterPath2', + path: 'interpreterPath2', + envType: EnvironmentType.VirtualEnvWrapper, + } as PythonEnvironment, + }, + { + description: 'interpreterPath3', + detail: '', + label: 'This is the selected Python path', + path: 'interpreterPath3', + interpreter: { + id: 'interpreterPath3', + path: 'interpreterPath3', + envType: EnvironmentType.VirtualEnvWrapper, + } as PythonEnvironment, + }, + { + description: 'interpreterPath4', + detail: '', + label: 'This is the selected Python path', + path: 'interpreterPath4', + interpreter: { + path: 'interpreterPath4', + id: 'interpreterPath4', + envType: EnvironmentType.Conda, + } as PythonEnvironment, + }, + item, + { + description: 'interpreterPath5', + detail: '', + label: 'This is the selected Python path', + path: 'interpreterPath5', + interpreter: { + path: 'interpreterPath5', + id: 'interpreterPath5', + envType: EnvironmentType.Global, + } as PythonEnvironment, + }, + ]; + interpreterSelector.reset(); + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => interpreterItems); + interpreterSelector + .setup((i) => i.getRecommendedSuggestion(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => item); + const recommended = cloneDeep(item); + recommended.label = `${Octicons.Star} ${item.label}`; + recommended.description = interpreterPath; + const suggestions = [ + expectedEnterInterpreterPathSuggestion, + defaultInterpreterPathSuggestion, + { kind: QuickPickItemKind.Separator, label: EnvGroups.Recommended }, + recommended, + { label: EnvGroups.VirtualEnvWrapper, kind: QuickPickItemKind.Separator }, + interpreterItems[1], + interpreterItems[2], + { label: EnvGroups.Global, kind: QuickPickItemKind.Separator }, + interpreterItems[5], + ]; + const expectedParameters: IQuickPickParameters = { + placeholder: `Selected Interpreter: ${currentPythonPath}`, + items: suggestions, + activeItem: recommended, + matchOnDetail: true, + matchOnDescription: true, + title: InterpreterQuickPickList.browsePath.openButtonLabel, + sortByLabel: true, + keepScrollPosition: true, + }; + 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, + (e) => e.envType === EnvironmentType.VirtualEnvWrapper || e.envType === EnvironmentType.Global, + ); + + expect(actualParameters).to.not.equal(undefined, 'Parameters not set'); + const refreshButtons = actualParameters!.customButtonSetups; + expect(refreshButtons).to.not.equal(undefined, 'Callback not set'); + delete actualParameters!.initialize; + delete actualParameters!.customButtonSetups; + delete actualParameters!.onChangeItem; + assert.deepStrictEqual(actualParameters?.items, expectedParameters.items, 'Params not equal'); + }); + test('If system variables are used in the default interpreter path, make sure they are resolved when the path is displayed', async () => { // Create a SetInterpreterCommand instance from scratch, and use a different defaultInterpreterPath from the rest of the tests. const workspaceDefaultInterpreterPath = '${workspaceFolder}/defaultInterpreterPath'; From 6ba9a955cb6a25c24b28d51ee50b453887e5ea7f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 14 Sep 2022 16:15:40 -0700 Subject: [PATCH 3/5] Tmep --- .../interpreterSelector/commands/setInterpreter.ts | 2 ++ src/client/interpreter/serviceRegistry.ts | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 0c04da6e3b3f..ee18b72dfdb3 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -69,6 +69,8 @@ export namespace EnvGroups { export const Recommended = Common.recommended; } +export const SET_INTERPRETER_ID = 'SET_INTERPRETER_ID'; + @injectable() export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implements IInterpreterQuickPick { private readonly manualEntrySuggestion: ISpecialQuickPickItem = { diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 8b5b12148e8f..25785e0d1c93 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -14,7 +14,7 @@ import { EnvironmentTypeComparer } from './configuration/environmentTypeComparer import { InstallPythonCommand } from './configuration/interpreterSelector/commands/installPython'; import { InstallPythonViaTerminal } from './configuration/interpreterSelector/commands/installPython/installPythonViaTerminal'; import { ResetInterpreterCommand } from './configuration/interpreterSelector/commands/resetInterpreter'; -import { SetInterpreterCommand } from './configuration/interpreterSelector/commands/setInterpreter'; +import { SetInterpreterCommand, SET_INTERPRETER_ID } from './configuration/interpreterSelector/commands/setInterpreter'; import { SetShebangInterpreterCommand } from './configuration/interpreterSelector/commands/setShebangInterpreter'; import { InterpreterSelector } from './configuration/interpreterSelector/interpreterSelector'; import { PythonPathUpdaterService } from './configuration/pythonPathUpdaterService'; @@ -63,8 +63,11 @@ export function registerInterpreterTypes(serviceManager: IServiceManager): void IExtensionSingleActivationService, SetShebangInterpreterCommand, ); - serviceManager.addSingleton(IInterpreterQuickPick, SetInterpreterCommand); - serviceManager.addBinding(IInterpreterQuickPick, IExtensionSingleActivationService); + serviceManager.addSingleton( + IExtensionSingleActivationService, + SetInterpreterCommand, + SET_INTERPRETER_ID, + ); serviceManager.addSingleton(IExtensionActivationService, VirtualEnvironmentPrompt); From d78034a48553f1f3971af47c90b2d01483978984 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 14 Sep 2022 16:22:41 -0700 Subject: [PATCH 4/5] Fix error --- .../interpreterSelector/commands/setInterpreter.ts | 2 -- src/client/interpreter/serviceRegistry.ts | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index ee18b72dfdb3..0c04da6e3b3f 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -69,8 +69,6 @@ export namespace EnvGroups { export const Recommended = Common.recommended; } -export const SET_INTERPRETER_ID = 'SET_INTERPRETER_ID'; - @injectable() export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implements IInterpreterQuickPick { private readonly manualEntrySuggestion: ISpecialQuickPickItem = { diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 25785e0d1c93..7a5ea6ef0678 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -14,7 +14,7 @@ import { EnvironmentTypeComparer } from './configuration/environmentTypeComparer import { InstallPythonCommand } from './configuration/interpreterSelector/commands/installPython'; import { InstallPythonViaTerminal } from './configuration/interpreterSelector/commands/installPython/installPythonViaTerminal'; import { ResetInterpreterCommand } from './configuration/interpreterSelector/commands/resetInterpreter'; -import { SetInterpreterCommand, SET_INTERPRETER_ID } from './configuration/interpreterSelector/commands/setInterpreter'; +import { SetInterpreterCommand } from './configuration/interpreterSelector/commands/setInterpreter'; import { SetShebangInterpreterCommand } from './configuration/interpreterSelector/commands/setShebangInterpreter'; import { InterpreterSelector } from './configuration/interpreterSelector/interpreterSelector'; import { PythonPathUpdaterService } from './configuration/pythonPathUpdaterService'; @@ -66,8 +66,8 @@ export function registerInterpreterTypes(serviceManager: IServiceManager): void serviceManager.addSingleton( IExtensionSingleActivationService, SetInterpreterCommand, - SET_INTERPRETER_ID, ); + serviceManager.addSingleton(IInterpreterQuickPick, SetInterpreterCommand); serviceManager.addSingleton(IExtensionActivationService, VirtualEnvironmentPrompt); From c41a44b42dcbad7db014c3479bf05f6cde21110e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 14 Sep 2022 16:30:23 -0700 Subject: [PATCH 5/5] Fix unit test --- src/client/interpreter/serviceRegistry.ts | 4 ---- src/test/interpreters/serviceRegistry.unit.test.ts | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 7a5ea6ef0678..cb60c370b84f 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -63,10 +63,6 @@ export function registerInterpreterTypes(serviceManager: IServiceManager): void IExtensionSingleActivationService, SetShebangInterpreterCommand, ); - serviceManager.addSingleton( - IExtensionSingleActivationService, - SetInterpreterCommand, - ); serviceManager.addSingleton(IInterpreterQuickPick, SetInterpreterCommand); serviceManager.addSingleton(IExtensionActivationService, VirtualEnvironmentPrompt); diff --git a/src/test/interpreters/serviceRegistry.unit.test.ts b/src/test/interpreters/serviceRegistry.unit.test.ts index 442ad7cdf3d6..a6fdce77dd3e 100644 --- a/src/test/interpreters/serviceRegistry.unit.test.ts +++ b/src/test/interpreters/serviceRegistry.unit.test.ts @@ -24,6 +24,7 @@ import { PythonPathUpdaterService } from '../../client/interpreter/configuration import { PythonPathUpdaterServiceFactory } from '../../client/interpreter/configuration/pythonPathUpdaterServiceFactory'; import { IInterpreterComparer, + IInterpreterQuickPick, IInterpreterSelector, IPythonPathUpdaterServiceFactory, IPythonPathUpdaterServiceManager, @@ -53,6 +54,7 @@ suite('Interpreters - Service Registry', () => { [IExtensionSingleActivationService, InstallPythonCommand], [IExtensionSingleActivationService, InstallPythonViaTerminal], [IExtensionSingleActivationService, SetInterpreterCommand], + [IInterpreterQuickPick, SetInterpreterCommand], [IExtensionSingleActivationService, ResetInterpreterCommand], [IExtensionSingleActivationService, SetShebangInterpreterCommand], @@ -63,7 +65,6 @@ suite('Interpreters - Service Registry', () => { [IPythonPathUpdaterServiceFactory, PythonPathUpdaterServiceFactory], [IPythonPathUpdaterServiceManager, PythonPathUpdaterService], - [IInterpreterSelector, InterpreterSelector], [IShebangCodeLensProvider, ShebangCodeLensProvider], [IInterpreterHelper, InterpreterHelper],