Skip to content

Commit 44d450f

Browse files
Kartik Rajeleanorjboyd
Kartik Raj
authored and
eleanorjboyd
committed
Ensure interpreter quickpick is initialized synchronously (microsoft#19828)
For microsoft#19101 Before the change item events start coming in, we have to ensure quickpick is ready to receive those events.
1 parent 142cd18 commit 44d450f

3 files changed

Lines changed: 48 additions & 41 deletions

File tree

‎src/client/common/utils/multiStepInput.ts‎

Lines changed: 38 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export interface IQuickPickParameters<T extends QuickPickItem, E = any> {
4646
totalSteps?: number;
4747
canGoBack?: boolean;
4848
items: T[];
49-
activeItem?: T;
49+
activeItem?: T | Promise<T>;
5050
placeholder: string;
5151
customButtonSetups?: QuickInputButtonSetup[];
5252
matchOnDescription?: boolean;
@@ -127,29 +127,45 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
127127
initialize,
128128
}: P): Promise<MultiStepInputQuickPicResponseType<T, P>> {
129129
const disposables: Disposable[] = [];
130+
const input = this.shell.createQuickPick<T>();
131+
input.title = title;
132+
input.step = step;
133+
input.sortByLabel = sortByLabel || false;
134+
input.totalSteps = totalSteps;
135+
input.placeholder = placeholder;
136+
input.ignoreFocusOut = true;
137+
input.items = items;
138+
input.matchOnDescription = matchOnDescription || false;
139+
input.matchOnDetail = matchOnDetail || false;
140+
input.buttons = this.steps.length > 1 ? [QuickInputButtons.Back] : [];
141+
if (customButtonSetups) {
142+
for (const customButtonSetup of customButtonSetups) {
143+
input.buttons = [...input.buttons, customButtonSetup.button];
144+
}
145+
}
146+
if (this.current) {
147+
this.current.dispose();
148+
}
149+
this.current = input;
150+
if (onChangeItem) {
151+
disposables.push(onChangeItem.event((e) => onChangeItem.callback(e, input)));
152+
}
153+
// Quickpick should be initialized synchronously and on changed item handlers are registered synchronously.
154+
if (initialize) {
155+
initialize();
156+
}
157+
if (activeItem) {
158+
input.activeItems = [await activeItem];
159+
} else {
160+
input.activeItems = [];
161+
}
162+
this.current.show();
163+
// Keep scroll position is only meant to keep scroll position when updating items,
164+
// so do it after initialization. This ensures quickpick starts with the active
165+
// item in focus when this is true, instead of having scroll position at top.
166+
input.keepScrollPosition = keepScrollPosition;
130167
try {
131168
return await new Promise<MultiStepInputQuickPicResponseType<T, P>>((resolve, reject) => {
132-
const input = this.shell.createQuickPick<T>();
133-
input.title = title;
134-
input.step = step;
135-
input.sortByLabel = sortByLabel || false;
136-
input.totalSteps = totalSteps;
137-
input.placeholder = placeholder;
138-
input.ignoreFocusOut = true;
139-
input.items = items;
140-
input.matchOnDescription = matchOnDescription || false;
141-
input.matchOnDetail = matchOnDetail || false;
142-
if (activeItem) {
143-
input.activeItems = [activeItem];
144-
} else {
145-
input.activeItems = [];
146-
}
147-
input.buttons = this.steps.length > 1 ? [QuickInputButtons.Back] : [];
148-
if (customButtonSetups) {
149-
for (const customButtonSetup of customButtonSetups) {
150-
input.buttons = [...input.buttons, customButtonSetup.button];
151-
}
152-
}
153169
disposables.push(
154170
input.onDidTriggerButton(async (item) => {
155171
if (item === QuickInputButtons.Back) {
@@ -176,21 +192,6 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
176192
}),
177193
);
178194
}
179-
if (this.current) {
180-
this.current.dispose();
181-
}
182-
this.current = input;
183-
if (onChangeItem) {
184-
disposables.push(onChangeItem.event((e) => onChangeItem.callback(e, input)));
185-
}
186-
this.current.show();
187-
if (initialize) {
188-
initialize();
189-
}
190-
// Keep scroll position is only meant to keep scroll position when updating items,
191-
// so do it after initialization. This ensures quickpick starts with the active
192-
// item in focus when this is true, instead of having scroll position at top.
193-
input.keepScrollPosition = keepScrollPosition;
194195
});
195196
} finally {
196197
disposables.forEach((d) => d.dispose());

‎src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
145145
items: suggestions,
146146
sortByLabel: !preserveOrderWhenFiltering,
147147
keepScrollPosition: true,
148-
activeItem: await this.getActiveItem(state.workspace, suggestions),
148+
activeItem: this.getActiveItem(state.workspace, suggestions), // Use a promise here to ensure quickpick is initialized synchronously.
149149
matchOnDetail: true,
150150
matchOnDescription: true,
151151
title: InterpreterQuickPickList.browsePath.openButtonLabel,

‎src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@ suite('Set Interpreter Command', () => {
244244
const expectedParameters: IQuickPickParameters<QuickPickItem> = {
245245
placeholder: `Selected Interpreter: ${currentPythonPath}`,
246246
items: suggestions,
247-
activeItem: recommended,
248247
matchOnDetail: true,
249248
matchOnDescription: true,
250249
title: InterpreterQuickPickList.browsePath.openButtonLabel,
@@ -267,6 +266,9 @@ suite('Set Interpreter Command', () => {
267266
delete actualParameters!.initialize;
268267
delete actualParameters!.customButtonSetups;
269268
delete actualParameters!.onChangeItem;
269+
const activeItem = await actualParameters!.activeItem;
270+
assert.deepStrictEqual(activeItem, recommended);
271+
delete actualParameters!.activeItem;
270272
assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal');
271273
});
272274

@@ -281,7 +283,6 @@ suite('Set Interpreter Command', () => {
281283
const expectedParameters: IQuickPickParameters<QuickPickItem> = {
282284
placeholder: `Selected Interpreter: ${currentPythonPath}`,
283285
items: suggestions, // Verify suggestions
284-
activeItem: noPythonInstalled, // Verify active item
285286
matchOnDetail: true,
286287
matchOnDescription: true,
287288
title: InterpreterQuickPickList.browsePath.openButtonLabel,
@@ -308,6 +309,9 @@ suite('Set Interpreter Command', () => {
308309
delete actualParameters!.initialize;
309310
delete actualParameters!.customButtonSetups;
310311
delete actualParameters!.onChangeItem;
312+
const activeItem = await actualParameters!.activeItem;
313+
assert.deepStrictEqual(activeItem, noPythonInstalled);
314+
delete actualParameters!.activeItem;
311315
assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal');
312316
});
313317

@@ -525,7 +529,6 @@ suite('Set Interpreter Command', () => {
525529
const expectedParameters: IQuickPickParameters<QuickPickItem> = {
526530
placeholder: `Selected Interpreter: ${currentPythonPath}`,
527531
items: suggestions,
528-
activeItem: recommended,
529532
matchOnDetail: true,
530533
matchOnDescription: true,
531534
title: InterpreterQuickPickList.browsePath.openButtonLabel,
@@ -549,6 +552,9 @@ suite('Set Interpreter Command', () => {
549552
delete actualParameters!.initialize;
550553
delete actualParameters!.customButtonSetups;
551554
delete actualParameters!.onChangeItem;
555+
const activeItem = await actualParameters!.activeItem;
556+
assert.deepStrictEqual(activeItem, recommended);
557+
delete actualParameters!.activeItem;
552558

553559
assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal');
554560
});

0 commit comments

Comments
 (0)