Skip to content

Commit

Permalink
Fix bugs related to discovery blocking other features (microsoft#22041)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kartik Raj authored and eleanorjboyd committed Oct 2, 2023
1 parent a9b4f0d commit 8d4e30d
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 15 deletions.
10 changes: 8 additions & 2 deletions src/client/common/utils/multiStepInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export interface IQuickPickParameters<T extends QuickPickItem, E = any> {
totalSteps?: number;
canGoBack?: boolean;
items: T[];
activeItem?: T | Promise<T>;
activeItem?: T | ((quickPick: QuickPick<T>) => Promise<T>);
placeholder: string | undefined;
customButtonSetups?: QuickInputButtonSetup[];
matchOnDescription?: boolean;
Expand Down Expand Up @@ -156,7 +156,13 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
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 = [];
}
Expand Down
13 changes: 12 additions & 1 deletion src/client/interpreter/autoSelection/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,11 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio
return this.stateFactory.createWorkspacePersistentState(key, undefined);
}

private getAutoSelectionQueriedOnceState(): IPersistentState<boolean | undefined> {
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)
Expand All @@ -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);

Expand All @@ -215,6 +225,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio
}

queriedState.updateValue(true);
globalQueriedState.updateValue(true);

this.didAutoSelectedInterpreterEmitter.fire();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<QuickPickType>) {
const interpreter = await this.interpreterService.getActiveInterpreter(resource);
const suggestions = quickPick.items;
const activeInterpreterItem = suggestions.find(
(i) => isInterpreterQuickPickItem(i) && i.interpreter.id === interpreter?.id,
);
Expand Down Expand Up @@ -339,7 +340,9 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand implem
return false;
})
: undefined;
quickPick.activeItems = activeItem ? [activeItem] : [];
if (activeItem) {
quickPick.activeItems = [activeItem];
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
// Do not trigger another refresh if a refresh has previously finished.
return Promise.resolve();
}
refreshPromise = this.startRefresh(query);
refreshPromise = this.startRefresh(query).then(() => this.sendTelemetry(query, stopWatch));
}
return refreshPromise.then(() => this.sendTelemetry(query, stopWatch));
return refreshPromise;
}

private startRefresh(query: PythonLocatorQuery | undefined): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import {
EnvGroups,
InterpreterStateArgs,
QuickPickType,
SetInterpreterCommand,
} from '../../../../client/interpreter/configuration/interpreterSelector/commands/setInterpreter';
import {
Expand Down Expand Up @@ -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');
});
Expand Down Expand Up @@ -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');
});
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit 8d4e30d

Please sign in to comment.