From ee7a56661d529b50bd60743e8c2cde397fcaa78c Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 19 Sep 2023 14:55:01 -0700 Subject: [PATCH 1/5] Only send telemetry when starting a new refresh --- .../base/locators/composite/envsCollectionService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts index 2567168c6325..fb1a791d07ed 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts @@ -115,9 +115,9 @@ export class EnvsCollectionService extends PythonEnvsWatcher this.sendTelemetry(query, stopWatch)); } - return refreshPromise.then(() => this.sendTelemetry(query, stopWatch)); + return refreshPromise; } private startRefresh(query: PythonLocatorQuery | undefined): Promise { From 89143aecf5b63f9969959f12c699feee48b3afe8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 20 Sep 2023 00:27:55 -0700 Subject: [PATCH 2/5] Do not block showing interpreter list on discovery --- src/client/common/utils/multiStepInput.ts | 10 ++++++++-- .../interpreterSelector/commands/setInterpreter.ts | 9 ++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/client/common/utils/multiStepInput.ts b/src/client/common/utils/multiStepInput.ts index e44879e8bbbb..4028451ee158 100644 --- a/src/client/common/utils/multiStepInput.ts +++ b/src/client/common/utils/multiStepInput.ts @@ -47,7 +47,7 @@ export interface IQuickPickParameters { totalSteps?: number; canGoBack?: boolean; items: T[]; - activeItem?: T | Promise; + activeItem?: T | ((quickPick: QuickPick) => Promise); placeholder: string | undefined; customButtonSetups?: QuickInputButtonSetup[]; matchOnDescription?: boolean; @@ -156,7 +156,13 @@ export class MultiStepInput implements IMultiStepInput { initialize(input); } if (activeItem) { - input.activeItems = [await activeItem]; + if(typeof activeItem === 'function') { + activeItem(input).then((item) =>{ + if (input.activeItems.length === 0) { + input.activeItems = [item]; + } + }) + } } else { input.activeItems = []; } diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index c0876ff518dd..75e2e6b0852a 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -177,7 +177,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implem items: suggestions, sortByLabel: !preserveOrderWhenFiltering, keepScrollPosition: true, - activeItem: this.getActiveItem(state.workspace, suggestions), // Use a promise here to ensure quickpick is initialized synchronously. + activeItem: (quickPick) => this.getActiveItem(state.workspace, quickPick), // Use a promise here to ensure quickpick is initialized synchronously. matchOnDetail: true, matchOnDescription: true, title, @@ -277,8 +277,9 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implem return getGroupedQuickPickItems(items, recommended, workspaceFolder?.uri.fsPath); } - private async getActiveItem(resource: Resource, suggestions: QuickPickType[]) { + private async getActiveItem(resource: Resource, quickPick: QuickPick) { const interpreter = await this.interpreterService.getActiveInterpreter(resource); + const suggestions = quickPick.items; const activeInterpreterItem = suggestions.find( (i) => isInterpreterQuickPickItem(i) && i.interpreter.id === interpreter?.id, ); @@ -339,7 +340,9 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implem return false; }) : undefined; - quickPick.activeItems = activeItem ? [activeItem] : []; + if(activeItem) { + quickPick.activeItems = [activeItem]; + } } /** From 03587a3cf2122844273e837b5852688e2c72fc53 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 20 Sep 2023 00:49:04 -0700 Subject: [PATCH 3/5] Fix autoselection bug --- src/client/interpreter/autoSelection/index.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index a57577c8c918..7714c487ed30 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -181,6 +181,11 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio return this.stateFactory.createWorkspacePersistentState(key, undefined); } + private getAutoSelectionQueriedOnceState(): IPersistentState { + const key = `autoSelectionInterpretersQueriedOnce`; + return this.stateFactory.createWorkspacePersistentState(key, undefined); + } + /** * Auto-selection logic: * 1. If there are cached interpreters (not the first session in this workspace) @@ -200,7 +205,12 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio }); } - await this.interpreterService.refreshPromise; + const globalQueriedState = this.getAutoSelectionQueriedOnceState(); + if (!globalQueriedState.value) { + // Global interpreters are loaded the first time an extension loads, after which we don't need to + // wait on global interpreter promise refresh. + await this.interpreterService.refreshPromise; + } const interpreters = this.interpreterService.getInterpreters(resource); const workspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource); @@ -215,6 +225,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio } queriedState.updateValue(true); + globalQueriedState.updateValue(true); this.didAutoSelectedInterpreterEmitter.fire(); } From ab0098a0bdd1961b8a9dc3f9cbf1e663c9dc8a76 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 20 Sep 2023 16:26:26 -0700 Subject: [PATCH 4/5] Fix lint --- src/client/common/utils/multiStepInput.ts | 8 ++++---- .../interpreterSelector/commands/setInterpreter.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/common/utils/multiStepInput.ts b/src/client/common/utils/multiStepInput.ts index 4028451ee158..e2b2567b5b4e 100644 --- a/src/client/common/utils/multiStepInput.ts +++ b/src/client/common/utils/multiStepInput.ts @@ -156,12 +156,12 @@ export class MultiStepInput implements IMultiStepInput { initialize(input); } if (activeItem) { - if(typeof activeItem === 'function') { - activeItem(input).then((item) =>{ + if (typeof activeItem === 'function') { + activeItem(input).then((item) => { if (input.activeItems.length === 0) { - input.activeItems = [item]; + input.activeItems = [item]; } - }) + }); } } else { input.activeItems = []; diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 75e2e6b0852a..3f76e3cb8684 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -340,7 +340,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implem return false; }) : undefined; - if(activeItem) { + if (activeItem) { quickPick.activeItems = [activeItem]; } } From 79fdd4d8963b611a12f91aa5762b21ed8b1b0f18 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 20 Sep 2023 20:11:57 -0700 Subject: [PATCH 5/5] Add unit tests --- .../commands/setInterpreter.ts | 2 +- .../commands/setInterpreter.unit.test.ts | 31 +++++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 3f76e3cb8684..9b8ecec74f9f 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -50,7 +50,7 @@ import { BaseInterpreterSelectorCommand } from './base'; const untildify = require('untildify'); export type InterpreterStateArgs = { path?: string; workspace: Resource }; -type QuickPickType = IInterpreterQuickPickItem | ISpecialQuickPickItem | QuickPickItem; +export type QuickPickType = IInterpreterQuickPickItem | ISpecialQuickPickItem | QuickPickItem; function isInterpreterQuickPickItem(item: QuickPickType): item is IInterpreterQuickPickItem { return 'interpreter' in item; diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index 7059fb7ab26f..f177db5c2a32 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -31,6 +31,7 @@ import { import { EnvGroups, InterpreterStateArgs, + QuickPickType, SetInterpreterCommand, } from '../../../../client/interpreter/configuration/interpreterSelector/commands/setInterpreter'; import { @@ -265,8 +266,14 @@ suite('Set Interpreter Command', () => { delete actualParameters!.initialize; delete actualParameters!.customButtonSetups; delete actualParameters!.onChangeItem; - const activeItem = await actualParameters!.activeItem; - assert.deepStrictEqual(activeItem, recommended); + if (typeof actualParameters!.activeItem === 'function') { + const activeItem = await actualParameters!.activeItem(({ items: suggestions } as unknown) as QuickPick< + QuickPickType + >); + assert.deepStrictEqual(activeItem, recommended); + } else { + assert(false, 'Not a function'); + } delete actualParameters!.activeItem; assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal'); }); @@ -308,8 +315,14 @@ suite('Set Interpreter Command', () => { delete actualParameters!.initialize; delete actualParameters!.customButtonSetups; delete actualParameters!.onChangeItem; - const activeItem = await actualParameters!.activeItem; - assert.deepStrictEqual(activeItem, noPythonInstalled); + if (typeof actualParameters!.activeItem === 'function') { + const activeItem = await actualParameters!.activeItem(({ items: suggestions } as unknown) as QuickPick< + QuickPickType + >); + assert.deepStrictEqual(activeItem, noPythonInstalled); + } else { + assert(false, 'Not a function'); + } delete actualParameters!.activeItem; assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal'); }); @@ -666,8 +679,14 @@ suite('Set Interpreter Command', () => { delete actualParameters!.initialize; delete actualParameters!.customButtonSetups; delete actualParameters!.onChangeItem; - const activeItem = await actualParameters!.activeItem; - assert.deepStrictEqual(activeItem, recommended); + if (typeof actualParameters!.activeItem === 'function') { + const activeItem = await actualParameters!.activeItem(({ items: suggestions } as unknown) as QuickPick< + QuickPickType + >); + assert.deepStrictEqual(activeItem, recommended); + } else { + assert(false, 'Not a function'); + } delete actualParameters!.activeItem; assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal');