diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index 95c87645843..8306de1f23a 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -177,7 +177,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape // --- kernel adding/updating/removal - $addKernel(handle: number, data: INotebookKernelDto2): void { + async $addKernel(handle: number, data: INotebookKernelDto2): Promise { const that = this; const kernel = new class extends MainThreadKernel { executeNotebookCellsRequest(uri: URI, ranges: ICellRange[]): void { diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index cd9fbb04c2e..439e3b6384c 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1107,7 +1107,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I }, createNotebookController(options) { checkProposedApiEnabled(extension); - return extHostNotebookKernels.createKernel(extension, options); + return extHostNotebookKernels.createNotebookController(extension, options); } }; diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index eba9420ea93..e8b9d92c210 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -924,7 +924,7 @@ export interface INotebookKernelDto2 { export interface MainThreadNotebookKernelsShape extends IDisposable { $postMessage(handle: number, editorId: string | undefined, message: any): Promise; - $addKernel(handle: number, data: INotebookKernelDto2): void; + $addKernel(handle: number, data: INotebookKernelDto2): Promise; $updateKernel(handle: number, data: Partial): void; $removeKernel(handle: number): void; } diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index f46bde32244..ce96dbab8d2 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -8,7 +8,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { ExtHostNotebookKernelsShape, IMainContext, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from 'vs/workbench/api/common/extHost.protocol'; import * as vscode from 'vscode'; import { ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebook'; -import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'; +import { ExtensionIdentifier, IExtensionDescription } from 'vs/platform/extensions/common/extensions'; import { URI, UriComponents } from 'vs/base/common/uri'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import * as extHostTypeConverters from 'vs/workbench/api/common/extHostTypeConverters'; @@ -16,8 +16,8 @@ import { isNonEmptyArray } from 'vs/base/common/arrays'; import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService'; import { asWebviewUri } from 'vs/workbench/api/common/shared/webview'; - interface IKernelData { + extensionId: ExtensionIdentifier, controller: vscode.NotebookController; onDidChangeSelection: Emitter<{ selected: boolean; notebook: vscode.NotebookDocument; }>; onDidReceiveMessage: Emitter<{ editor: vscode.NotebookEditor, message: any }>; @@ -38,7 +38,13 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { this._proxy = mainContext.getProxy(MainContext.MainThreadNotebookKernels); } - createKernel(extension: IExtensionDescription, options: vscode.NotebookControllerOptions): vscode.NotebookController { + createNotebookController(extension: IExtensionDescription, options: vscode.NotebookControllerOptions): vscode.NotebookController { + + for (let data of this._kernelData.values()) { + if (data.controller.id === options.id) { + throw new Error(`notebook controller with id '${options.id}' ALREADY exist`); + } + } const handle = this._handlePool++; const that = this; @@ -64,7 +70,11 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { let _interruptHandler = options.interruptHandler; // todo@jrieken the selector needs to be massaged - this._proxy.$addKernel(handle, data); + this._proxy.$addKernel(handle, data).catch(err => { + // this can happen when a kernel with that ID is already registered + console.log(err); + isDisposed = true; + }); // update: all setters write directly into the dto object // and trigger an update. the actual update will only happen @@ -127,6 +137,9 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { get executeHandler() { return _executeHandler; }, + set executeHandler(value) { + _executeHandler = value ?? (() => console.warn(`NO execute handler from notebook controller '${data.id}' of extension: '${extension.identifier}'`)); + }, get interruptHandler() { return _interruptHandler; }, @@ -162,9 +175,10 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { } }; - this._kernelData.set(handle, { controller, onDidChangeSelection, onDidReceiveMessage }); + this._kernelData.set(handle, { extensionId: extension.identifier, controller, onDidChangeSelection, onDidReceiveMessage }); controller.supportedLanguages = options.supportedLanguages ?? []; + controller.executeHandler = options.executeHandler; controller.interruptHandler = options.interruptHandler; controller.hasExecutionOrder = options.hasExecutionOrder ?? false; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts index 6ceaa6b68f6..448dea23ef3 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts @@ -40,7 +40,7 @@ export class NotebookKernelService implements INotebookKernelService { registerKernel(kernel: INotebookKernel2): IDisposable { if (this._kernels.has(kernel.id)) { - throw new Error(`KERNEL with id '${kernel.id}' already exists`); + throw new Error(`NOTEBOOK CONTROLLER with id '${kernel.id}' already exists`); } this._kernels.set(kernel.id, kernel); diff --git a/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts b/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts index 02b4cce8d00..ee38b4fdba5 100644 --- a/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts +++ b/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts @@ -29,7 +29,7 @@ suite('NotebookKernel', function () { override $registerCommand() { } }); rpcProtocol.set(MainContext.MainThreadNotebookKernels, new class extends mock() { - override $addKernel(handle: number, data: INotebookKernelDto2): void { + async override $addKernel(handle: number, data: INotebookKernelDto2): Promise { kernelData.set(handle, data); } override $removeKernel(handle: number) { @@ -50,7 +50,7 @@ suite('NotebookKernel', function () { test('create/dispose kernel', async function () { - const kernel = extHostNotebookKernels.createKernel(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); assert.ok(kernel); assert.strictEqual(kernel.id, 'foo'); @@ -73,7 +73,7 @@ suite('NotebookKernel', function () { test('update kernel', async function () { - const kernel = extHostNotebookKernels.createKernel(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, { id: 'foo', label: 'Foo', selector: '*', executeHandler: () => { }, supportedLanguages: ['plaintext'] }); await rpcProtocol.sync(); assert.ok(kernel);