Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Categorize kernels in the notebook kernel picker #135502

Merged
merged 6 commits into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/vs/vscode.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1566,6 +1566,10 @@ declare module 'vscode' {
}

export interface NotebookController {
/**
* The human-readable label used to categorise controllers.
*/
kind?: string;

DonJayamanne marked this conversation as resolved.
Show resolved Hide resolved
// todo@API allow add, not remove
readonly rendererScripts: NotebookRendererScript[];
Expand Down
6 changes: 6 additions & 0 deletions src/vs/workbench/api/browser/mainThreadNotebookKernels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ abstract class MainThreadKernel implements INotebookKernel {
label: string;
description?: string;
detail?: string;
kind?: string;
supportedLanguages: string[];
implementsExecutionOrder: boolean;
localResourceRoot: URI;
Expand All @@ -54,6 +55,7 @@ abstract class MainThreadKernel implements INotebookKernel {
this.label = data.label;
this.description = data.description;
this.detail = data.detail;
this.kind = data.kind;
this.supportedLanguages = isNonEmptyArray(data.supportedLanguages) ? data.supportedLanguages : _modeService.getRegisteredModes();
this.implementsExecutionOrder = data.supportsExecutionOrder ?? false;
this.localResourceRoot = URI.revive(data.extensionLocation);
Expand All @@ -76,6 +78,10 @@ abstract class MainThreadKernel implements INotebookKernel {
this.detail = data.detail;
event.detail = true;
}
if (data.kind !== undefined) {
this.kind = data.kind;
event.kind = true;
}
if (data.supportedLanguages !== undefined) {
this.supportedLanguages = isNonEmptyArray(data.supportedLanguages) ? data.supportedLanguages : this._modeService.getRegisteredModes();
event.supportedLanguages = true;
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,7 @@ export interface INotebookKernelDto2 {
label: string;
detail?: string;
description?: string;
kind?: string;
supportedLanguages?: string[];
supportsInterrupt?: boolean;
supportsExecutionOrder?: boolean;
Expand Down
9 changes: 9 additions & 0 deletions src/vs/workbench/api/common/extHostNotebookKernels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape {
data.description = value;
_update();
},
get kind() {
checkProposedApiEnabled(extension);
return data.kind ?? '';
},
DonJayamanne marked this conversation as resolved.
Show resolved Hide resolved
set kind(value) {
checkProposedApiEnabled(extension);
data.kind = value;
_update();
},
get supportedLanguages() {
return data.supportedLanguages;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ export class InteractiveEditor extends EditorPane {
}

const info = this.#notebookKernelService.getMatchingKernel(notebook);
const selectedOrSuggested = info.selected ?? info.suggested;
const selectedOrSuggested = info.selected ?? info.suggestions[0];

if (selectedOrSuggested) {
const language = selectedOrSuggested.supportedLanguages[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { groupBy } from 'vs/base/common/arrays';
import { Disposable, DisposableStore, IDisposable, MutableDisposable } from 'vs/base/common/lifecycle';
import { Schemas } from 'vs/base/common/network';
import { uppercaseFirstLetter } from 'vs/base/common/strings';
import { compareIgnoreCase, uppercaseFirstLetter } from 'vs/base/common/strings';
import { HoverProviderRegistry } from 'vs/editor/common/modes';
import * as nls from 'vs/nls';
import { Action2, MenuId, registerAction2 } from 'vs/platform/actions/common/actions';
Expand All @@ -14,7 +15,7 @@ import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
import { ILabelService } from 'vs/platform/label/common/label';
import { ILogService } from 'vs/platform/log/common/log';
import { IQuickInputButton, IQuickInputService, IQuickPickItem } from 'vs/platform/quickinput/common/quickInput';
import { IQuickInputButton, IQuickInputService, IQuickPickItem, QuickPickInput } from 'vs/platform/quickinput/common/quickInput';
import { Registry } from 'vs/platform/registry/common/platform';
import { ThemeIcon } from 'vs/platform/theme/common/themeService';
import type { SelectKernelReturnArgs } from 'vs/workbench/api/common/extHostNotebookKernels';
Expand Down Expand Up @@ -128,7 +129,7 @@ registerAction2(class extends Action2 {
}

const notebook = editor.textModel;
const { selected, all } = notebookKernelService.getMatchingKernel(notebook);
const { selected, all, suggestions } = notebookKernelService.getMatchingKernel(notebook);

if (selected && controllerId && selected.id === controllerId && ExtensionIdentifier.equals(selected.extension, extensionId)) {
// current kernel is wanted kernel -> done
Expand Down Expand Up @@ -156,7 +157,7 @@ registerAction2(class extends Action2 {
iconClass: ThemeIcon.asClassName(configureKernelIcon),
tooltip: nls.localize('notebook.promptKernel.setDefaultTooltip', "Set as default for '{0}' notebooks", editor.textModel.viewType)
};
const picks: (KernelPick | IQuickPickItem)[] = all.map(kernel => {
function toQuickPick(kernel: INotebookKernel) {
const res = <KernelPick>{
kernel,
picked: kernel.id === selected?.id,
Expand All @@ -172,16 +173,38 @@ registerAction2(class extends Action2 {
res.description = nls.localize('current2', "{0} - Currently Selected", res.description);
}
}
{ return res; }
});
return res;
}
const quickPickItems: QuickPickInput<IQuickPickItem | KernelPick>[] = [];
if (!all.length) {
picks.push({
quickPickItems.push({
id: 'install',
label: nls.localize('installKernels', "Install kernels from the marketplace"),
});
} else {
// Always display suggested kernels on the top.
if (suggestions.length) {
quickPickItems.push({
type: 'separator',
label: nls.localize('suggestedKernels', "Suggested")
DonJayamanne marked this conversation as resolved.
Show resolved Hide resolved
});
quickPickItems.push(...suggestions.map(toQuickPick));
}

// Next display all of the kernels grouped by categories or extensions.
// If we don't have a kind, always display those at the bottom.
const picks = all.filter(item => item !== selected && !suggestions.includes(item)).map(toQuickPick);
const kernelsPerCategory = groupBy(picks, (a, b) => compareIgnoreCase(a.kernel.kind || 'z', b.kernel.kind || 'z'));
DonJayamanne marked this conversation as resolved.
Show resolved Hide resolved
kernelsPerCategory.forEach(items => {
quickPickItems.push({
type: 'separator',
label: items[0].kernel.kind || nls.localize('otherKernelKinds', "Other")
});
quickPickItems.push(...items);
});
}

const pick = await quickInputService.pick(picks, {
const pick = await quickInputService.pick(quickPickItems, {
placeHolder: selected
? nls.localize('prompt.placeholder.change', "Change kernel for '{0}'", labelService.getUriLabel(notebook.uri, { relative: true }))
: nls.localize('prompt.placeholder.select', "Select kernel for '{0}'", labelService.getUriLabel(notebook.uri, { relative: true })),
Expand Down Expand Up @@ -323,7 +346,8 @@ export class KernelStatus extends Disposable implements IWorkbenchContribution {

this._kernelInfoElement.clear();

let { selected, suggested, all } = this._notebookKernelService.getMatchingKernel(notebook);
let { selected, suggestions, all } = this._notebookKernelService.getMatchingKernel(notebook);
const suggested = suggestions[0];
let isSuggested = false;

if (all.length === 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ CommandsRegistry.registerCommand('_resolveNotebookKernels', async (accessor, arg
return kernels.all.map(provider => ({
id: provider.id,
label: provider.label,
kind: provider.kind,
description: provider.description,
detail: provider.detail,
isPreferred: false, // todo@jrieken,@rebornix
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class NotebookEditorKernelManager extends Disposable {
getSelectedOrSuggestedKernel(notebook: INotebookTextModel): INotebookKernel | undefined {
// returns SELECTED or the ONLY available kernel
const info = this._notebookKernelService.getMatchingKernel(notebook);
return info.selected ?? info.suggested;
return info.selected ?? info.suggestions[0];
}

async executeNotebookCells(notebook: INotebookTextModel, cells: Iterable<ICellViewModel>): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,18 @@ export class NotebookKernelService extends Disposable implements INotebookKernel
}
}

const all = kernels
.sort((a, b) => b.instanceAffinity - a.instanceAffinity || b.typeAffinity - a.typeAffinity || a.score - b.score || a.kernel.label.localeCompare(b.kernel.label))
.map(obj => obj.kernel);
kernels
.sort((a, b) => b.instanceAffinity - a.instanceAffinity || b.typeAffinity - a.typeAffinity || a.score - b.score || a.kernel.label.localeCompare(b.kernel.label));
const all = kernels.map(obj => obj.kernel);

// bound kernel
const selectedId = this._notebookBindings.get(NotebookTextModelLikeId.str(notebook));
const selected = selectedId ? this._kernels.get(selectedId)?.kernel : undefined;

return { all, selected, suggested: all.length === 1 ? all[0] : undefined };
const suggestions = kernels.filter(item => item.instanceAffinity > 1 && item.kernel !== selected).map(item => item.kernel);
if (!suggestions.length && all.length) {
suggestions.push(all[0]);
}
return { all, selected, suggestions };
DonJayamanne marked this conversation as resolved.
Show resolved Hide resolved
}

// default kernel for notebookType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export class NotebooKernelActionViewItem extends ActionViewItem {
private _updateActionFromKernelInfo(info: INotebookKernelMatchResult): void {

this._action.enabled = true;
const selectedOrSuggested = info.selected ?? info.suggested;
const selectedOrSuggested = info.selected ?? info.suggestions[0];
if (selectedOrSuggested) {
// selected or suggested kernel
this._action.label = selectedOrSuggested.label;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface ISelectedNotebooksChangeEvent {

export interface INotebookKernelMatchResult {
readonly selected: INotebookKernel | undefined;
readonly suggested: INotebookKernel | undefined;
readonly suggestions: INotebookKernel[];
readonly all: INotebookKernel[];
}

Expand All @@ -26,6 +26,7 @@ export interface INotebookKernelChangeEvent {
label?: true;
description?: true;
detail?: true;
kind?: true;
supportedLanguages?: true;
hasExecutionOrder?: true;
}
Expand All @@ -44,6 +45,7 @@ export interface INotebookKernel {
label: string;
description?: string;
detail?: string;
kind?: string;
supportedLanguages: string[];
implementsInterrupt?: boolean;
implementsExecutionOrder?: boolean;
Expand Down