Skip to content

Commit 2f4c796

Browse files
authored
Separate select kernel command and quick pick logic. (microsoft#166739)
1 parent e7a78e0 commit 2f4c796

File tree

2 files changed

+87
-82
lines changed

2 files changed

+87
-82
lines changed

src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelQuickPickStrategy.ts

Lines changed: 36 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,36 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import 'vs/css!./notebookKernelActionViewItem';
6+
import { IAction } from 'vs/base/common/actions';
77
import { groupBy } from 'vs/base/common/arrays';
88
import { createCancelablePromise } from 'vs/base/common/async';
99
import { CancellationToken } from 'vs/base/common/cancellation';
1010
import { Codicon } from 'vs/base/common/codicons';
1111
import { Event } from 'vs/base/common/event';
12+
import { DisposableStore } from 'vs/base/common/lifecycle';
13+
import { MarshalledId } from 'vs/base/common/marshallingIds';
1214
import { compareIgnoreCase, uppercaseFirstLetter } from 'vs/base/common/strings';
15+
import 'vs/css!./notebookKernelActionViewItem';
16+
import { Command } from 'vs/editor/common/languages';
1317
import { localize } from 'vs/nls';
18+
import { MenuItemAction } from 'vs/platform/actions/common/actions';
19+
import { ICommandService } from 'vs/platform/commands/common/commands';
1420
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
15-
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
1621
import { ILabelService } from 'vs/platform/label/common/label';
1722
import { ILogService } from 'vs/platform/log/common/log';
1823
import { IProductService } from 'vs/platform/product/common/productService';
1924
import { ProgressLocation } from 'vs/platform/progress/common/progress';
2025
import { IQuickInputService, IQuickPick, IQuickPickItem, QuickPickInput } from 'vs/platform/quickinput/common/quickInput';
26+
import { ThemeIcon } from 'vs/platform/theme/common/themeService';
2127
import { ViewContainerLocation } from 'vs/workbench/common/views';
2228
import { IExtensionsViewPaneContainer, IExtensionsWorkbenchService, VIEWLET_ID as EXTENSION_VIEWLET_ID } from 'vs/workbench/contrib/extensions/common/extensions';
23-
import { getNotebookEditorFromEditorPane, INotebookEditor, INotebookExtensionRecommendation, JUPYTER_EXTENSION_ID, KERNEL_RECOMMENDATIONS } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
29+
import { IActiveNotebookEditor, INotebookExtensionRecommendation, JUPYTER_EXTENSION_ID, KERNEL_RECOMMENDATIONS } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
2430
import { NotebookEditorWidget } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget';
31+
import { executingStateIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons';
2532
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
2633
import { INotebookKernel, INotebookKernelHistoryService, INotebookKernelMatchResult, INotebookKernelService, ISourceAction } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
27-
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
2834
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
2935
import { IPaneCompositePartService } from 'vs/workbench/services/panecomposite/browser/panecomposite';
30-
import { Command } from 'vs/editor/common/languages';
31-
import { ICommandService } from 'vs/platform/commands/common/commands';
32-
import { DisposableStore } from 'vs/base/common/lifecycle';
33-
import { MarshalledId } from 'vs/base/common/marshallingIds';
34-
import { IAction } from 'vs/base/common/actions';
35-
import { ThemeIcon } from 'vs/platform/theme/common/themeService';
36-
import { executingStateIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons';
37-
import { MenuItemAction } from 'vs/platform/actions/common/actions';
3836

3937
type KernelPick = IQuickPickItem & { kernel: INotebookKernel };
4038
function isKernelPick(item: QuickPickInput<IQuickPickItem>): item is KernelPick {
@@ -49,6 +47,9 @@ function isInstallExtensionPick(item: QuickPickInput<IQuickPickItem>): item is I
4947
return item.id === 'installSuggested' && 'extensionId' in item;
5048
}
5149
type KernelSourceQuickPickItem = IQuickPickItem & { command: Command };
50+
function isKernelSourceQuickPickItem(item: IQuickPickItem): item is KernelSourceQuickPickItem {
51+
return 'command' in item;
52+
}
5253
type KernelQuickPickItem = IQuickPickItem | InstallExtensionPick | KernelPick | SourcePick | KernelSourceQuickPickItem;
5354
const KERNEL_PICKER_UPDATE_DEBOUNCE = 200;
5455

@@ -59,25 +60,7 @@ export type KernelQuickPickContext =
5960
{ ui?: boolean; notebookEditor?: NotebookEditorWidget };
6061

6162
export interface IKernelPickerStrategy {
62-
showQuickPick(context?: KernelQuickPickContext): Promise<boolean>;
63-
}
64-
65-
function getEditorFromContext(editorService: IEditorService, context?: KernelQuickPickContext): INotebookEditor | undefined {
66-
let editor: INotebookEditor | undefined;
67-
if (context !== undefined && 'notebookEditorId' in context) {
68-
const editorId = context.notebookEditorId;
69-
const matchingEditor = editorService.visibleEditorPanes.find((editorPane) => {
70-
const notebookEditor = getNotebookEditorFromEditorPane(editorPane);
71-
return notebookEditor?.getId() === editorId;
72-
});
73-
editor = getNotebookEditorFromEditorPane(matchingEditor);
74-
} else if (context !== undefined && 'notebookEditor' in context) {
75-
editor = context?.notebookEditor;
76-
} else {
77-
editor = getNotebookEditorFromEditorPane(editorService.activeEditorPane);
78-
}
79-
80-
return editor;
63+
showQuickPick(editor: IActiveNotebookEditor, wantedKernelId?: string): Promise<boolean>;
8164
}
8265

8366
function toQuickPick(kernel: INotebookKernel, selected: INotebookKernel | undefined) {
@@ -102,7 +85,6 @@ function toQuickPick(kernel: INotebookKernel, selected: INotebookKernel | undefi
10285
abstract class KernelPickerStrategyBase implements IKernelPickerStrategy {
10386
constructor(
10487
protected readonly _notebookKernelService: INotebookKernelService,
105-
protected readonly _editorService: IEditorService,
10688
protected readonly _productService: IProductService,
10789
protected readonly _quickInputService: IQuickInputService,
10890
protected readonly _labelService: ILabelService,
@@ -113,34 +95,14 @@ abstract class KernelPickerStrategyBase implements IKernelPickerStrategy {
11395
protected readonly _commandService: ICommandService
11496
) { }
11597

116-
async showQuickPick(context?: KernelQuickPickContext): Promise<boolean> {
117-
const editor = getEditorFromContext(this._editorService, context);
118-
119-
if (!editor || !editor.hasModel()) {
120-
return false;
121-
}
122-
let controllerId = context && 'id' in context ? context.id : undefined;
123-
let extensionId = context && 'extension' in context ? context.extension : undefined;
124-
125-
if (controllerId && (typeof controllerId !== 'string' || typeof extensionId !== 'string')) {
126-
// validate context: id & extension MUST be strings
127-
controllerId = undefined;
128-
extensionId = undefined;
129-
}
130-
98+
async showQuickPick(editor: IActiveNotebookEditor, wantedId?: string): Promise<boolean> {
13199
const notebook = editor.textModel;
132100
const scopedContextKeyService = editor.scopedContextKeyService;
133101
const matchResult = this._getMatchingResult(notebook);
134102
const { selected, all } = matchResult;
135103

136-
if (selected && controllerId && selected.id === controllerId && ExtensionIdentifier.equals(selected.extension, extensionId)) {
137-
// current kernel is wanted kernel -> done
138-
return true;
139-
}
140-
141104
let newKernel: INotebookKernel | undefined;
142-
if (controllerId) {
143-
const wantedId = `${extensionId}/${controllerId}`;
105+
if (wantedId) {
144106
for (const candidate of all) {
145107
if (candidate.id === wantedId) {
146108
newKernel = candidate;
@@ -239,7 +201,7 @@ abstract class KernelPickerStrategyBase implements IKernelPickerStrategy {
239201
});
240202

241203
if (pick) {
242-
return await this._handleQuickPick(notebook, pick, context, editor.scopedContextKeyService);
204+
return await this._handleQuickPick(editor, pick);
243205
}
244206

245207
return false;
@@ -256,10 +218,10 @@ abstract class KernelPickerStrategyBase implements IKernelPickerStrategy {
256218
scopedContextKeyService: IContextKeyService
257219
): QuickPickInput<KernelQuickPickItem>[];
258220

259-
protected async _handleQuickPick(notebook: NotebookTextModel, pick: KernelQuickPickItem, context?: KernelQuickPickContext, contextKeyService?: IContextKeyService) {
221+
protected async _handleQuickPick(editor: IActiveNotebookEditor, pick: KernelQuickPickItem) {
260222
if (isKernelPick(pick)) {
261223
const newKernel = pick.kernel;
262-
this._selecteKernel(notebook, newKernel);
224+
this._selecteKernel(editor.textModel, newKernel);
263225
return true;
264226
}
265227

@@ -269,15 +231,15 @@ abstract class KernelPickerStrategyBase implements IKernelPickerStrategy {
269231
this._paneCompositePartService,
270232
this._extensionWorkbenchService,
271233
this._extensionService,
272-
notebook.viewType
234+
editor.textModel.viewType
273235
);
274236
// suggestedExtension must be defined for this option to be shown, but still check to make TS happy
275237
} else if (isInstallExtensionPick(pick)) {
276238
await this._showKernelExtension(
277239
this._paneCompositePartService,
278240
this._extensionWorkbenchService,
279241
this._extensionService,
280-
notebook.viewType,
242+
editor.textModel.viewType,
281243
pick.extensionId,
282244
this._productService.quality !== 'stable'
283245
);
@@ -417,7 +379,6 @@ export class KernelPickerFlatStrategy extends KernelPickerStrategyBase {
417379

418380
constructor(
419381
@INotebookKernelService _notebookKernelService: INotebookKernelService,
420-
@IEditorService _editorService: IEditorService,
421382
@IProductService _productService: IProductService,
422383
@IQuickInputService _quickInputService: IQuickInputService,
423384
@ILabelService _labelService: ILabelService,
@@ -430,7 +391,6 @@ export class KernelPickerFlatStrategy extends KernelPickerStrategyBase {
430391
) {
431392
super(
432393
_notebookKernelService,
433-
_editorService,
434394
_productService,
435395
_quickInputService,
436396
_labelService,
@@ -475,15 +435,15 @@ export class KernelPickerFlatStrategy extends KernelPickerStrategyBase {
475435
// label: localize('sourceActions', "")
476436
});
477437

478-
sourceActions.forEach(sourceAction => {
479-
const res = <SourcePick>{
438+
for (const sourceAction of sourceActions) {
439+
const res: SourcePick = {
480440
action: sourceAction,
481441
picked: false,
482442
label: sourceAction.action.label,
483443
};
484444

485445
quickPickItems.push(res);
486-
});
446+
}
487447
}
488448

489449
return quickPickItems;
@@ -575,7 +535,6 @@ export class KernelPickerFlatStrategy extends KernelPickerStrategyBase {
575535
export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
576536
constructor(
577537
@INotebookKernelService _notebookKernelService: INotebookKernelService,
578-
@IEditorService _editorService: IEditorService,
579538
@IProductService _productService: IProductService,
580539
@IQuickInputService _quickInputService: IQuickInputService,
581540
@ILabelService _labelService: ILabelService,
@@ -589,7 +548,6 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
589548
) {
590549
super(
591550
_notebookKernelService,
592-
_editorService,
593551
_productService,
594552
_quickInputService,
595553
_labelService,
@@ -659,15 +617,16 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
659617
};
660618
}
661619

662-
protected override async _handleQuickPick(notebook: NotebookTextModel, pick: KernelQuickPickItem, context?: KernelQuickPickContext, contextKeyService?: IContextKeyService): Promise<boolean> {
620+
protected override async _handleQuickPick(editor: IActiveNotebookEditor, pick: KernelQuickPickItem): Promise<boolean> {
663621
if (pick.id === 'selectAnother') {
664-
return this.displaySelectAnotherQuickPick(notebook, context);
622+
return this.displaySelectAnotherQuickPick(editor);
665623
}
666624

667-
return super._handleQuickPick(notebook, pick, context);
625+
return super._handleQuickPick(editor, pick);
668626
}
669627

670-
private async displaySelectAnotherQuickPick(notebook: NotebookTextModel, context?: KernelQuickPickContext, contextKeyService?: IContextKeyService) {
628+
private async displaySelectAnotherQuickPick(editor: IActiveNotebookEditor) {
629+
const notebook: NotebookTextModel = editor.textModel;
671630
const disposables = new DisposableStore();
672631
return new Promise<boolean>(resolve => {
673632
// select from kernel sources
@@ -681,14 +640,14 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
681640
disposables.add(quickPick.onDidTriggerButton(button => {
682641
if (button === this._quickInputService.backButton) {
683642
quickPick.hide();
684-
resolve(this.showQuickPick(context));
643+
resolve(this.showQuickPick(editor));
685644
}
686645
}));
687646
disposables.add(quickPick.onDidAccept(async () => {
688647
quickPick.hide();
689648
quickPick.dispose();
690649
if (quickPick.selectedItems) {
691-
if ('command' in quickPick.selectedItems[0]) {
650+
if (isKernelSourceQuickPickItem(quickPick.selectedItems[0])) {
692651
const selectedKernelId = await this._executeCommand<string>(notebook, quickPick.selectedItems[0].command);
693652
if (selectedKernelId) {
694653
const { all } = await this._getMatchingResult(notebook);
@@ -699,7 +658,7 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
699658
}
700659
resolve(true);
701660
} else {
702-
return resolve(this.displaySelectAnotherQuickPick(notebook, context, contextKeyService));
661+
return resolve(this.displaySelectAnotherQuickPick(editor));
703662
}
704663
} else if (isKernelPick(quickPick.selectedItems[0])) {
705664
await this._selecteKernel(notebook, quickPick.selectedItems[0].kernel);
@@ -732,17 +691,17 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
732691
};
733692
}));
734693

735-
const sourceActionCommands = this._notebookKernelService.getSourceActions(notebook, contextKeyService);
736-
sourceActionCommands.forEach(sourceAction => {
737-
const res = <SourcePick>{
694+
const sourceActionCommands = this._notebookKernelService.getSourceActions(notebook, editor.scopedContextKeyService);
695+
for (const sourceAction of sourceActionCommands) {
696+
const res: SourcePick = {
738697
action: sourceAction,
739698
picked: false,
740699
label: sourceAction.action.label,
741700
detail: (sourceAction.action as MenuItemAction)?.item?.source
742701
};
743702

744703
quickPickItems.push(res);
745-
});
704+
}
746705

747706
quickPick.items = quickPickItems;
748707
});

src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelView.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,37 @@ import { Action, IAction } from 'vs/base/common/actions';
99
import { Event } from 'vs/base/common/event';
1010
import { localize } from 'vs/nls';
1111
import { Action2, MenuId, registerAction2 } from 'vs/platform/actions/common/actions';
12+
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
1213
import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
14+
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
1315
import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
1416
import { ThemeIcon } from 'vs/platform/theme/common/themeService';
1517
import { NOTEBOOK_ACTIONS_CATEGORY, SELECT_KERNEL_ID } from 'vs/workbench/contrib/notebook/browser/controller/coreActions';
16-
import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
18+
import { getNotebookEditorFromEditorPane, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
1719
import { selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons';
20+
import { KernelPickerFlatStrategy, KernelPickerMRUStrategy, KernelQuickPickContext } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookKernelQuickPickStrategy';
1821
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
1922
import { NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_KERNEL_COUNT } from 'vs/workbench/contrib/notebook/common/notebookContextKeys';
2023
import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
21-
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
22-
import { KernelPickerFlatStrategy, KernelPickerMRUStrategy, KernelQuickPickContext } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookKernelQuickPickStrategy';
24+
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
25+
26+
function getEditorFromContext(editorService: IEditorService, context?: KernelQuickPickContext): INotebookEditor | undefined {
27+
let editor: INotebookEditor | undefined;
28+
if (context !== undefined && 'notebookEditorId' in context) {
29+
const editorId = context.notebookEditorId;
30+
const matchingEditor = editorService.visibleEditorPanes.find((editorPane) => {
31+
const notebookEditor = getNotebookEditorFromEditorPane(editorPane);
32+
return notebookEditor?.getId() === editorId;
33+
});
34+
editor = getNotebookEditorFromEditorPane(matchingEditor);
35+
} else if (context !== undefined && 'notebookEditor' in context) {
36+
editor = context?.notebookEditor;
37+
} else {
38+
editor = getNotebookEditorFromEditorPane(editorService.activeEditorPane);
39+
}
2340

41+
return editor;
42+
}
2443

2544
registerAction2(class extends Action2 {
2645
constructor() {
@@ -79,15 +98,42 @@ registerAction2(class extends Action2 {
7998
async run(accessor: ServicesAccessor, context?: KernelQuickPickContext): Promise<boolean> {
8099
const instantiationService = accessor.get(IInstantiationService);
81100
const configurationService = accessor.get(IConfigurationService);
101+
const editorService = accessor.get(IEditorService);
102+
103+
const editor = getEditorFromContext(editorService, context);
104+
105+
if (!editor || !editor.hasModel()) {
106+
return false;
107+
}
108+
109+
let controllerId = context && 'id' in context ? context.id : undefined;
110+
let extensionId = context && 'extension' in context ? context.extension : undefined;
111+
112+
if (controllerId && (typeof controllerId !== 'string' || typeof extensionId !== 'string')) {
113+
// validate context: id & extension MUST be strings
114+
controllerId = undefined;
115+
extensionId = undefined;
116+
}
117+
118+
const notebook = editor.textModel;
119+
const notebookKernelService = accessor.get(INotebookKernelService);
120+
const matchResult = notebookKernelService.getMatchingKernel(notebook);
121+
const { selected } = matchResult;
122+
123+
if (selected && controllerId && selected.id === controllerId && ExtensionIdentifier.equals(selected.extension, extensionId)) {
124+
// current kernel is wanted kernel -> done
125+
return true;
126+
}
82127

128+
const wantedKernelId = controllerId ? `${extensionId}/${controllerId}` : undefined;
83129
const kernelPickerType = configurationService.getValue<'all' | 'mru'>('notebook.kernelPicker.type');
84130

85131
if (kernelPickerType === 'mru') {
86132
const strategy = instantiationService.createInstance(KernelPickerMRUStrategy);
87-
return await strategy.showQuickPick(context);
133+
return await strategy.showQuickPick(editor, wantedKernelId);
88134
} else {
89135
const strategy = instantiationService.createInstance(KernelPickerFlatStrategy);
90-
return await strategy.showQuickPick(context);
136+
return await strategy.showQuickPick(editor, wantedKernelId);
91137
}
92138
}
93139
});

0 commit comments

Comments
 (0)