This commit is contained in:
rebornix 2021-04-21 21:27:53 -07:00
parent 27675a99be
commit 60af02d758
No known key found for this signature in database
GPG key ID: 181FC90D15393C20
3 changed files with 78 additions and 25 deletions

View file

@ -20,24 +20,24 @@ suite('Notebook Undo/Redo', () => {
const viewModel = editor.viewModel;
assert.strictEqual(viewModel.length, 2);
assert.strictEqual(viewModel.getVersionId(), 0);
assert.strictEqual(viewModel.getAlternativeId(), '0,1;1,1');
assert.strictEqual(viewModel.getAlternativeId(), '0_0,1;1,1');
viewModel.notebookDocument.applyEdits([{
editType: CellEditType.Replace, index: 0, count: 2, cells: []
}], true, undefined, () => undefined, undefined, true);
assert.strictEqual(viewModel.length, 0);
assert.strictEqual(viewModel.getVersionId(), 1);
assert.strictEqual(viewModel.getAlternativeId(), '');
assert.strictEqual(viewModel.getAlternativeId(), '1_');
await viewModel.undo();
assert.strictEqual(viewModel.length, 2);
assert.strictEqual(viewModel.getVersionId(), 2);
assert.strictEqual(viewModel.getAlternativeId(), '0,1;1,1');
assert.strictEqual(viewModel.getAlternativeId(), '0_0,1;1,1');
await viewModel.redo();
assert.strictEqual(viewModel.length, 0);
assert.strictEqual(viewModel.getVersionId(), 3);
assert.strictEqual(viewModel.getAlternativeId(), '');
assert.strictEqual(viewModel.getAlternativeId(), '1_');
viewModel.notebookDocument.applyEdits([{
editType: CellEditType.Replace, index: 0, count: 0, cells: [
@ -45,11 +45,11 @@ suite('Notebook Undo/Redo', () => {
]
}], true, undefined, () => undefined, undefined, true);
assert.strictEqual(viewModel.getVersionId(), 4);
assert.strictEqual(viewModel.getAlternativeId(), '2,1');
assert.strictEqual(viewModel.getAlternativeId(), '4_2,1');
await viewModel.undo();
assert.strictEqual(viewModel.getVersionId(), 5);
assert.strictEqual(viewModel.getAlternativeId(), '');
assert.strictEqual(viewModel.getAlternativeId(), '1_');
}
);
});

View file

@ -94,6 +94,10 @@ export class NotebookOperationManager {
) {
}
isUndoStackEmpty(): boolean {
return this._pendingStackOperation === null || this._pendingStackOperation.isEmpty;
}
pushStackElement(label: string, selectionState: ISelectionState | undefined, undoRedoGroup: UndoRedoGroup | undefined, alternativeVersionId: string) {
if (this._pendingStackOperation) {
this._pendingStackOperation.pushEndState(alternativeVersionId, selectionState);
@ -196,6 +200,11 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
transientOptions: TransientOptions = { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false };
private _versionId = 0;
/**
* This alternative id is only for non-cell-content changes.
*/
private _notebookSpecificAlternativeId = 0;
/**
* Unlike, versionId, this can go down (via undo) or go to previous values (via redo)
*/
@ -260,7 +269,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
uri,
this._eventEmitter,
(alternativeVersionId: string) => {
this._increaseVersionId();
this._increaseVersionId(true);
this._overwriteAlternativeVersionId(alternativeVersionId);
}
);
@ -269,6 +278,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
private _initialize(cells: ICellDto2[]) {
this._cells = [];
this._versionId = 0;
this._notebookSpecificAlternativeId = 0;
const mainCells = cells.map(cell => {
const cellHandle = this._cellhandlePool++;
@ -278,7 +288,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
for (let i = 0; i < mainCells.length; i++) {
const dirtyStateListener = mainCells[i].onDidChangeContent(() => {
this._increaseVersionId();
this._increaseVersionIdForCellContentChange();
this._eventEmitter.emit({ kind: NotebookCellsChangeType.ChangeCellContent, transient: false }, true);
});
@ -290,7 +300,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
}
private _generateAlternativeId() {
return this.cells.map(cell => cell.handle + ',' + cell.alternativeId).join(';');
return `${this._notebookSpecificAlternativeId}_` + this.cells.map(cell => cell.handle + ',' + cell.alternativeId).join(';');
}
override dispose() {
@ -337,7 +347,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
} finally {
// Update selection and versionId after applying edits.
const endSelections = endSelectionsComputer();
this._increaseVersionId();
this._increaseVersionId(this._operationManager.isUndoStackEmpty());
// Finalize undo element
this.pushStackElement('edit', endSelections, undefined);
@ -465,7 +475,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
cell.language = cellDto.language;
}
const dirtyStateListener = cell.onDidChangeContent(() => {
this._increaseVersionId();
this._increaseVersionIdForCellContentChange();
this._eventEmitter.emit({ kind: NotebookCellsChangeType.ChangeCellContent, transient: false }, true);
});
this._cellListeners.set(cell.handle, dirtyStateListener);
@ -502,13 +512,22 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
}, synchronous);
}
private _increaseVersionId(): void {
private _increaseVersionIdForCellContentChange(): void {
this._versionId = this._versionId + 1;
this._alternativeVersionId = this._generateAlternativeId();
}
private _increaseVersionId(undoStackEmpty: boolean): void {
this._versionId = this._versionId + 1;
if (!undoStackEmpty) {
this._notebookSpecificAlternativeId = this._versionId;
}
this._alternativeVersionId = this._generateAlternativeId();
}
private _overwriteAlternativeVersionId(newAlternativeVersionId: string): void {
this._alternativeVersionId = newAlternativeVersionId;
this._notebookSpecificAlternativeId = Number(newAlternativeVersionId.substr(0, newAlternativeVersionId.indexOf('_')));
}
private _isDocumentMetadataChangeTransient(a: NotebookDocumentMetadata, b: NotebookDocumentMetadata) {
@ -552,7 +571,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
private _insertNewCell(index: number, cells: NotebookCellTextModel[], synchronous: boolean, endSelections: ISelectionState | undefined): void {
for (let i = 0; i < cells.length; i++) {
const dirtyStateListener = cells[i].onDidChangeContent(() => {
this._increaseVersionId();
this._increaseVersionIdForCellContentChange();
this._eventEmitter.emit({ kind: NotebookCellsChangeType.ChangeCellContent, transient: false }, true);
});
@ -593,7 +612,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
for (let i = 0; i < cells.length; i++) {
const dirtyStateListener = cells[i].onDidChangeContent(() => {
this._increaseVersionId();
this._increaseVersionIdForCellContentChange();
this._eventEmitter.emit({ kind: NotebookCellsChangeType.ChangeCellContent, transient: false }, true);
});
@ -611,7 +630,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
if (key === 'custom') {
if (!this._customMetadataEqual(a[key], b[key])
&&
!(this.transientOptions.transientCellMetadata[key as keyof NotebookCellMetadata])
!(this.transientOptions.transientDocumentMetadata[key as keyof NotebookDocumentMetadata])
) {
return true;
}

View file

@ -4,7 +4,6 @@
*--------------------------------------------------------------------------------------------*/
import * as assert from 'assert';
import { Range } from 'vs/editor/common/core/range';
import { CellKind, CellEditType, NotebookTextModelChangedEvent, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { withTestNotebook, TestCell, setupInstantiationService } from 'vs/workbench/contrib/notebook/test/testNotebookEditor';
import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo';
@ -369,8 +368,8 @@ suite('NotebookTextModel', () => {
});
test('Updating appending/updating output in Notebooks does not work as expected #117273', function () {
withTestNotebook([
test('Updating appending/updating output in Notebooks does not work as expected #117273', async function () {
await withTestNotebook([
['var a = 1;', 'javascript', CellKind.Code, [], {}]
], (editor) => {
const model = editor.viewModel.notebookDocument;
@ -404,8 +403,8 @@ suite('NotebookTextModel', () => {
});
});
test('Clearing output of an empty notebook makes it dirty #119608', function () {
withTestNotebook([
test('Clearing output of an empty notebook makes it dirty #119608', async function () {
await withTestNotebook([
['var a = 1;', 'javascript', CellKind.Code, [], {}],
['var b = 2;', 'javascript', CellKind.Code, [], {}]
], (editor) => {
@ -474,16 +473,51 @@ suite('NotebookTextModel', () => {
});
});
test('Cell text model update increases notebook model version id #119561', function () {
withTestNotebook([
test('Cell metadata/output change should update version id and alternative id #121807', async function () {
await withTestNotebook([
['var a = 1;', 'javascript', CellKind.Code, [], {}],
['var b = 2;', 'javascript', CellKind.Code, [], {}]
], async (editor) => {
const textModel = await editor.viewModel.cellAt(0)!.resolveTextModel();
assert.ok(textModel !== undefined);
assert.strictEqual(editor.viewModel.getVersionId(), 0);
textModel.applyEdits([{ range: new Range(1, 1, 1, 1), text: 'x' }], true);
assert.strictEqual(editor.viewModel.getAlternativeId(), '0_0,1;1,1');
editor.viewModel.notebookDocument.applyEdits([
{
index: 0,
editType: CellEditType.Metadata,
metadata: {
inputCollapsed: true
}
}
], true, undefined, () => undefined, undefined, true);
assert.strictEqual(editor.viewModel.getVersionId(), 1);
assert.notStrictEqual(editor.viewModel.getAlternativeId(), '0_0,1;1,1');
assert.strictEqual(editor.viewModel.getAlternativeId(), '1_0,1;1,1');
await editor.viewModel.undo();
assert.strictEqual(editor.viewModel.getVersionId(), 2);
assert.strictEqual(editor.viewModel.getAlternativeId(), '0_0,1;1,1');
await editor.viewModel.redo();
assert.strictEqual(editor.viewModel.getVersionId(), 3);
assert.notStrictEqual(editor.viewModel.getAlternativeId(), '0_0,1;1,1');
assert.strictEqual(editor.viewModel.getAlternativeId(), '1_0,1;1,1');
editor.viewModel.notebookDocument.applyEdits([
{
index: 1,
editType: CellEditType.Metadata,
metadata: {
inputCollapsed: true
}
}
], true, undefined, () => undefined, undefined, true);
assert.strictEqual(editor.viewModel.getVersionId(), 4);
assert.strictEqual(editor.viewModel.getAlternativeId(), '4_0,1;1,1');
await editor.viewModel.undo();
assert.strictEqual(editor.viewModel.getVersionId(), 5);
assert.strictEqual(editor.viewModel.getAlternativeId(), '1_0,1;1,1');
});
});
});