remove isPreferred as instance property, replace selector with only viewType, allow controller to set a notebook priority instead

This commit is contained in:
Johannes Rieken 2021-04-22 16:13:42 +02:00
parent dab0e063bc
commit 5390ec0e7c
No known key found for this signature in database
GPG key ID: 96634B5AF12F8798
15 changed files with 187 additions and 112 deletions

View file

@ -65,7 +65,6 @@ class Kernel {
constructor(id: string, label: string) {
this.controller = vscode.notebook.createNotebookController(id, 'notebookCoreTest', label);
this.controller.executeHandler = this._execute.bind(this);
this.controller.isPreferred = true;
this.controller.hasExecutionOrder = true;
this.controller.supportedLanguages = ['typescript', 'javascript'];
}
@ -170,12 +169,11 @@ suite('Notebook API tests', function () {
setup(() => {
const kernel1 = new Kernel('mainKernel', 'Notebook Test Kernel');
const kernel1 = new Kernel('mainKernel', 'Notebook Primary Test Kernel');
const kernel2 = new class extends Kernel {
constructor() {
super('secondaryKernel', 'Notebook Secondary Test Kernel');
this.controller.isPreferred = false;
this.controller.hasExecutionOrder = false;
}
@ -767,7 +765,6 @@ suite('Notebook API tests', function () {
constructor() {
super('cancelableKernel', 'Notebook Cancelable Test Kernel');
this.controller.isPreferred = false;
}
override async _execute(cells: vscode.NotebookCell[]) {
@ -813,7 +810,6 @@ suite('Notebook API tests', function () {
constructor() {
super('interruptableKernel', 'Notebook Interruptable Test Kernel');
this.controller.isPreferred = false;
this.controller.interruptHandler = this.interrupt.bind(this);
}
@ -1196,7 +1192,6 @@ suite('Notebook API tests', function () {
constructor() {
super('verifyOutputSyncKernel', '');
this.controller.isPreferred = false;
}
override async _execute(cells: vscode.NotebookCell[]) {

View file

@ -60,11 +60,10 @@ export function activate(context: vscode.ExtensionContext): any {
const controller = vscode.notebook.createNotebookController(
'notebookSmokeTest',
{ pattern: '*.smoke-nb' },
'notebookSmokeTest',
'notebookSmokeTest'
);
controller.isPreferred = true;
controller.executeHandler = (cells) => {
for (const cell of cells) {
const task = controller.createNotebookCellExecutionTask(cell);

View file

@ -1560,7 +1560,13 @@ declare module 'vscode' {
* documents of the type `notebook.test`, whereas `{ pattern: '/my/file/test.nb' }`
* selects only the notebook with the path `/my/file/test.nb`.
*/
readonly selector: NotebookSelector;
readonly viewType: string;
/**
* An array of language identifiers that are supported by this
* controller. When falsy all languages are supported.
*/
supportedLanguages?: string[];
/**
* The human-readable label of this notebook controller.
@ -1577,21 +1583,6 @@ declare module 'vscode' {
*/
detail?: string;
/**
* Marks this notebook controller as preferred.
*
* When multiple notebook controller apply to a single notebook then
* users can select one - preferred controllers will be shows more
* prominent then.
*/
isPreferred?: boolean;
/**
* An array of language identifiers that are supported by this
* controller. When falsy all languages are supported.
*/
supportedLanguages?: string[];
/**
* Whether this controller supports execution order so that the
* editor can render placeholders for them.
@ -1660,6 +1651,20 @@ declare module 'vscode' {
//todo@API validate this works
asWebviewUri(localResource: Uri): Uri;
/**
* A controller can set priorities for specific notebook documents. This allows a controller to be the
* preferred controller for certain notebooks.
*
* @param notebook The notebook for which a priority is set.
* @param priority A controller priority
*/
updateNotebookPriority(notebook: NotebookDocument, priority: NotebookControllerPriority): void;
}
export enum NotebookControllerPriority {
Default = 1,
Preferred = 2
}
export namespace notebook {
@ -1673,7 +1678,7 @@ declare module 'vscode' {
* @param handler
* @param preloads
*/
export function createNotebookController(id: string, selector: NotebookSelector, label: string, handler?: NotebookExecuteHandler, preloads?: NotebookKernelPreload[]): NotebookController;
export function createNotebookController(id: string, viewType: string, label: string, handler?: NotebookExecuteHandler, preloads?: NotebookKernelPreload[]): NotebookController;
}
//#endregion

View file

@ -7,7 +7,7 @@ import { flatten, isNonEmptyArray } from 'vs/base/common/arrays';
import { runWhenIdle } from 'vs/base/common/async';
import { Emitter, Event } from 'vs/base/common/event';
import { combinedDisposable, DisposableStore, IDisposable, MutableDisposable } from 'vs/base/common/lifecycle';
import { URI } from 'vs/base/common/uri';
import { URI, UriComponents } from 'vs/base/common/uri';
import { IModeService } from 'vs/editor/common/services/modeService';
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
@ -15,7 +15,6 @@ import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookB
import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService';
import { INotebookKernel, INotebookKernelChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
import { NotebookSelector } from 'vs/workbench/contrib/notebook/common/notebookSelector';
import { ExtHostContext, ExtHostNotebookKernelsShape, IExtHostContext, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from '../common/extHost.protocol';
abstract class MainThreadKernel implements INotebookKernel {
@ -25,14 +24,13 @@ abstract class MainThreadKernel implements INotebookKernel {
readonly onDidChange: Event<INotebookKernelChangeEvent> = this._onDidChange.event;
readonly id: string;
readonly selector: NotebookSelector;
readonly viewType: string;
readonly extension: ExtensionIdentifier;
implementsInterrupt: boolean;
label: string;
description?: string;
detail?: string;
isPreferred?: boolean;
supportedLanguages: string[];
implementsExecutionOrder: boolean;
localResourceRoot: URI;
@ -47,14 +45,13 @@ abstract class MainThreadKernel implements INotebookKernel {
constructor(data: INotebookKernelDto2, private _modeService: IModeService) {
this.id = data.id;
this.selector = data.selector;
this.viewType = data.viewType;
this.extension = data.extensionId;
this.implementsInterrupt = data.supportsInterrupt ?? false;
this.label = data.label;
this.description = data.description;
this.detail = data.detail;
this.isPreferred = data.isPreferred;
this.supportedLanguages = isNonEmptyArray(data.supportedLanguages) ? data.supportedLanguages : _modeService.getRegisteredModes();
this.implementsExecutionOrder = data.hasExecutionOrder ?? false;
this.localResourceRoot = URI.revive(data.extensionLocation);
@ -77,10 +74,6 @@ abstract class MainThreadKernel implements INotebookKernel {
this.detail = data.detail;
event.detail = true;
}
if (data.isPreferred !== undefined) {
this.isPreferred = data.isPreferred;
event.isPreferred = true;
}
if (data.supportedLanguages !== undefined) {
this.supportedLanguages = isNonEmptyArray(data.supportedLanguages) ? data.supportedLanguages : this._modeService.getRegisteredModes();
event.supportedLanguages = true;
@ -229,4 +222,11 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape
this._kernels.delete(handle);
}
}
$updateNotebookPriority(handle: number, notebook: UriComponents, value: number | undefined): void {
const tuple = this._kernels.get(handle);
if (tuple) {
this._notebookKernelService.updateKernelNotebookPriority(tuple[0], URI.revive(notebook), value);
}
}
}

View file

@ -1101,9 +1101,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
checkProposedApiEnabled(extension);
return extHostNotebook.createNotebookCellExecution(uri, index, kernelId);
},
createNotebookController(id, selector, label, executeHandler, preloads) {
createNotebookController(id, viewType, label, executeHandler, preloads) {
checkProposedApiEnabled(extension);
return extHostNotebookKernels.createNotebookController(extension, id, selector, label, executeHandler, preloads);
return extHostNotebookKernels.createNotebookController(extension, id, viewType, label, executeHandler, preloads);
}
};
@ -1253,6 +1253,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
NotebookCellOutput: extHostTypes.NotebookCellOutput,
NotebookCellOutputItem: extHostTypes.NotebookCellOutputItem,
NotebookCellStatusBarItem: extHostTypes.NotebookCellStatusBarItem,
NotebookControllerPriority: extHostTypes.NotebookControllerPriority,
LinkedEditingRanges: extHostTypes.LinkedEditingRanges,
TestItemStatus: extHostTypes.TestItemStatus,
TestResultState: extHostTypes.TestResultState,

View file

@ -907,13 +907,12 @@ export interface MainThreadNotebookDocumentsShape extends IDisposable {
export interface INotebookKernelDto2 {
id: string;
selector: NotebookSelector;
viewType: string;
extensionId: ExtensionIdentifier;
extensionLocation: UriComponents;
label: string;
detail?: string;
description?: string;
isPreferred?: boolean;
supportedLanguages?: string[];
supportsInterrupt?: boolean;
hasExecutionOrder?: boolean;
@ -925,6 +924,7 @@ export interface MainThreadNotebookKernelsShape extends IDisposable {
$addKernel(handle: number, data: INotebookKernelDto2): Promise<void>;
$updateKernel(handle: number, data: Partial<INotebookKernelDto2>): void;
$removeKernel(handle: number): void;
$updateNotebookPriority(handle: number, uri: UriComponents, value: number | undefined): void;
}
export interface MainThreadUrlsShape extends IDisposable {

View file

@ -36,7 +36,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape {
this._proxy = mainContext.getProxy(MainContext.MainThreadNotebookKernels);
}
createNotebookController(extension: IExtensionDescription, id: string, selector: vscode.NotebookSelector, label: string, handler?: vscode.NotebookExecuteHandler, preloads?: vscode.NotebookKernelPreload[]): vscode.NotebookController {
createNotebookController(extension: IExtensionDescription, id: string, viewType: string, label: string, handler?: vscode.NotebookExecuteHandler, preloads?: vscode.NotebookKernelPreload[]): vscode.NotebookController {
for (let data of this._kernelData.values()) {
if (data.controller.id === id) {
@ -56,8 +56,8 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape {
const onDidReceiveMessage = new Emitter<{ editor: vscode.NotebookEditor, message: any }>();
const data: INotebookKernelDto2 = {
id: id,
selector: selector,
id,
viewType,
extensionId: extension.identifier,
extensionLocation: extension.extensionLocation,
label: label || extension.identifier.value,
@ -93,7 +93,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape {
const controller: vscode.NotebookController = {
get id() { return data.id; },
get selector() { return data.selector; },
get viewType() { return data.viewType; },
onDidChangeNotebookAssociation: onDidChangeSelection.event,
get label() {
return data.label;
@ -116,13 +116,6 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape {
data.description = value;
_update();
},
get isPreferred() {
return data.isPreferred ?? false;
},
set isPreferred(value) {
data.isPreferred = value;
_update();
},
get supportedLanguages() {
return data.supportedLanguages;
},
@ -178,6 +171,10 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape {
},
asWebviewUri(uri: URI) {
return asWebviewUri(that._initData.environment, String(handle), uri);
},
// --- priority
updateNotebookPriority(notebook, priority) {
that._proxy.$updateNotebookPriority(handle, notebook.uri, priority);
}
};

View file

@ -3163,6 +3163,12 @@ export class NotebookCellStatusBarItem {
readonly accessibilityInformation?: vscode.AccessibilityInformation) { }
}
export enum NotebookControllerPriority {
Default = 1,
Preferred = 2
}
//#endregion
//#region Timeline

View file

@ -1759,7 +1759,7 @@ CommandsRegistry.registerCommand('_resolveNotebookKernels', async (accessor, arg
label: provider.label,
description: provider.description,
detail: provider.detail,
isPreferred: provider.isPreferred,
isPreferred: false, // todo@jrieken,@rebornix
preloads: provider.preloadUris,
}));
});

View file

@ -7,13 +7,11 @@ import { Event, Emitter } from 'vs/base/common/event';
import { DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { INotebookKernel, INotebookTextModel } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookKernelBindEvent, INotebookKernelService, INotebookTextModelLike } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
import { score } from 'vs/workbench/contrib/notebook/common/notebookSelector';
import { LRUCache } from 'vs/base/common/map';
import { LRUCache, ResourceMap } from 'vs/base/common/map';
import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
import { URI } from 'vs/base/common/uri';
import { runWhenIdle } from 'vs/base/common/async';
import { ILogService } from 'vs/platform/log/common/log';
import { isEqual } from 'vs/base/common/resources';
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
class KernelInfo {
@ -24,6 +22,8 @@ class KernelInfo {
public score: number;
readonly time: number;
readonly notebookPriorities = new ResourceMap<number>();
constructor(kernel: INotebookKernel) {
this.kernel = kernel;
this.score = -1;
@ -31,28 +31,6 @@ class KernelInfo {
}
}
class ScoreInfo {
private _anchor?: INotebookTextModelLike;
constructor() { }
matches(candidate: INotebookTextModelLike): boolean {
if (!this._anchor) {
return false;
}
if (this._anchor.viewType === candidate.viewType && isEqual(this._anchor.uri, candidate.uri)) {
return true;
}
this._anchor = candidate;
return false;
}
reset(): void {
this._anchor = undefined;
}
}
export class NotebookKernelService implements INotebookKernelService {
declare _serviceBrand: undefined;
@ -62,7 +40,6 @@ export class NotebookKernelService implements INotebookKernelService {
private readonly _disposables = new DisposableStore();
private readonly _kernels = new Map<string, KernelInfo>();
private readonly _kernelBindings = new LRUCache<string, string>(1000, 0.7);
private readonly _scoreInfo = new ScoreInfo();
private readonly _onDidChangeNotebookKernelBinding = new Emitter<INotebookKernelBindEvent>();
private readonly _onDidAddKernel = new Emitter<INotebookKernel>();
@ -128,7 +105,6 @@ export class NotebookKernelService implements INotebookKernelService {
}
this._kernels.set(kernel.id, new KernelInfo(kernel));
this._scoreInfo.reset();
this._onDidAddKernel.fire(kernel);
// auto associate the new kernel to existing notebooks it was
@ -138,7 +114,6 @@ export class NotebookKernelService implements INotebookKernelService {
}
return toDisposable(() => {
this._scoreInfo.reset();
if (this._kernels.delete(kernel.id)) {
this._onDidRemoveKernel.fire(kernel);
}
@ -153,33 +128,17 @@ export class NotebookKernelService implements INotebookKernelService {
getNotebookKernels(notebook: INotebookTextModelLike): { bound: INotebookKernel | undefined, all: INotebookKernel[] } {
// update score if needed
if (!this._scoreInfo.matches(notebook)) {
for (const item of this._kernels.values()) {
item.score = score(item.kernel.selector, notebook.uri, notebook.viewType);
// all applicable kernels
const kernels: { kernel: INotebookKernel, priority: number }[] = [];
for (const info of this._kernels.values()) {
if (info.kernel.viewType === notebook.viewType || info.kernel.viewType === '*') {
kernels.push({ kernel: info.kernel, priority: info.notebookPriorities.get(notebook.uri) ?? 1 /* vscode.NotebookControllerPriority.Default */ });
}
}
// all applicable kernels
const all = Array.from(this._kernels.values())
.filter(item => item.score > 0)
.sort((a, b) => {
// (1) sort by preference
if (a.kernel.isPreferred !== b.kernel.isPreferred) {
if (a.kernel.isPreferred) {
return -1;
} else {
return 1;
}
}
// (2) sort by score
if (b.score !== a.score) {
return b.score - a.score;
}
// (3) sort by time
return a.time - b.time;
})
.map(item => item.kernel);
const all = kernels
.sort((a, b) => b.priority - a.priority || a.kernel.label.localeCompare(b.kernel.label))
.map(obj => obj.kernel);
// bound kernel
const boundId = this._kernelBindings.get(notebook.uri.toString());
@ -203,4 +162,16 @@ export class NotebookKernelService implements INotebookKernelService {
this._persistBindingsSoon();
}
}
updateKernelNotebookPriority(kernel: INotebookKernel, notebook: URI, preference: number | undefined): void {
const info = this._kernels.get(kernel.id);
if (!info) {
throw new Error(`UNKNOWN kernel '${kernel.id}'`);
}
if (preference === undefined) {
info.notebookPriorities.delete(notebook);
} else {
info.notebookPriorities.set(notebook, preference);
}
}
}

View file

@ -758,7 +758,6 @@ export interface INotebookKernelChangeEvent {
label?: true;
description?: true;
detail?: true;
isPreferred?: true;
supportedLanguages?: true;
hasExecutionOrder?: true;
}
@ -766,7 +765,7 @@ export interface INotebookKernelChangeEvent {
export interface INotebookKernel {
readonly id: string;
readonly selector: NotebookSelector;
readonly viewType: string;
readonly onDidChange: Event<Readonly<INotebookKernelChangeEvent>>;
readonly extension: ExtensionIdentifier;
@ -777,7 +776,6 @@ export interface INotebookKernel {
label: string;
description?: string;
detail?: string;
isPreferred?: boolean;
supportedLanguages: string[]
implementsInterrupt?: boolean;
implementsExecutionOrder?: boolean;
@ -848,5 +846,3 @@ export interface INotebookDecorationRenderOptions {
borderColor?: string | ThemeColor;
top?: editorCommon.IContentDecorationRenderOptions;
}

View file

@ -36,4 +36,9 @@ export interface INotebookKernelService {
*/
updateNotebookKernelBinding(notebook: INotebookTextModel, kernel: INotebookKernel | undefined): void;
/**
* Set a perference of a kernel for a certain notebook. Higher values win, `undefined` removes the preference
*/
updateKernelNotebookPriority(kernel: INotebookKernel, notebook: URI, preference: number | undefined): void;
}

View file

@ -133,13 +133,12 @@ suite('NotebookEditorKernelManager', () => {
class TestNotebookKernel implements INotebookKernel {
id: string = 'test';
label: string = '';
selector = '*';
viewType = '*';
onDidChange = Event.None;
extension: ExtensionIdentifier = new ExtensionIdentifier('test');
localResourceRoot: URI = URI.file('/test');
description?: string | undefined;
detail?: string | undefined;
isPreferred?: boolean | undefined;
preloadUris: URI[] = [];
preloadProvides: string[] = [];
supportedLanguages: string[] = [];

View file

@ -0,0 +1,98 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import * as assert from 'assert';
import { URI } from 'vs/base/common/uri';
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
import { INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { setupInstantiationService, withTestNotebook as _withTestNotebook } from 'vs/workbench/contrib/notebook/test/testNotebookEditor';
import { Event } from 'vs/base/common/event';
import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
import { NotebookKernelService } from 'vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl';
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
import { mock } from 'vs/base/test/common/mock';
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
import { DisposableStore } from 'vs/base/common/lifecycle';
suite('NotebookKernelService', () => {
let instantiationService: TestInstantiationService;
let kernelService: INotebookKernelService;
const dispoables = new DisposableStore();
setup(function () {
dispoables.clear();
instantiationService = setupInstantiationService();
instantiationService.stub(INotebookService, new class extends mock<INotebookService>() {
onDidAddNotebookDocument = Event.None;
getNotebookTextModels() { return []; }
});
kernelService = instantiationService.createInstance(NotebookKernelService);
instantiationService.set(INotebookKernelService, kernelService);
});
test('notebook priorities', function () {
const u1 = URI.parse('foo:///one');
const u2 = URI.parse('foo:///two');
const k1 = new TestNotebookKernel({ label: 'z' });
const k2 = new TestNotebookKernel({ label: 'a' });
kernelService.registerKernel(k1);
kernelService.registerKernel(k2);
// equal priorities -> sort by name
let info = kernelService.getNotebookKernels({ uri: u1, viewType: 'foo' });
assert.ok(info.all[0] === k2);
assert.ok(info.all[1] === k1);
// update priorities for u1 notebook
kernelService.updateKernelNotebookPriority(k2, u1, 2);
kernelService.updateKernelNotebookPriority(k2, u2, 1);
// updated
info = kernelService.getNotebookKernels({ uri: u1, viewType: 'foo' });
assert.ok(info.all[0] === k2);
assert.ok(info.all[1] === k1);
// NOT updated
info = kernelService.getNotebookKernels({ uri: u2, viewType: 'foo' });
assert.ok(info.all[0] === k2);
assert.ok(info.all[1] === k1);
// reset
kernelService.updateKernelNotebookPriority(k2, u1, undefined);
info = kernelService.getNotebookKernels({ uri: u1, viewType: 'foo' });
assert.ok(info.all[0] === k2);
assert.ok(info.all[1] === k1);
});
});
class TestNotebookKernel implements INotebookKernel {
id: string = Math.random() + 'kernel';
label: string = '';
viewType = '*';
onDidChange = Event.None;
extension: ExtensionIdentifier = new ExtensionIdentifier('test');
localResourceRoot: URI = URI.file('/test');
description?: string | undefined;
detail?: string | undefined;
preloadUris: URI[] = [];
preloadProvides: string[] = [];
supportedLanguages: string[] = [];
executeNotebookCellsRequest(): Promise<void> {
throw new Error('Method not implemented.');
}
cancelNotebookCellExecution(): Promise<void> {
throw new Error('Method not implemented.');
}
constructor(opts?: { languages?: string[], label?: string }) {
this.supportedLanguages = opts?.languages ?? ['text/plain'];
this.label = opts?.label ?? 'test-label';
}
}

View file

@ -52,10 +52,13 @@ suite('NotebookKernel', function () {
const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, 'foo', '*', 'Foo');
assert.throws(() => (<any>kernel).id = 'dd');
assert.throws(() => (<any>kernel).viewType = 'dd');
assert.ok(kernel);
assert.strictEqual(kernel.id, 'foo');
assert.strictEqual(kernel.label, 'Foo');
assert.strictEqual(kernel.selector, '*');
assert.strictEqual(kernel.viewType, '*');
await rpcProtocol.sync();
assert.strictEqual(kernelData.size, 1);
@ -64,7 +67,7 @@ suite('NotebookKernel', function () {
assert.strictEqual(first.id, 'foo');
assert.strictEqual(ExtensionIdentifier.equals(first.extensionId, nullExtensionDescription.identifier), true);
assert.strictEqual(first.label, 'Foo');
assert.strictEqual(first.selector, '*');
assert.strictEqual(first.viewType, '*');
kernel.dispose();
await rpcProtocol.sync();