diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2754190..e386e15 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,6 +30,7 @@ jobs: test: name: Run tests on ${{ matrix.os }} runs-on: ${{ matrix.os }} + timeout-minutes: 10 strategy: matrix: diff --git a/javascript/src/api.ts b/javascript/src/api.ts index d861e14..7073307 100644 --- a/javascript/src/api.ts +++ b/javascript/src/api.ts @@ -73,8 +73,11 @@ export interface ISharedBase extends IObservableDisposable { /** * Perform a transaction. While the function f is called, all changes to the shared * document are bundled into a single event. + * + * @param f Transaction to execute + * @param undoable Whether to track the change in the action history or not (default `true`) */ - transact(f: () => void): void; + transact(f: () => void, undoable?: boolean): void; } /** diff --git a/javascript/src/ycell.ts b/javascript/src/ycell.ts index d19c591..316dd7a 100644 --- a/javascript/src/ycell.ts +++ b/javascript/src/ycell.ts @@ -431,21 +431,23 @@ export class YBaseCell return; } - this._ymetadata.delete(key); + this.transact(() => { + this._ymetadata.delete(key); - const jupyter = this.getMetadata('jupyter') as any; - if (key === 'collapsed' && jupyter) { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { outputs_hidden, ...others } = jupyter; + const jupyter = this.getMetadata('jupyter') as any; + if (key === 'collapsed' && jupyter) { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { outputs_hidden, ...others } = jupyter; - if (Object.keys(others).length === 0) { - this._ymetadata.delete('jupyter'); - } else { - this._ymetadata.set('jupyter', others); + if (Object.keys(others).length === 0) { + this._ymetadata.delete('jupyter'); + } else { + this._ymetadata.set('jupyter', others); + } + } else if (key === 'jupyter') { + this._ymetadata.delete('collapsed'); } - } else if (key === 'jupyter') { - this._ymetadata.delete('collapsed'); - } + }, false); } /** @@ -524,7 +526,7 @@ export class YBaseCell this.deleteMetadata('collapsed'); } } - }); + }, false); } else { const clone = JSONExt.deepCopy(metadata) as any; if (clone.collapsed != null) { @@ -538,7 +540,7 @@ export class YBaseCell for (const [key, value] of Object.entries(clone)) { this._ymetadata.set(key, value); } - }); + }, false); } } } @@ -558,13 +560,16 @@ export class YBaseCell /** * Perform a transaction. While the function f is called, all changes to the shared * document are bundled into a single event. + * + * @param f Transaction to execute + * @param undoable Whether to track the change in the action history or not (default `true`) */ transact(f: () => void, undoable = true): void { - this.notebook && undoable - ? this.notebook.transact(f) - : this.ymodel.doc == null - ? f() - : this.ymodel.doc.transact(f, this); + !this.notebook || this.notebook.disableDocumentWideUndoRedo + ? this.ymodel.doc == null + ? f() + : this.ymodel.doc.transact(f, undoable ? this : null) + : this.notebook.transact(f, undoable); } /** @@ -606,22 +611,24 @@ export class YBaseCell }); break; case 'update': - const newValue = this._ymetadata.get(key); - const oldValue = change.oldValue; - let equal = true; - if (typeof oldValue == 'object' && typeof newValue == 'object') { - equal = JSONExt.deepEqual(oldValue, newValue); - } else { - equal = oldValue === newValue; - } - - if (!equal) { - this._metadataChanged.emit({ - key, - type: 'change', - oldValue, - newValue - }); + { + const newValue = this._ymetadata.get(key); + const oldValue = change.oldValue; + let equal = true; + if (typeof oldValue == 'object' && typeof newValue == 'object') { + equal = JSONExt.deepEqual(oldValue, newValue); + } else { + equal = oldValue === newValue; + } + + if (!equal) { + this._metadataChanged.emit({ + key, + type: 'change', + oldValue, + newValue + }); + } } break; } @@ -732,7 +739,7 @@ export class YCodeCell if (this.ymodel.get('execution_count') !== count) { this.transact(() => { this.ymodel.set('execution_count', count); - }); + }, false); } } @@ -865,7 +872,7 @@ class YAttachmentCell } else { this.ymodel.set('attachments', attachments); } - }); + }, false); } /** diff --git a/javascript/test/ycell.spec.ts b/javascript/test/ycell.spec.ts index 23f213f..c8addfe 100644 --- a/javascript/test/ycell.spec.ts +++ b/javascript/test/ycell.spec.ts @@ -1,3 +1,4 @@ +/* eslint-disable camelcase */ // Copyright (c) Jupyter Development Team. // Distributed under the terms of the Modified BSD License. @@ -227,4 +228,89 @@ describe('@jupyter/ydoc', () => { } }); }); + + describe('#undo', () => { + test('should undo source change', () => { + const codeCell = YCodeCell.create(); + codeCell.setSource('test'); + codeCell.undoManager?.stopCapturing(); + codeCell.updateSource(0, 0, 'hello'); + + codeCell.undo(); + + expect(codeCell.getSource()).toEqual('test'); + }); + + test('should not undo execution count change', () => { + const codeCell = YCodeCell.create(); + codeCell.setSource('test'); + codeCell.undoManager?.stopCapturing(); + codeCell.execution_count = 22; + codeCell.undoManager?.stopCapturing(); + codeCell.execution_count = 42; + + codeCell.undo(); + + expect(codeCell.execution_count).toEqual(42); + }); + + test('should not undo output change', () => { + const codeCell = YCodeCell.create(); + codeCell.setSource('test'); + codeCell.undoManager?.stopCapturing(); + const outputs = [ + { + data: { + 'application/geo+json': { + geometry: { + coordinates: [-118.4563712, 34.0163116], + type: 'Point' + }, + type: 'Feature' + }, + 'text/plain': [''] + }, + metadata: { + 'application/geo+json': { + expanded: false + } + }, + output_type: 'display_data' + }, + { + data: { + 'application/vnd.jupyter.widget-view+json': { + model_id: '4619c172d65e496baa5d1230894b535a', + version_major: 2, + version_minor: 0 + }, + 'text/plain': [ + "HBox(children=(Text(value='text input', layout=Layout(border='1px dashed red', width='80px')), Button(descript…" + ] + }, + metadata: {}, + output_type: 'display_data' + } + ]; + codeCell.setOutputs(outputs); + codeCell.undoManager?.stopCapturing(); + + codeCell.undo(); + + expect(codeCell.getOutputs()).toEqual(outputs); + }); + + test('should not undo metadata change', () => { + const codeCell = YCodeCell.create(); + codeCell.setSource('test'); + codeCell.undoManager?.stopCapturing(); + codeCell.setMetadata({ collapsed: false }); + codeCell.undoManager?.stopCapturing(); + codeCell.setMetadata({ collapsed: true }); + + codeCell.undo(); + + expect(codeCell.getMetadata('collapsed')).toEqual(true); + }); + }); }); diff --git a/javascript/test/ynotebook.spec.ts b/javascript/test/ynotebook.spec.ts index a1afab2..3317982 100644 --- a/javascript/test/ynotebook.spec.ts +++ b/javascript/test/ynotebook.spec.ts @@ -482,5 +482,103 @@ describe('@jupyter/ydoc', () => { notebook.dispose(); }); }); + + describe('#undo', () => { + describe('globally', () => { + test('should undo cell addition', () => { + const notebook = YNotebook.create(); + notebook.addCell({ cell_type: 'code' }); + notebook.undoManager.stopCapturing(); + notebook.addCell({ cell_type: 'markdown' }); + + expect(notebook.cells.length).toEqual(2); + + notebook.undo(); + + expect(notebook.cells.length).toEqual(1); + }); + + test('should undo cell source update', () => { + const notebook = YNotebook.create(); + const codeCell = notebook.addCell({ cell_type: 'code' }); + notebook.undoManager.stopCapturing(); + notebook.addCell({ cell_type: 'markdown' }); + notebook.undoManager.stopCapturing(); + codeCell.updateSource(0, 0, 'print(hello);'); + + notebook.undo(); + + expect(notebook.cells.length).toEqual(2); + expect(notebook.getCell(0).getSource()).toEqual(''); + }); + + test('should undo at global level when called locally', () => { + const notebook = YNotebook.create(); + const codeCell = notebook.addCell({ cell_type: 'code' }); + notebook.undoManager.stopCapturing(); + const markdownCell = notebook.addCell({ cell_type: 'markdown' }); + notebook.undoManager.stopCapturing(); + codeCell.updateSource(0, 0, 'print(hello);'); + notebook.undoManager.stopCapturing(); + markdownCell.updateSource(0, 0, '# Title'); + + codeCell.undo(); + + expect(notebook.cells.length).toEqual(2); + expect(notebook.getCell(0).getSource()).toEqual('print(hello);'); + expect(notebook.getCell(1).getSource()).toEqual(''); + }); + }); + + describe('per cells', () => { + test('should undo cell addition', () => { + const notebook = YNotebook.create({ + disableDocumentWideUndoRedo: true + }); + notebook.addCell({ cell_type: 'code' }); + notebook.undoManager.stopCapturing(); + notebook.addCell({ cell_type: 'markdown' }); + + expect(notebook.cells.length).toEqual(2); + + notebook.undo(); + + expect(notebook.cells.length).toEqual(1); + }); + + test('should not undo cell source update', () => { + const notebook = YNotebook.create({ + disableDocumentWideUndoRedo: true + }); + const codeCell = notebook.addCell({ cell_type: 'code' }); + notebook.undoManager.stopCapturing(); + notebook.addCell({ cell_type: 'markdown' }); + + codeCell.updateSource(0, 0, 'print(hello);'); + + notebook.undo(); + + expect(notebook.cells.length).toEqual(1); + expect(notebook.getCell(0).getSource()).toEqual('print(hello);'); + }); + + test('should only undo cell source update', () => { + const notebook = YNotebook.create({ + disableDocumentWideUndoRedo: true + }); + const codeCell = notebook.addCell({ cell_type: 'code' }); + notebook.undoManager.stopCapturing(); + const markdownCell = notebook.addCell({ cell_type: 'markdown' }); + codeCell.updateSource(0, 0, 'print(hello);'); + markdownCell.updateSource(0, 0, '# Title'); + + codeCell.undo(); + + expect(notebook.cells.length).toEqual(2); + expect(notebook.getCell(0).getSource()).toEqual(''); + expect(notebook.getCell(1).getSource()).toEqual('# Title'); + }); + }); + }); }); });