From 8bac4d17ad3693bd5059002fb90e44fac58d3e08 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 27 May 2020 16:00:49 -0700 Subject: [PATCH] combine events from edits from ext host. --- .../src/notebook.test.ts | 21 +++++ .../api/browser/mainThreadNotebook.ts | 2 +- .../workbench/api/common/extHostNotebook.ts | 72 ++++++++-------- .../common/model/notebookTextModel.ts | 83 +++++++++++-------- .../contrib/notebook/common/notebookCommon.ts | 2 +- .../api/extHostNotebookConcatDocument.test.ts | 38 ++++----- 6 files changed, 129 insertions(+), 89 deletions(-) diff --git a/extensions/vscode-notebook-tests/src/notebook.test.ts b/extensions/vscode-notebook-tests/src/notebook.test.ts index c24afdc56f2..bd3a32e8204 100644 --- a/extensions/vscode-notebook-tests/src/notebook.test.ts +++ b/extensions/vscode-notebook-tests/src/notebook.test.ts @@ -238,8 +238,29 @@ suite('API tests', () => { await vscode.commands.executeCommand('workbench.action.splitEditor'); await firstEditorDeactivate; + await vscode.commands.executeCommand('workbench.action.files.save'); await vscode.commands.executeCommand('workbench.action.closeAllEditors'); }); + + test('edit API', async function () { + const resource = vscode.Uri.parse(join(vscode.workspace.rootPath || '', './first.vsctestnb')); + await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); + + const cellsChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeNotebookCells); + await vscode.notebook.activeNotebookEditor!.edit(editBuilder => { + editBuilder.insert(1, 'test 2', 'javascript', vscode.CellKind.Code, [], undefined); + }); + + const cellChangeEventRet = await cellsChangeEvent; + assert.equal(cellChangeEventRet.document, vscode.notebook.activeNotebookEditor?.document); + assert.equal(cellChangeEventRet.changes.length, 1); + assert.deepEqual(cellChangeEventRet.changes[0].start, 1); + assert.deepEqual(cellChangeEventRet.changes[0].deletedCount, 0); + assert.equal(cellChangeEventRet.changes[0].items[0], vscode.notebook.activeNotebookEditor!.document.cells[1]); + + await vscode.commands.executeCommand('workbench.action.files.save'); + await vscode.commands.executeCommand('workbench.action.closeActiveEditor'); + }); }); suite('notebook workflow', () => { diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index 55ea5fae139..f88f0d1ec0d 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -34,7 +34,7 @@ export class MainThreadNotebookDocument extends Disposable { ) { super(); this._textModel = new NotebookTextModel(handle, viewType, uri); - this._register(this._textModel.onDidModelChange(e => { + this._register(this._textModel.onDidModelChangeProxy(e => { this._proxy.$acceptModelChanged(this.uri, e); this._proxy.$acceptEditorPropertiesChanged(uri, { selections: { selections: this._textModel.selections }, metadata: null }); })); diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 00e533b2260..1109053d164 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -261,7 +261,7 @@ export class ExtHostNotebookDocument extends Disposable implements vscode.Notebo accpetModelChanged(event: NotebookCellsChangedEvent): void { this._versionId = event.versionId; if (event.kind === NotebookCellsChangeType.ModelChange) { - this.$spliceNotebookCells(event.change); + this.$spliceNotebookCells(event.changes); } else if (event.kind === NotebookCellsChangeType.Move) { this.$moveCell(event.index, event.newIdx); } else if (event.kind === NotebookCellsChangeType.CellClearOutput) { @@ -273,51 +273,53 @@ export class ExtHostNotebookDocument extends Disposable implements vscode.Notebo } } - private $spliceNotebookCells(splice: NotebookCellsSplice2): void { + private $spliceNotebookCells(splices: NotebookCellsSplice2[]): void { if (this._disposed) { return; } let contentChangeEvents: vscode.NotebookCellsChangeData[] = []; - let cellDtos = splice[2]; - let newCells = cellDtos.map(cell => { - const extCell = new ExtHostCell(this.viewType, this.uri, cell.handle, URI.revive(cell.uri), cell.source.join('\n'), cell.cellKind, cell.language, cell.outputs, cell.metadata, this._proxy); - const documentData = this._documentsAndEditors.getDocument(URI.revive(cell.uri)); + splices.reverse().forEach(splice => { + let cellDtos = splice[2]; + let newCells = cellDtos.map(cell => { + const extCell = new ExtHostCell(this.viewType, this.uri, cell.handle, URI.revive(cell.uri), cell.source.join('\n'), cell.cellKind, cell.language, cell.outputs, cell.metadata, this._proxy); + const documentData = this._documentsAndEditors.getDocument(URI.revive(cell.uri)); + + if (documentData) { + extCell.attachTextDocument(documentData); + } + + if (!this._cellDisposableMapping.has(extCell.handle)) { + this._cellDisposableMapping.set(extCell.handle, new DisposableStore()); + } + + let store = this._cellDisposableMapping.get(extCell.handle)!; + + store.add(extCell.onDidChangeOutputs((diffs) => { + this.eventuallyUpdateCellOutputs(extCell, diffs); + })); + + return extCell; + }); + + for (let j = splice[0]; j < splice[0] + splice[1]; j++) { + this._cellDisposableMapping.get(this.cells[j].handle)?.dispose(); + this._cellDisposableMapping.delete(this.cells[j].handle); - if (documentData) { - extCell.attachTextDocument(documentData); } - if (!this._cellDisposableMapping.has(extCell.handle)) { - this._cellDisposableMapping.set(extCell.handle, new DisposableStore()); - } + this.cells.splice(splice[0], splice[1], ...newCells); - let store = this._cellDisposableMapping.get(extCell.handle)!; + const event: vscode.NotebookCellsChangeData = { + start: splice[0], + deletedCount: splice[1], + items: newCells + }; - store.add(extCell.onDidChangeOutputs((diffs) => { - this.eventuallyUpdateCellOutputs(extCell, diffs); - })); - - return extCell; + contentChangeEvents.push(event); }); - for (let j = splice[0]; j < splice[0] + splice[1]; j++) { - this._cellDisposableMapping.get(this.cells[j].handle)?.dispose(); - this._cellDisposableMapping.delete(this.cells[j].handle); - - } - - this.cells.splice(splice[0], splice[1], ...newCells); - - const event: vscode.NotebookCellsChangeData = { - start: splice[0], - deletedCount: splice[1], - items: newCells - }; - - contentChangeEvents.push(event); - this._emitter.emitModelChange({ document: this, changes: contentChangeEvents @@ -1096,11 +1098,11 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN document.accpetModelChanged({ kind: NotebookCellsChangeType.ModelChange, versionId: modelData.versionId, - change: [ + changes: [[ 0, 0, modelData.cells - ] + ]] }); this._documents.get(revivedUriStr)?.dispose(); diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index ef96c12335e..9232209ce59 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -7,7 +7,7 @@ import { Emitter, Event } from 'vs/base/common/event'; import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; -import { INotebookTextModel, NotebookCellOutputsSplice, NotebookCellTextModelSplice, NotebookDocumentMetadata, NotebookCellMetadata, ICellEditOperation, CellEditType, CellUri, ICellInsertEdit, NotebookCellsChangedEvent, CellKind, IProcessedOutput, notebookDocumentMetadataDefaults, diff, ICellDeleteEdit, NotebookCellsChangeType, ICellDto2 } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookTextModel, NotebookCellOutputsSplice, NotebookCellTextModelSplice, NotebookDocumentMetadata, NotebookCellMetadata, ICellEditOperation, CellEditType, CellUri, ICellInsertEdit, NotebookCellsChangedEvent, CellKind, IProcessedOutput, notebookDocumentMetadataDefaults, diff, ICellDeleteEdit, NotebookCellsChangeType, ICellDto2, IMainCellDto } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { ITextSnapshot } from 'vs/editor/common/model'; function compareRangesUsingEnds(a: [number, number], b: [number, number]): number { @@ -71,7 +71,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel private readonly _onDidChangeCells = new Emitter(); get onDidChangeCells(): Event { return this._onDidChangeCells.event; } private _onDidModelChangeProxy = new Emitter(); - get onDidModelChange(): Event { return this._onDidModelChangeProxy.event; } + get onDidModelChangeProxy(): Event { return this._onDidModelChangeProxy.event; } private _onDidSelectionChangeProxy = new Emitter(); get onDidSelectionChange(): Event { return this._onDidSelectionChangeProxy.event; } private _onDidChangeContent = new Emitter(); @@ -182,10 +182,10 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel const cellUri = CellUri.generate(this.uri, cellHandle); return new NotebookCellTextModel(cellUri, cellHandle, cell.source, cell.language, cell.cellKind, cell.outputs || [], cell.metadata); }); - this.insertNewCell(insertEdit.index, mainCells); + this.insertNewCell(insertEdit.index, mainCells, false); break; case CellEditType.Delete: - this.removeCell(operations[i].index, operations[i].end - operations[i].start); + this.removeCell(operations[i].index, operations[i].end - operations[i].start, false); break; } } @@ -196,10 +196,19 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel return [diff.start, diff.deleteCount, diff.toInsert] as [number, number, NotebookCellTextModel[]]; }); - // this._onDidModelChangeProxy.fire({kind: NotebookCellsChangeType.ModelChange, - // versionId: this._versionId, change: diffs - // } - // ); + this._onDidModelChangeProxy.fire({ + kind: NotebookCellsChangeType.ModelChange, + versionId: this._versionId, + changes: diffs.map(diff => [diff[0], diff[1], diff[2].map(cell => ({ + handle: cell.handle, + uri: cell.uri, + source: cell.textBuffer.getLinesContent(), + language: cell.language, + cellKind: cell.cellKind, + outputs: cell.outputs, + metadata: cell.metadata + }))] as [number, number, IMainCellDto[]]) + }); this._onDidChangeCells.fire(diffs); return true; @@ -260,8 +269,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._onDidModelChangeProxy.fire({ kind: NotebookCellsChangeType.ModelChange, - versionId: this._versionId, change: - [ + versionId: this._versionId, changes: + [[ 0, 0, [{ @@ -273,13 +282,13 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel outputs: cell.outputs, metadata: cell.metadata }] - ] + ]] }); return; } - insertNewCell(index: number, cells: NotebookCellTextModel[]): void { + insertNewCell(index: number, cells: NotebookCellTextModel[], emitToExtHost: boolean = true): void { this._isUntitled = false; for (let i = 0; i < cells.length; i++) { @@ -294,28 +303,31 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this.cells.splice(index, 0, ...cells); this._onDidChangeContent.fire(); this._increaseVersionId(); - this._onDidModelChangeProxy.fire({ - kind: NotebookCellsChangeType.ModelChange, - versionId: this._versionId, change: - [ - index, - 0, - cells.map(cell => ({ - handle: cell.handle, - uri: cell.uri, - source: cell.textBuffer.getLinesContent(), - language: cell.language, - cellKind: cell.cellKind, - outputs: cell.outputs, - metadata: cell.metadata - })) - ] - }); + + if (emitToExtHost) { + this._onDidModelChangeProxy.fire({ + kind: NotebookCellsChangeType.ModelChange, + versionId: this._versionId, changes: + [[ + index, + 0, + cells.map(cell => ({ + handle: cell.handle, + uri: cell.uri, + source: cell.textBuffer.getLinesContent(), + language: cell.language, + cellKind: cell.cellKind, + outputs: cell.outputs, + metadata: cell.metadata + })) + ]] + }); + } return; } - removeCell(index: number, count: number) { + removeCell(index: number, count: number, emitToExtHost: boolean = true) { this._isUntitled = false; for (let i = index; i < index + count; i++) { @@ -327,10 +339,12 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._onDidChangeContent.fire(); this._increaseVersionId(); - this._onDidModelChangeProxy.fire({ kind: NotebookCellsChangeType.ModelChange, versionId: this._versionId, change: [index, count, []] }); + if (emitToExtHost) { + this._onDidModelChangeProxy.fire({ kind: NotebookCellsChangeType.ModelChange, versionId: this._versionId, changes: [[index, count, []]] }); + } } - moveCellToIdx(index: number, newIdx: number) { + moveCellToIdx(index: number, newIdx: number, emitToExtHost: boolean = true) { this.assertIndex(index); this.assertIndex(newIdx); @@ -338,7 +352,10 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this.cells.splice(newIdx, 0, ...cells); this._increaseVersionId(); - this._onDidModelChangeProxy.fire({ kind: NotebookCellsChangeType.Move, versionId: this._versionId, index, newIdx }); + + if (emitToExtHost) { + this._onDidModelChangeProxy.fire({ kind: NotebookCellsChangeType.Move, versionId: this._versionId, index, newIdx }); + } } assertIndex(index: number) { diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 5a3ae109687..9b53e3870ce 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -304,7 +304,7 @@ export enum NotebookCellsChangeType { export interface NotebookCellsModelChangedEvent { readonly kind: NotebookCellsChangeType.ModelChange; - readonly change: NotebookCellsSplice2; + readonly changes: NotebookCellsSplice2[]; readonly versionId: number; } diff --git a/src/vs/workbench/test/browser/api/extHostNotebookConcatDocument.test.ts b/src/vs/workbench/test/browser/api/extHostNotebookConcatDocument.test.ts index 5503ea47f21..26b9fde6c3b 100644 --- a/src/vs/workbench/test/browser/api/extHostNotebookConcatDocument.test.ts +++ b/src/vs/workbench/test/browser/api/extHostNotebookConcatDocument.test.ts @@ -116,7 +116,7 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [0, 0, [{ + changes: [[0, 0, [{ handle: 1, uri: CellUri.generate(notebook.uri, 1), source: ['Hello', 'World', 'Hello World!'], @@ -130,7 +130,7 @@ suite('NotebookConcatDocument', function () { language: 'test', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); @@ -155,14 +155,14 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [0, 0, [{ + changes: [[0, 0, [{ handle: 1, uri: CellUri.generate(notebook.uri, 1), source: ['Hello', 'World', 'Hello World!'], language: 'test', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); assert.equal(notebook.cells.length, 1 + 1); assert.equal(doc.version, 1); @@ -177,14 +177,14 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [1, 0, [{ + changes: [[1, 0, [{ handle: 2, uri: CellUri.generate(notebook.uri, 2), source: ['Hallo', 'Welt', 'Hallo Welt!'], language: 'test', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); assert.equal(notebook.cells.length, 1 + 2); @@ -200,7 +200,7 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [1, 1, []] + changes: [[1, 1, []]] }); assert.equal(notebook.cells.length, 1 + 1); assert.equal(doc.version, 3); @@ -218,7 +218,7 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [0, 0, [{ + changes: [[0, 0, [{ handle: 1, uri: CellUri.generate(notebook.uri, 1), source: ['Hello', 'World', 'Hello World!'], @@ -232,7 +232,7 @@ suite('NotebookConcatDocument', function () { language: 'test', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); assert.equal(notebook.cells.length, 1 + 2); assert.equal(doc.version, 1); @@ -281,7 +281,7 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [0, 0, [{ + changes: [[0, 0, [{ handle: 1, uri: CellUri.generate(notebook.uri, 1), source: ['fooLang-document'], @@ -295,7 +295,7 @@ suite('NotebookConcatDocument', function () { language: 'barLang', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); const mixedDoc = new ExtHostNotebookConcatDocument(extHostNotebooks, extHostDocuments, notebook, undefined); @@ -309,14 +309,14 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [2, 0, [{ + changes: [[2, 0, [{ handle: 3, uri: CellUri.generate(notebook.uri, 3), source: ['barLang-document2'], language: 'barLang', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); assertLines(mixedDoc, 'fooLang-document', 'barLang-document', 'barLang-document2'); @@ -342,7 +342,7 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [0, 0, [{ + changes: [[0, 0, [{ handle: 1, uri: CellUri.generate(notebook.uri, 1), source: ['Hello', 'World', 'Hello World!'], @@ -356,7 +356,7 @@ suite('NotebookConcatDocument', function () { language: 'test', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); assert.equal(notebook.cells.length, 1 + 2); // markdown and code @@ -393,7 +393,7 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [0, 0, [{ + changes: [[0, 0, [{ handle: 1, uri: CellUri.generate(notebook.uri, 1), source: ['Hello', 'World', 'Hello World!'], @@ -407,7 +407,7 @@ suite('NotebookConcatDocument', function () { language: 'test', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); assert.equal(notebook.cells.length, 1 + 2); // markdown and code @@ -428,7 +428,7 @@ suite('NotebookConcatDocument', function () { extHostNotebooks.$acceptModelChanged(notebookUri, { kind: NotebookCellsChangeType.ModelChange, versionId: notebook.versionId + 1, - change: [0, 0, [{ + changes: [[0, 0, [{ handle: 1, uri: CellUri.generate(notebook.uri, 1), source: ['Hello', 'World', 'Hello World!'], @@ -442,7 +442,7 @@ suite('NotebookConcatDocument', function () { language: 'test', cellKind: CellKind.Code, outputs: [], - }]] + }]]] }); assert.equal(notebook.cells.length, 1 + 2); // markdown and code