From fdf5b95a51c540d17d55c90a8ed3d9f7f9022279 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 27 Aug 2020 11:24:55 +0200 Subject: [PATCH] replace ICellInsertEdit, ICellDeleteEdit with ICellReplaceEdit, https://github.com/microsoft/vscode/issues/105283 --- .../api/browser/mainThreadNotebook.ts | 3 +- .../workbench/api/common/extHostNotebook.ts | 48 +++++----------- src/vs/workbench/api/common/extHostTypes.ts | 3 +- .../notebook/browser/notebookServiceImpl.ts | 2 +- .../common/model/notebookTextModel.ts | 55 +++++++++++++++---- .../contrib/notebook/common/notebookCommon.ts | 20 +++---- .../notebook/test/notebookTextModel.test.ts | 44 +++++++++++---- 7 files changed, 103 insertions(+), 72 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index cda01b3eab6b6..91d4c528fde42 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -392,8 +392,7 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo mainthreadTextModel.transientOptions = options; const edits: ICellEditOperation[] = [ - { editType: CellEditType.Delete, count: mainthreadTextModel.cells.length, index: 0 }, - { editType: CellEditType.Insert, index: 0, cells: data.cells } + { editType: CellEditType.Replace, index: 0, count: mainthreadTextModel.cells.length, cells: data.cells } ]; this._notebookService.transformEditsOutputs(mainthreadTextModel, edits); diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 7bb9f60586ac2..2c6410400de9c 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -22,7 +22,7 @@ import { IExtensionStoragePaths } from 'vs/workbench/api/common/extHostStoragePa import * as typeConverters from 'vs/workbench/api/common/extHostTypeConverters'; import * as extHostTypes from 'vs/workbench/api/common/extHostTypes'; import { asWebviewUri, WebviewInitData } from 'vs/workbench/api/common/shared/webview'; -import { addIdToOutput, CellEditType, CellOutputKind, CellStatusbarAlignment, CellUri, diff, ICellDeleteEdit, ICellDto2, ICellEditOperation, ICellInsertEdit, IMainCellDto, INotebookCellStatusBarEntry, INotebookDisplayOrder, INotebookEditData, INotebookKernelInfoDto2, IProcessedOutput, NotebookCellMetadata, NotebookCellsChangedEvent, NotebookCellsChangeType, NotebookCellsSplice2, NotebookDataDto, notebookDocumentMetadataDefaults } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { addIdToOutput, CellEditType, CellOutputKind, CellStatusbarAlignment, CellUri, diff, ICellEditOperation, ICellReplaceEdit, IMainCellDto, INotebookCellStatusBarEntry, INotebookDisplayOrder, INotebookEditData, INotebookKernelInfoDto2, IProcessedOutput, NotebookCellMetadata, NotebookCellsChangedEvent, NotebookCellsChangeType, NotebookCellsSplice2, NotebookDataDto, notebookDocumentMetadataDefaults } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import * as vscode from 'vscode'; import { Cache } from './cache'; import { ResourceMap } from 'vs/base/common/map'; @@ -539,29 +539,17 @@ export class NotebookEditorCellEditBuilder implements vscode.NotebookEditorCellE replaceCells(from: number, to: number, cells: vscode.NotebookCellData[]): void { this._throwIfFinalized(); - // deletion - if (to > from) { - this._collectedEdits.push({ - editType: CellEditType.Delete, - index: from, - count: to - from - }); - } - // insert - if (cells.length > 0) { - this._collectedEdits.push({ - editType: CellEditType.Insert, - index: from, - cells: cells.map(data => { - return { - cellKind: data.cellKind, - language: data.language, - outputs: data.outputs.map(output => addIdToOutput(output)), - source: data.source - }; - }) - }); - } + this._collectedEdits.push({ + editType: CellEditType.Replace, + index: from, + count: to - from, + cells: cells.map(data => { + return { + ...data, + outputs: data.outputs.map(output => addIdToOutput(output)), + }; + }) + }); } insert(index: number, content: string | string[], language: string, type: CellKind, outputs: vscode.CellOutput[], metadata: vscode.NotebookCellMetadata | undefined): void { @@ -716,16 +704,10 @@ export class ExtHostNotebookEditor extends Disposable implements vscode.Notebook const prevIndex = compressedEditsIndex; const prev = compressedEdits[prevIndex]; - if (prev.editType === CellEditType.Insert && editData.edits[i].editType === CellEditType.Insert) { - if (prev.index === editData.edits[i].index) { - prev.cells.push(...(editData.edits[i] as ICellInsertEdit).cells); - continue; - } - } - - if (prev.editType === CellEditType.Delete && editData.edits[i].editType === CellEditType.Delete) { + if (prev.editType === CellEditType.Replace && editData.edits[i].editType === CellEditType.Replace) { if (prev.index === editData.edits[i].index) { - prev.count += (editData.edits[i] as ICellDeleteEdit).count; + prev.cells.push(...(editData.edits[i] as ICellReplaceEdit).cells); + prev.count += (editData.edits[i] as ICellReplaceEdit).count; continue; } } diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 3cc938b9889d5..bb8a0e560d3c5 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -632,8 +632,7 @@ export class WorkspaceEdit implements vscode.WorkspaceEdit { // --- cell replaceCells(uri: URI, start: number, end: number, cells: vscode.NotebookCellData[], metadata?: vscode.WorkspaceEditEntryMetadata): void { - this._edits.push({ _type: FileEditType.Cell, metadata, uri, edit: { editType: CellEditType.Delete, index: start, count: end - start } }); - this._edits.push({ _type: FileEditType.Cell, metadata, uri, edit: { editType: CellEditType.Insert, index: start, cells: cells.map(cell => ({ ...cell, outputs: cell.outputs.map(output => addIdToOutput(output)) })) } }); + this._edits.push({ _type: FileEditType.Cell, metadata, uri, edit: { editType: CellEditType.Replace, index: start, count: end - start, cells: cells.map(cell => ({ ...cell, outputs: cell.outputs.map(output => addIdToOutput(output)) })) } }); } replaceCellOutput(uri: URI, index: number, outputs: vscode.CellOutput[], metadata?: vscode.WorkspaceEditEntryMetadata): void { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts index 5a72f03823cf8..80598b122bb58 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts @@ -720,7 +720,7 @@ export class NotebookService extends Disposable implements INotebookService, ICu transformEditsOutputs(textModel: NotebookTextModel, edits: ICellEditOperation[]) { edits.forEach((edit) => { - if (edit.editType === CellEditType.Insert) { + if (edit.editType === CellEditType.Replace) { edit.cells.forEach((cell) => { const outputs = cell.outputs; outputs.map((output) => { diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index a27d7e8d2180d..1f68435cbdce6 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -232,7 +232,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel const edits = rawEdits.map((edit, index) => { return { edit, - end: edit.editType === CellEditType.Delete ? edit.index + edit.count : edit.index, + end: edit.editType === CellEditType.Replace ? edit.index + edit.count : edit.index, originalIndex: index, }; }).sort((a, b) => { @@ -241,16 +241,8 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel for (const { edit } of edits) { switch (edit.editType) { - case CellEditType.Insert: - const mainCells = edit.cells.map(cell => { - const cellHandle = this._cellhandlePool++; - const cellUri = CellUri.generate(this.uri, cellHandle); - return new NotebookCellTextModel(cellUri, cellHandle, cell.source, cell.language, cell.cellKind, cell.outputs || [], cell.metadata, this.transientOptions, this._modelService); - }); - this.insertNewCell(edit.index, mainCells, false); - break; - case CellEditType.Delete: - this.removeCell(edit.index, edit.count, false); + case CellEditType.Replace: + this._replaceCells(edit.index, edit.count, edit.cells); break; case CellEditType.Output: //TODO@joh,@rebornix no event, no undo stop (?) @@ -302,6 +294,47 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel return true; } + private _replaceCells(index: number, count: number, cellDtos: ICellDto2[]): void { + + if (count === 0 && cellDtos.length === 0) { + return; + } + + this._isUntitled = false; //TODO@rebornix fishy? + + // prepare remove + for (let i = index; i < index + count; i++) { + const cell = this.cells[i]; + this._cellListeners.get(cell.handle)?.dispose(); + this._cellListeners.delete(cell.handle); + } + + // prepare add + const cells = cellDtos.map(cellDto => { + const cellHandle = this._cellhandlePool++; + const cellUri = CellUri.generate(this.uri, cellHandle); + const cell = new NotebookCellTextModel( + cellUri, cellHandle, + cellDto.source, cellDto.language, cellDto.cellKind, cellDto.outputs || [], cellDto.metadata, this.transientOptions, + this._modelService + ); + const dirtyStateListener = cell.onDidChangeContent(() => { + this.setDirty(true); + this._increaseVersionId(); + this._onDidChangeContent.fire(); + }); + this._cellListeners.set(cell.handle, dirtyStateListener); + this._mapping.set(cell.handle, cell); + return cell; + }); + + // make change + this.cells.splice(index, count, ...cells); + this.setDirty(true); + this._increaseVersionId(); + this._onDidChangeContent.fire(); + } + handleEdit(label: string | undefined, undo: () => void, redo: () => void): void { this._operationManager.pushEditOperation({ type: UndoRedoElementType.Resource, diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index fcb70b1599cf0..c4e05bd2be484 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -416,10 +416,9 @@ export interface NotebookCellsChangeMetadataEvent { export type NotebookCellsChangedEvent = NotebookCellsInitializeEvent | NotebookCellsModelChangedEvent | NotebookCellsModelMoveEvent | NotebookCellClearOutputEvent | NotebookCellsClearOutputEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent; export const enum CellEditType { - Insert = 1, - Delete = 2, - Output = 3, - Metadata = 4, + Replace = 1, + Output = 2, + Metadata = 3, } export interface ICellDto2 { @@ -430,16 +429,11 @@ export interface ICellDto2 { metadata?: NotebookCellMetadata; } -export interface ICellInsertEdit { - editType: CellEditType.Insert; - index: number; - cells: ICellDto2[]; -} - -export interface ICellDeleteEdit { - editType: CellEditType.Delete; +export interface ICellReplaceEdit { + editType: CellEditType.Replace; index: number; count: number; + cells: ICellDto2[]; } export interface ICellOutputEdit { @@ -454,7 +448,7 @@ export interface ICellMetadataEdit { metadata: NotebookCellMetadata; } -export type ICellEditOperation = ICellInsertEdit | ICellDeleteEdit | ICellOutputEdit | ICellMetadataEdit; +export type ICellEditOperation = ICellReplaceEdit | ICellOutputEdit | ICellMetadataEdit; export interface INotebookEditData { documentVersionId: number; diff --git a/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts b/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts index 444121d2af7e6..010ddba6af262 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookTextModel.test.ts @@ -30,8 +30,8 @@ suite('NotebookTextModel', () => { ], (editor, viewModel, textModel) => { textModel.applyEdit(textModel.versionId, [ - { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, - { editType: CellEditType.Insert, index: 3, cells: [new TestCell(viewModel.viewType, 6, 'var f = 6;', 'javascript', CellKind.Code, [], textModelService)] }, + { editType: CellEditType.Replace, index: 1, count: 0, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, + { editType: CellEditType.Replace, index: 3, count: 0, cells: [new TestCell(viewModel.viewType, 6, 'var f = 6;', 'javascript', CellKind.Code, [], textModelService)] }, ], true); assert.equal(textModel.cells.length, 6); @@ -55,8 +55,8 @@ suite('NotebookTextModel', () => { ], (editor, viewModel, textModel) => { textModel.applyEdit(textModel.versionId, [ - { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, - { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 6, 'var f = 6;', 'javascript', CellKind.Code, [], textModelService)] }, + { editType: CellEditType.Replace, index: 1, count: 0, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, + { editType: CellEditType.Replace, index: 1, count: 0, cells: [new TestCell(viewModel.viewType, 6, 'var f = 6;', 'javascript', CellKind.Code, [], textModelService)] }, ], true); assert.equal(textModel.cells.length, 6); @@ -80,8 +80,8 @@ suite('NotebookTextModel', () => { ], (editor, viewModel, textModel) => { textModel.applyEdit(textModel.versionId, [ - { editType: CellEditType.Delete, index: 1, count: 1 }, - { editType: CellEditType.Delete, index: 3, count: 1 }, + { editType: CellEditType.Replace, index: 1, count: 1, cells: [] }, + { editType: CellEditType.Replace, index: 3, count: 1, cells: [] }, ], true); assert.equal(textModel.cells[0].getValue(), 'var a = 1;'); @@ -103,8 +103,8 @@ suite('NotebookTextModel', () => { ], (editor, viewModel, textModel) => { textModel.applyEdit(textModel.versionId, [ - { editType: CellEditType.Delete, index: 1, count: 1 }, - { editType: CellEditType.Insert, index: 3, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, + { editType: CellEditType.Replace, index: 1, count: 1, cells: [] }, + { editType: CellEditType.Replace, index: 3, count: 0, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, ], true); assert.equal(textModel.cells.length, 4); @@ -128,8 +128,32 @@ suite('NotebookTextModel', () => { ], (editor, viewModel, textModel) => { textModel.applyEdit(textModel.versionId, [ - { editType: CellEditType.Delete, index: 1, count: 1 }, - { editType: CellEditType.Insert, index: 1, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, + { editType: CellEditType.Replace, index: 1, count: 1, cells: [] }, + { editType: CellEditType.Replace, index: 1, count: 0, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, + ], true); + + assert.equal(textModel.cells.length, 4); + assert.equal(textModel.cells[0].getValue(), 'var a = 1;'); + assert.equal(textModel.cells[1].getValue(), 'var e = 5;'); + assert.equal(textModel.cells[2].getValue(), 'var c = 3;'); + } + ); + }); + + test('(replace) delete + insert at same position', function () { + withTestNotebook( + instantiationService, + blukEditService, + undoRedoService, + [ + ['var a = 1;', 'javascript', CellKind.Code, [], { editable: true }], + ['var b = 2;', 'javascript', CellKind.Code, [], { editable: false }], + ['var c = 3;', 'javascript', CellKind.Code, [], { editable: false }], + ['var d = 4;', 'javascript', CellKind.Code, [], { editable: false }] + ], + (editor, viewModel, textModel) => { + textModel.applyEdit(textModel.versionId, [ + { editType: CellEditType.Replace, index: 1, count: 1, cells: [new TestCell(viewModel.viewType, 5, 'var e = 5;', 'javascript', CellKind.Code, [], textModelService)] }, ], true); assert.equal(textModel.cells.length, 4);