Skip to content

Commit 7b55caa

Browse files
Show interpreter session picker immediately, fetch contributed items after (#14868)
1 parent 830656e commit 7b55caa

2 files changed

Lines changed: 76 additions & 20 deletions

File tree

src/vs/workbench/contrib/languageRuntime/browser/languageRuntimeActions.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,8 +573,6 @@ export const selectNewLanguageRuntime = async (
573573
return items;
574574
};
575575

576-
await fetchContributedItems();
577-
578576
const disposables = new DisposableStore();
579577
const quickPick = disposables.add(quickInputService.createQuickPick<IQuickPickItem>({ useSeparators: true }));
580578
quickPick.title = options?.title || localize('positron.languageRuntime.startSession', 'Start New Interpreter Session');
@@ -706,6 +704,20 @@ export const selectNewLanguageRuntime = async (
706704
}));
707705

708706
quickPick.show();
707+
708+
// Fold in contributed items after show() rather than awaiting them first:
709+
// getItems() is an extension-host RPC that can hang for seconds right after
710+
// a window reload, which would leave the picker invisible until it resolves.
711+
// When startup isn't Complete yet, the onDidChangeRuntimeStartupPhase
712+
// handler above does the fetch instead.
713+
if (languageRuntimeService.startupPhase === RuntimeStartupPhase.Complete) {
714+
fetchContributedItems().then(() => {
715+
// Skip if the user dismissed the picker while the fetch was pending.
716+
if (!disposables.isDisposed) {
717+
rebuildItems();
718+
}
719+
});
720+
}
709721
});
710722
};
711723

src/vs/workbench/contrib/languageRuntime/test/browser/languageRuntimeActions.vitest.ts

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import { ExtensionIdentifier } from '../../../../../platform/extensions/common/extensions.js';
99
import { IQuickInputService, IQuickPickItem, QuickInputHideReason, QuickPickItem } from '../../../../../platform/quickinput/common/quickInput.js';
10-
import { ILanguageRuntimeMetadata, ILanguageRuntimeService, IRuntimePickerContribution, LanguageRuntimeSessionLocation, LanguageRuntimeSessionMode, LanguageRuntimeStartupBehavior, RuntimeStartupPhase } from '../../../../services/languageRuntime/common/languageRuntimeService.js';
10+
import { ILanguageRuntimeMetadata, ILanguageRuntimeService, IRuntimePickerContribution, IRuntimePickerItem, LanguageRuntimeSessionLocation, LanguageRuntimeSessionMode, LanguageRuntimeStartupBehavior, RuntimeStartupPhase } from '../../../../services/languageRuntime/common/languageRuntimeService.js';
1111
import { IRuntimeStartupService } from '../../../../services/runtimeStartup/common/runtimeStartupService.js';
1212
import { stubInterface } from '../../../../../test/vitest/stubInterface.js';
1313
import { TestQuickPick } from '../../../../../test/vitest/testQuickPick.js';
@@ -78,9 +78,11 @@ describe('selectNewLanguageRuntime', () => {
7878
return ctx.instantiationService.invokeFunction(accessor => selectNewLanguageRuntime(accessor, options));
7979
}
8080

81-
// The helper does `await fetchContributedItems()` before setting up the
82-
// picker, so the items array isn't populated until pick.show() is called.
83-
// Poll until that happens before reading items / firing events from tests.
81+
// The helper builds the runtime rows synchronously and calls pick.show()
82+
// immediately; contributed items (from picker contributions) are fetched
83+
// afterwards and folded in via a rebuild, so they may not be present the
84+
// instant show() is called. Poll for show() before reading runtime rows;
85+
// poll again (vi.waitFor) when asserting on contributed items.
8486
async function waitUntilOpened(): Promise<void> {
8587
await vi.waitFor(() => expect(pick.show).toHaveBeenCalled());
8688
}
@@ -99,6 +101,19 @@ describe('selectNewLanguageRuntime', () => {
99101
);
100102
}
101103

104+
function pickItemByLabel(label: string): IQuickPickItem | undefined {
105+
return pick.items.find(
106+
(item): item is IQuickPickItem => item.type !== 'separator' && item.label === label,
107+
);
108+
}
109+
110+
// Contributed items are fetched after show() and folded in via a rebuild,
111+
// so tests must poll for them rather than reading synchronously.
112+
async function waitForItemByLabel(label: string): Promise<IQuickPickItem> {
113+
await vi.waitFor(() => expect(pickItemByLabel(label)).toBeDefined());
114+
return pickItemByLabel(label)!;
115+
}
116+
102117

103118
describe('resolution', () => {
104119
it('resolves undefined when the picker is hidden without acceptance', async () => {
@@ -402,12 +417,9 @@ describe('selectNewLanguageRuntime', () => {
402417
const promise = runPicker();
403418
await waitUntilOpened();
404419

405-
// A contributed item counts as a selectable row, so the empty-state
406-
// placeholder must NOT appear even though there are no runtimes.
407-
const contributedItem = pick.items.find(
408-
(item): item is IQuickPickItem => item.type !== 'separator' && item.label === 'Install Python via uv',
409-
);
410-
expect(contributedItem).toBeDefined();
420+
// A contributed item counts as a selectable row, so once it arrives the
421+
// empty-state placeholder must NOT appear even though there are no runtimes.
422+
await waitForItemByLabel('Install Python via uv');
411423
expect(pick.busy).toBe(false);
412424
expect(pick.placeholder).toBeUndefined();
413425

@@ -417,6 +429,42 @@ describe('selectNewLanguageRuntime', () => {
417429
});
418430

419431
describe('contributed items', () => {
432+
it('opens the picker immediately without waiting for slow contributed items', async () => {
433+
// Regression: getItems() is an extension-host RPC that enumerates
434+
// interpreters and can hang for seconds right after a window reload
435+
// while the extension host is still activating. The picker previously
436+
// awaited it before show(), so a slow RPC left the picker invisible --
437+
// clicking the session button appeared to do nothing. show() must now
438+
// happen up front, with contributed items folded in once they resolve.
439+
const runtimeService = ctx.get(ILanguageRuntimeService);
440+
registerRuntime(makeRuntime({ runtimeId: 'py-1' }));
441+
442+
let resolveItems!: (items: IRuntimePickerItem[]) => void;
443+
const contribution: IRuntimePickerContribution = {
444+
handle: 9,
445+
languageId: 'python',
446+
getItems: vi.fn(() => new Promise<IRuntimePickerItem[]>(resolve => { resolveItems = resolve; })),
447+
onSelect: vi.fn(),
448+
};
449+
ctx.disposables.add(runtimeService.registerPickerContribution(contribution));
450+
451+
const promise = runPicker();
452+
453+
// The picker shows even though getItems() has not resolved: runtimes
454+
// are visible immediately, the pending contributed item is not.
455+
await waitUntilOpened();
456+
expect(contribution.getItems).toHaveBeenCalled();
457+
expect(pickItemById('py-1')).toBeDefined();
458+
expect(pickItemByLabel('Install Python via uv')).toBeUndefined();
459+
460+
// Once the slow RPC resolves, the contributed item folds in.
461+
resolveItems([{ id: 'install-uv', label: 'Install Python via uv' }]);
462+
await waitForItemByLabel('Install Python via uv');
463+
464+
pick.cancel(QuickInputHideReason.Gesture);
465+
await promise;
466+
});
467+
420468
it('resolves the registered runtime and triggers a quiet rediscovery on selection', async () => {
421469
const installedRuntime = makeRuntime({ runtimeId: 'py-installed-by-uv' });
422470
const runtimeService = ctx.get(ILanguageRuntimeService);
@@ -436,8 +484,7 @@ describe('selectNewLanguageRuntime', () => {
436484
const promise = runPicker();
437485
await waitUntilOpened();
438486

439-
const installItem = pick.items
440-
.find((item): item is IQuickPickItem => item.type !== 'separator' && item.label === 'Install Python via uv')!;
487+
const installItem = await waitForItemByLabel('Install Python via uv');
441488
pick.accept(installItem);
442489

443490
await expect(promise).resolves.toEqual(installedRuntime);
@@ -458,8 +505,7 @@ describe('selectNewLanguageRuntime', () => {
458505
const promise = runPicker();
459506
await waitUntilOpened();
460507

461-
const item = pick.items
462-
.find((it): it is IQuickPickItem => it.type !== 'separator' && it.label === 'No-op installer')!;
508+
const item = await waitForItemByLabel('No-op installer');
463509
pick.accept(item);
464510
await expect(promise).resolves.toBeUndefined();
465511
});
@@ -478,8 +524,7 @@ describe('selectNewLanguageRuntime', () => {
478524
const promise = runPicker();
479525
await waitUntilOpened();
480526

481-
const item = pick.items
482-
.find((it): it is IQuickPickItem => it.type !== 'separator' && it.label === 'Failing installer')!;
527+
const item = await waitForItemByLabel('Failing installer');
483528
pick.accept(item);
484529
await expect(promise).resolves.toBeUndefined();
485530
expect(consoleErrorSpy).toHaveBeenCalled();
@@ -507,8 +552,7 @@ describe('selectNewLanguageRuntime', () => {
507552
const promise = runPicker();
508553
await waitUntilOpened();
509554

510-
const labels = pick.items.map(i => i.label);
511-
expect(labels).toContain('Working option');
555+
await waitForItemByLabel('Working option');
512556
expect(consoleErrorSpy).toHaveBeenCalled();
513557

514558
pick.cancel(QuickInputHideReason.Gesture);

0 commit comments

Comments
 (0)