diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 685b52b81af..42635c61191 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -155,13 +155,11 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { return [...this._documents.values()]; } - lookupNotebookDocument(uri: URI): ExtHostNotebookDocument | undefined { - return this._documents.get(uri); - } - - private _getNotebookDocument(uri: URI): ExtHostNotebookDocument { + getNotebookDocument(uri: URI, relaxed: true): ExtHostNotebookDocument | undefined; + getNotebookDocument(uri: URI): ExtHostNotebookDocument; + getNotebookDocument(uri: URI, relaxed?: true): ExtHostNotebookDocument | undefined { const result = this._documents.get(uri); - if (!result) { + if (!result && !relaxed) { throw new Error(`NO notebook document for '${uri}'`); } return result; @@ -399,14 +397,14 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { } async $saveNotebook(viewType: string, uri: UriComponents, token: CancellationToken): Promise { - const document = this._getNotebookDocument(URI.revive(uri)); + const document = this.getNotebookDocument(URI.revive(uri)); const { provider } = this._getProviderData(viewType); await provider.saveNotebook(document.apiNotebook, token); return true; } async $saveNotebookAs(viewType: string, uri: UriComponents, target: UriComponents, token: CancellationToken): Promise { - const document = this._getNotebookDocument(URI.revive(uri)); + const document = this.getNotebookDocument(URI.revive(uri)); const { provider } = this._getProviderData(viewType); await provider.saveNotebookAs(URI.revive(target), document.apiNotebook, token); return true; @@ -415,7 +413,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { private _backupIdPool: number = 0; async $backupNotebook(viewType: string, uri: UriComponents, cancellation: CancellationToken): Promise { - const document = this._getNotebookDocument(URI.revive(uri)); + const document = this.getNotebookDocument(URI.revive(uri)); const provider = this._getProviderData(viewType); const storagePath = this._extensionStoragePaths.workspaceValue(provider.extension) ?? this._extensionStoragePaths.globalValue(provider.extension); @@ -428,17 +426,17 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { } $acceptModelChanged(uri: UriComponents, event: NotebookCellsChangedEventDto, isDirty: boolean): void { - const document = this._getNotebookDocument(URI.revive(uri)); + const document = this.getNotebookDocument(URI.revive(uri)); document.acceptModelChanged(event, isDirty); } $acceptDirtyStateChanged(uri: UriComponents, isDirty: boolean): void { - const document = this._getNotebookDocument(URI.revive(uri)); + const document = this.getNotebookDocument(URI.revive(uri)); document.acceptModelChanged({ rawEvents: [], versionId: document.apiNotebook.version }, isDirty); } $acceptModelSaved(uri: UriComponents): void { - const document = this._getNotebookDocument(URI.revive(uri)); + const document = this.getNotebookDocument(URI.revive(uri)); this._onDidSaveNotebookDocument.fire(document.apiNotebook); } @@ -485,7 +483,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { $acceptDocumentPropertiesChanged(uri: UriComponents, data: INotebookDocumentPropertiesChangeData): void { this.logService.debug('ExtHostNotebook#$acceptDocumentPropertiesChanged', uri.path, data); - const document = this._getNotebookDocument(URI.revive(uri)); + const document = this.getNotebookDocument(URI.revive(uri)); document.acceptDocumentPropertiesChanged(data); if (data.metadata) { this._onDidChangeNotebookDocumentMetadata.fire({ document: document.apiNotebook }); @@ -644,23 +642,15 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { this._onDidChangeActiveNotebookEditor.fire(this._activeNotebookEditor?.apiEditor); } } - createNotebookCellExecution(docUri: vscode.Uri, index: number, kernelId: string): vscode.NotebookCellExecutionTask | undefined { - const document = this.lookupNotebookDocument(docUri); - if (!document) { - throw new Error(`Invalid uri: ${docUri} `); - } - const cell = document.getCellFromIndex(index); - if (!cell) { - throw new Error(`Invalid cell index: ${docUri}, ${index} `); - } + createNotebookCellExecution(cell: ExtHostCell): vscode.NotebookCellExecutionTask | undefined { // TODO@roblou also validate kernelId, once kernel has moved from editor to document if (this._activeExecutions.has(cell.uri)) { throw new Error(`duplicate execution for ${cell.uri}`); } - const execution = new NotebookCellExecutionTask(docUri, document, cell, this._notebookDocumentsProxy); + const execution = new NotebookCellExecutionTask(cell.notebook, cell, this._notebookDocumentsProxy); this._activeExecutions.set(cell.uri, execution); const listener = execution.onDidChangeState(() => { if (execution.state === NotebookCellExecutionTaskState.Resolved) { @@ -694,7 +684,6 @@ class NotebookCellExecutionTask extends Disposable { private _executionOrder: number | undefined; constructor( - private readonly _uri: vscode.Uri, private readonly _document: ExtHostNotebookDocument, private readonly _cell: ExtHostCell, private readonly _proxy: MainThreadNotebookDocumentsShape) { @@ -718,7 +707,7 @@ class NotebookCellExecutionTask extends Disposable { } private async applyEdits(edits: IImmediateCellEditOperation[]): Promise { - return this._proxy.$applyEdits(this._uri, edits, false); + return this._proxy.$applyEdits(this._document.uri, edits, false); } private verifyStateForOutput() { diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index 8f70037a039..4ec48ddf8f0 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -53,10 +53,10 @@ export class ExtHostCell { readonly uri: URI; readonly cellKind: CellKind; - private _cell: vscode.NotebookCell | undefined; + private _apiCell: vscode.NotebookCell | undefined; constructor( - private readonly _notebook: ExtHostNotebookDocument, + readonly notebook: ExtHostNotebookDocument, private readonly _extHostDocument: ExtHostDocumentsAndEditors, private readonly _cellData: IMainCellDto, ) { @@ -74,15 +74,15 @@ export class ExtHostCell { } get apiCell(): vscode.NotebookCell { - if (!this._cell) { + if (!this._apiCell) { const that = this; const data = this._extHostDocument.getDocument(this.uri); if (!data) { throw new Error(`MISSING extHostDocument for notebook cell: ${this.uri}`); } - this._cell = Object.freeze({ - get index() { return that._notebook.getCellIndex(that); }, - notebook: that._notebook.apiNotebook, + this._apiCell = Object.freeze({ + get index() { return that.notebook.getCellIndex(that); }, + notebook: that.notebook.apiNotebook, kind: extHostTypeConverters.NotebookCellKind.to(this._cellData.cellKind), document: data.document, get outputs() { return that._outputs.slice(0); }, @@ -90,7 +90,7 @@ export class ExtHostCell { get latestExecutionSummary() { return that._previousResult; } }); } - return this._cell; + return this._apiCell; } setOutputs(newOutputs: IOutputDto[]): void { @@ -278,7 +278,7 @@ export class ExtHostNotebookDocument { const changeEvent = new RawContentChangeEvent(splice[0], splice[1], [], newCells); const deletedItems = this._cells.splice(splice[0], splice[1], ...newCells); - for (let cell of deletedItems) { + for (const cell of deletedItems) { removedCellDocuments.push(cell.uri); changeEvent.deletedItems.push(cell.apiCell); } @@ -349,6 +349,10 @@ export class ExtHostNotebookDocument { } } + getCellFromApiCell(apiCell: vscode.NotebookCell): ExtHostCell | undefined { + return this._cells.find(cell => cell.apiCell === apiCell); + } + getCellFromIndex(index: number): ExtHostCell | undefined { return this._cells[index]; } diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index 7bf47b43dc7..b663ee5bd23 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -153,14 +153,21 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { _update(); }, createNotebookCellExecutionTask(cell) { + if (cell.index < 0) { + throw new Error('CANNOT execute cell that has been REMOVED from notebook'); + } if (isDisposed) { throw new Error('notebook controller is DISPOSED'); } if (!associatedNotebooks.has(cell.notebook.uri)) { throw new Error(`notebook controller is NOT associated to notebook: ${cell.notebook.uri.toString()}`); } - //todo@jrieken - return that._extHostNotebook.createNotebookCellExecution(cell.notebook.uri, cell.index, data.id)!; + const notebook = that._extHostNotebook.getNotebookDocument(cell.notebook.uri); + const cellObj = notebook.getCellFromApiCell(cell); + if (!cellObj) { + throw new Error('invalid cell'); + } + return that._extHostNotebook.createNotebookCellExecution(cellObj)!; }, dispose: () => { if (!isDisposed) { @@ -200,7 +207,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { const obj = this._kernelData.get(handle); if (obj) { // update data structure - const notebook = this._extHostNotebook.lookupNotebookDocument(URI.revive(uri))!; + const notebook = this._extHostNotebook.getNotebookDocument(URI.revive(uri))!; if (value) { obj.associatedNotebooks.set(notebook.uri, true); } else { @@ -220,11 +227,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { // extension can dispose kernels in the meantime return; } - const document = this._extHostNotebook.lookupNotebookDocument(URI.revive(uri)); - if (!document) { - throw new Error('MISSING notebook'); - } - + const document = this._extHostNotebook.getNotebookDocument(URI.revive(uri)); const cells: vscode.NotebookCell[] = []; for (let cellHandle of handles) { const cell = document.getCell(cellHandle); @@ -247,10 +250,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { // extension can dispose kernels in the meantime return; } - const document = this._extHostNotebook.lookupNotebookDocument(URI.revive(uri)); - if (!document) { - throw new Error('MISSING notebook'); - } + const document = this._extHostNotebook.getNotebookDocument(URI.revive(uri)); if (obj.controller.interruptHandler) { await obj.controller.interruptHandler.call(obj.controller, document.apiNotebook); } diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index 6a9efa8f95c..f68561ac73a 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -545,7 +545,7 @@ export namespace WorkspaceEdit { resource: entry.uri, edit: entry.edit, notebookMetadata: entry.notebookMetadata, - notebookVersionId: extHostNotebooks?.lookupNotebookDocument(entry.uri)?.apiNotebook.version + notebookVersionId: extHostNotebooks?.getNotebookDocument(entry.uri, true)?.apiNotebook.version }); } else if (entry._type === types.FileEditType.CellOutput) { @@ -580,7 +580,7 @@ export namespace WorkspaceEdit { _type: extHostProtocol.WorkspaceEditType.Cell, metadata: entry.metadata, resource: entry.uri, - notebookVersionId: extHostNotebooks?.lookupNotebookDocument(entry.uri)?.apiNotebook.version, + notebookVersionId: extHostNotebooks?.getNotebookDocument(entry.uri, true)?.apiNotebook.version, edit: { editType: notebooks.CellEditType.Replace, index: entry.index, diff --git a/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts b/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts index 1af0ca885cc..2d07e246675 100644 --- a/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts +++ b/src/vs/workbench/test/browser/api/extHostNotebookKernel2.test.ts @@ -8,18 +8,37 @@ import { TestRPCProtocol } from 'vs/workbench/test/browser/api/testRPCProtocol'; import { ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebook'; import { nullExtensionDescription } from 'vs/workbench/services/extensions/common/extensions'; import { mock } from 'vs/workbench/test/common/workbenchTestServices'; -import { INotebookKernelDto2, MainContext, MainThreadCommandsShape, MainThreadNotebookKernelsShape } from 'vs/workbench/api/common/extHost.protocol'; +import { INotebookKernelDto2, MainContext, MainThreadCommandsShape, MainThreadNotebookKernelsShape, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostNotebookKernels } from 'vs/workbench/api/common/extHostNotebookKernels'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService'; +import { NullLogService } from 'vs/platform/log/common/log'; +import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; +import { ExtHostDocuments } from 'vs/workbench/api/common/extHostDocuments'; +import { ExtHostNotebookDocument } from 'vs/workbench/api/common/extHostNotebookDocument'; +import { IExtensionStoragePaths } from 'vs/workbench/api/common/extHostStoragePaths'; +import { URI } from 'vs/base/common/uri'; +import { generateUuid } from 'vs/base/common/uuid'; +import { ExtHostCommands } from 'vs/workbench/api/common/extHostCommands'; +import { CellKind, CellUri, NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { DisposableStore } from 'vs/base/common/lifecycle'; suite('NotebookKernel', function () { let rpcProtocol: TestRPCProtocol; let extHostNotebookKernels: ExtHostNotebookKernels; + let notebook: ExtHostNotebookDocument; + let extHostDocumentsAndEditors: ExtHostDocumentsAndEditors; + let extHostDocuments: ExtHostDocuments; + let extHostNotebooks: ExtHostNotebookController; + const notebookUri = URI.parse('test:///notebook.file'); const kernelData = new Map(); + const disposables = new DisposableStore(); + teardown(function () { + disposables.clear(); + }); setup(async function () { kernelData.clear(); @@ -41,10 +60,61 @@ suite('NotebookKernel', function () { } }); + rpcProtocol.set(MainContext.MainThreadNotebook, new class extends mock() { + override async $registerNotebookProvider() { } + override async $unregisterNotebookProvider() { } + }); + extHostDocumentsAndEditors = new ExtHostDocumentsAndEditors(rpcProtocol, new NullLogService()); + extHostDocuments = new ExtHostDocuments(rpcProtocol, extHostDocumentsAndEditors); + const extHostStoragePaths = new class extends mock() { + override workspaceValue() { + return URI.from({ scheme: 'test', path: generateUuid() }); + } + }; + extHostNotebooks = new ExtHostNotebookController(rpcProtocol, new ExtHostCommands(rpcProtocol, new NullLogService()), extHostDocumentsAndEditors, extHostDocuments, new NullLogService(), extHostStoragePaths); + + extHostNotebooks.$acceptDocumentAndEditorsDelta({ + addedDocuments: [{ + uri: notebookUri, + viewType: 'test', + versionId: 0, + cells: [{ + handle: 0, + uri: CellUri.generate(notebookUri, 0), + source: ['### Heading'], + eol: '\n', + language: 'markdown', + cellKind: CellKind.Markdown, + outputs: [], + }, { + handle: 1, + uri: CellUri.generate(notebookUri, 1), + source: ['console.log("aaa")', 'console.log("bbb")'], + eol: '\n', + language: 'javascript', + cellKind: CellKind.Code, + outputs: [], + }], + }], + addedEditors: [{ + documentUri: notebookUri, + id: '_notebook_editor_0', + selections: [{ start: 0, end: 1 }], + visibleRanges: [] + }] + }); + extHostNotebooks.$acceptDocumentAndEditorsDelta({ newActiveEditor: '_notebook_editor_0' }); + + notebook = extHostNotebooks.notebookDocuments[0]!; + + disposables.add(notebook); + disposables.add(extHostDocuments); + + extHostNotebookKernels = new ExtHostNotebookKernels( rpcProtocol, new class extends mock() { }, - new class extends mock() { } + extHostNotebooks ); }); @@ -93,4 +163,46 @@ suite('NotebookKernel', function () { assert.strictEqual(first.id, 'nullExtensionDescription/foo'); assert.strictEqual(first.label, 'Far'); }); + + test('execute - simple createNotebookCellExecutionTask', function () { + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, 'foo', '*', 'Foo'); + + extHostNotebookKernels.$acceptSelection(0, notebook.uri, true); + + const cell1 = notebook.apiNotebook.cellAt(0); + const task = kernel.createNotebookCellExecutionTask(cell1); + task.start(); + task.end(); + }); + + test('createNotebookCellExecutionTask, must be selected/associated', function () { + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, 'foo', '*', 'Foo'); + assert.throws(() => { + kernel.createNotebookCellExecutionTask(notebook.apiNotebook.cellAt(0)); + }); + + extHostNotebookKernels.$acceptSelection(0, notebook.uri, true); + kernel.createNotebookCellExecutionTask(notebook.apiNotebook.cellAt(0)); + }); + + test('createNotebookCellExecutionTask, cell must be alive', function () { + const kernel = extHostNotebookKernels.createNotebookController(nullExtensionDescription, 'foo', '*', 'Foo'); + + const cell1 = notebook.apiNotebook.cellAt(0); + + extHostNotebookKernels.$acceptSelection(0, notebook.uri, true); + extHostNotebooks.$acceptModelChanged(notebook.uri, { + versionId: 12, + rawEvents: [{ + kind: NotebookCellsChangeType.ModelChange, + changes: [[0, notebook.apiNotebook.cellCount, []]] + }] + }, true); + + assert.strictEqual(cell1.index, -1); + + assert.throws(() => { + kernel.createNotebookCellExecutionTask(cell1); + }); + }); });