From 0ab9d9926d17ee525587cd200fa38993b0be2c8e Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 11 Aug 2021 16:03:45 -0700 Subject: [PATCH] revert undo/redo fix for scrolling. fix #130607, #130617 --- .../src/singlefolder-tests/notebook.test.ts | 24 +++++++ .../contrib/undoRedo/notebookUndoRedo.ts | 70 +++++++------------ 2 files changed, 51 insertions(+), 43 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts index b7bfc767d5d..e0e1358d9a6 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts @@ -257,6 +257,30 @@ suite('Notebook API tests', function () { assert.strictEqual(version + 1, editor.document.version); }); + test('edit API batch edits undo/redo', async function () { + const notebook = await openRandomNotebookDocument(); + const editor = await vscode.window.showNotebookDocument(notebook); + + const cellsChangeEvent = asPromise(vscode.notebooks.onDidChangeNotebookCells); + const cellMetadataChangeEvent = asPromise(vscode.notebooks.onDidChangeCellMetadata); + const version = editor.document.version; + await editor.edit(editBuilder => { + editBuilder.replaceCells(1, 0, [{ kind: vscode.NotebookCellKind.Code, languageId: 'javascript', value: 'test 2', outputs: [], metadata: undefined }]); + editBuilder.replaceCellMetadata(0, { inputCollapsed: false }); + }); + + await cellsChangeEvent; + await cellMetadataChangeEvent; + assert.strictEqual(editor.document.cellCount, 3); + assert.strictEqual(editor.document.cellAt(0)?.metadata.inputCollapsed, false); + assert.strictEqual(version + 1, editor.document.version); + + await vscode.commands.executeCommand('undo'); + assert.strictEqual(version + 2, editor.document.version); + assert.strictEqual(editor.document.cellAt(0)?.metadata.inputCollapsed, undefined); + assert.strictEqual(editor.document.cellCount, 2); + }); + test('#98841, initialzation should not emit cell change events.', async function () { let count = 0; diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/undoRedo/notebookUndoRedo.ts b/src/vs/workbench/contrib/notebook/browser/contrib/undoRedo/notebookUndoRedo.ts index a6fe96e97f1..c2e5a97057e 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/undoRedo/notebookUndoRedo.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/undoRedo/notebookUndoRedo.ts @@ -11,62 +11,46 @@ import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { CellEditState, getNotebookEditorFromEditorPane } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { RedoCommand, UndoCommand } from 'vs/editor/browser/editorExtensions'; -import { NotebookEditorWidget } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget'; class NotebookUndoRedoContribution extends Disposable { constructor(@IEditorService private readonly _editorService: IEditorService) { super(); - /** - * The undo/redo priority needs to be above code editors due to how the undo redo service works. Say we have two cells, cell 0 and cell 100, which can't be rendered in the same viewport - * 1. focus cell 0, type - * 2. focus cell 100. Cell 0 becomes invisible, the text model for it is disposed, which will mark the undo element "invalid" - * 3. undo. Since the last undo element is invalid, it will remove this undo element directly other than performing any real undo. - * - * We now make the notebook undo/redo impl the highest priority so we don't skip the "invalid" undo/redo element in the same notebook document - */ - this._register(UndoCommand.addImplementation(10000 + 5, 'notebook-undo-redo', () => { - const editor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane) as NotebookEditorWidget | undefined; - if (editor?.hasModel()) { - const activeCodeEditor = editor.activeCodeEditor; + const PRIORITY = 105; + this._register(UndoCommand.addImplementation(PRIORITY, 'notebook-undo-redo', () => { + const editor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane); + if (editor?.viewModel) { + return editor.viewModel.undo().then(cellResources => { + if (cellResources?.length) { + editor?.viewModel?.viewCells.forEach(cell => { + if (cell.cellKind === CellKind.Markup && cellResources.find(resource => resource.fragment === cell.model.uri.fragment)) { + cell.updateEditState(CellEditState.Editing, 'undo'); + } + }); - if ((activeCodeEditor && activeCodeEditor.hasTextFocus()) || !activeCodeEditor) { - return editor.viewModel.undo().then(cellResources => { - if (cellResources?.length) { - editor?.viewModel?.viewCells.forEach(cell => { - if (cell.cellKind === CellKind.Markup && cellResources.find(resource => resource.fragment === cell.model.uri.fragment)) { - cell.updateEditState(CellEditState.Editing, 'undo'); - } - }); - - editor?.setOptions({ cellOptions: { resource: cellResources[0] }, preserveFocus: true }); - } - }); - } + editor?.setOptions({ cellOptions: { resource: cellResources[0] }, preserveFocus: true }); + } + }); } return false; })); - this._register(RedoCommand.addImplementation(10000 + 5, 'notebook-undo-redo', () => { - const editor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane) as NotebookEditorWidget | undefined; - if (editor?.hasModel()) { - const activeCodeEditor = editor.activeCodeEditor; + this._register(RedoCommand.addImplementation(PRIORITY, 'notebook-undo-redo', () => { + const editor = getNotebookEditorFromEditorPane(this._editorService.activeEditorPane); + if (editor?.viewModel) { + return editor.viewModel.redo().then(cellResources => { + if (cellResources?.length) { + editor?.viewModel?.viewCells.forEach(cell => { + if (cell.cellKind === CellKind.Markup && cellResources.find(resource => resource.fragment === cell.model.uri.fragment)) { + cell.updateEditState(CellEditState.Editing, 'redo'); + } + }); - if ((activeCodeEditor && activeCodeEditor.hasTextFocus()) || !activeCodeEditor) { - return editor.viewModel.redo().then(cellResources => { - if (cellResources?.length) { - editor?.viewModel?.viewCells.forEach(cell => { - if (cell.cellKind === CellKind.Markup && cellResources.find(resource => resource.fragment === cell.model.uri.fragment)) { - cell.updateEditState(CellEditState.Editing, 'redo'); - } - }); - - editor?.setOptions({ cellOptions: { resource: cellResources[0] }, preserveFocus: true }); - } - }); - } + editor?.setOptions({ cellOptions: { resource: cellResources[0] }, preserveFocus: true }); + } + }); } return false;