CTRL+Z should not undo at the top level

news/2 Fixes/7999.md

@@ -0,0 +1 @@
+CTRL+Z is deleting cells. It should only undo changes inside of the code for a cell. 'Z' and 'SHIFT+Z' are for undoing/redoing cell adds/moves.

src/client/datascience/interactive-common/interactiveWindowTypes.ts

@@ -97,6 +97,7 @@ export enum NativeCommandType {
     InsertBelow,
     MoveCellDown,
     MoveCellUp,
+    Redo,
     Run,
     RunAbove,
     RunAll,

src/datascience-ui/interactive-common/mainStateController.ts

@@ -267,31 +267,35 @@ export class MainStateController implements IMessageHandler {
     }
 
     public redo = () => {
-        // Pop one off of our redo stack and update our undo
-        const cells = this.pendingState.redoStack[this.pendingState.redoStack.length - 1];
-        const redoStack = this.pendingState.redoStack.slice(0, this.pendingState.redoStack.length - 1);
-        const undoStack = this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs);
-        this.sendMessage(InteractiveWindowMessages.Redo);
-        this.setState({
-            cellVMs: cells,
-            undoStack: undoStack,
-            redoStack: redoStack,
-            skipNextScroll: true
-        });
+        if (this.pendingState.redoStack.length > 0) {
+            // Pop one off of our redo stack and update our undo
+            const cells = this.pendingState.redoStack[this.pendingState.redoStack.length - 1];
+            const redoStack = this.pendingState.redoStack.slice(0, this.pendingState.redoStack.length - 1);
+            const undoStack = this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs);
+            this.sendMessage(InteractiveWindowMessages.Redo);
+            this.setState({
+                cellVMs: cells,
+                undoStack: undoStack,
+                redoStack: redoStack,
+                skipNextScroll: true
+            });
+        }
     }
 
     public undo = () => {
-        // Pop one off of our undo stack and update our redo
-        const cells = this.pendingState.undoStack[this.pendingState.undoStack.length - 1];
-        const undoStack = this.pendingState.undoStack.slice(0, this.pendingState.undoStack.length - 1);
-        const redoStack = this.pushStack(this.pendingState.redoStack, this.pendingState.cellVMs);
-        this.sendMessage(InteractiveWindowMessages.Undo);
-        this.setState({
-            cellVMs: cells,
-            undoStack: undoStack,
-            redoStack: redoStack,
-            skipNextScroll: true
-        });
+        if (this.pendingState.undoStack.length > 0) {
+            // Pop one off of our undo stack and update our redo
+            const cells = this.pendingState.undoStack[this.pendingState.undoStack.length - 1];
+            const undoStack = this.pendingState.undoStack.slice(0, this.pendingState.undoStack.length - 1);
+            const redoStack = this.pushStack(this.pendingState.redoStack, this.pendingState.cellVMs);
+            this.sendMessage(InteractiveWindowMessages.Undo);
+            this.setState({
+                cellVMs: cells,
+                undoStack: undoStack,
+                redoStack: redoStack,
+                skipNextScroll: true
+            });
+        }
     }
 
     public deleteCell = (cellId: string) => {

src/datascience-ui/native-editor/nativeCell.tsx

@@ -325,10 +325,17 @@ export class NativeCell extends React.Component<INativeCellProps> {
                 }
                 break;
             case 'z':
-                if (!this.isFocused() && this.props.stateController.canUndo()) {
-                    e.stopPropagation();
-                    this.props.stateController.undo();
-                    this.props.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
+            case 'Z':
+                if (!this.isFocused()) {
+                    if (e.shiftKey && !e.ctrlKey && !e.altKey && this.props.stateController.canRedo()) {
+                        e.stopPropagation();
+                        this.props.stateController.redo();
+                        this.props.stateController.sendCommand(NativeCommandType.Redo, 'keyboard');
+                    } else if (!e.shiftKey && !e.altKey && !e.ctrlKey && this.props.stateController.canUndo()) {
+                        e.stopPropagation();
+                        this.props.stateController.undo();
+                        this.props.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
+                    }
                 }
                 break;
 

src/datascience-ui/native-editor/nativeEditor.tsx

@@ -365,6 +365,18 @@ export class NativeEditor extends React.Component<INativeEditorProps, IMainState
                 }
                 break;
             }
+            case 'z':
+            case 'Z':
+                if (event.shiftKey && !event.ctrlKey && !event.altKey && this.stateController.canRedo()) {
+                    event.stopPropagation();
+                    this.stateController.redo();
+                    this.stateController.sendCommand(NativeCommandType.Redo, 'keyboard');
+                } else if (!event.shiftKey && !event.altKey && !event.ctrlKey && this.stateController.canUndo()) {
+                    event.stopPropagation();
+                    this.stateController.undo();
+                    this.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
+                }
+                break;
             default:
                 break;
         }

src/test/datascience/nativeEditor.functional.test.tsx

@@ -14,7 +14,7 @@ import { Disposable, TextDocument, TextEditor, Uri, WindowState } from 'vscode';
 
 import { IApplicationShell, IDocumentManager } from '../../client/common/application/types';
 import { IFileSystem } from '../../client/common/platform/types';
-import { createDeferred } from '../../client/common/utils/async';
+import { createDeferred, sleep, waitForPromise } from '../../client/common/utils/async';
 import { createTemporaryFile } from '../../client/common/utils/fs';
 import { noop } from '../../client/common/utils/misc';
 import { Identifiers } from '../../client/datascience/constants';
@@ -939,10 +939,14 @@ for _ in range(50):
                 // Add, then undo, keep doing at least 3 times and confirm it works as expected.
                 for (let i = 0; i < 3; i += 1) {
                     // Add a new cell
-                    let update = waitForUpdate(wrapper, NativeEditor, 1);
+                    let update = waitForMessage(ioc, InteractiveWindowMessages.FocusedCellEditor);
                     simulateKeyPressOnCell(0, { code: 'a' });
                     await update;
 
+                    // Wait a bit for the time out to try and set focus a second time (this will be
+                    // fixed when we switch to redux)
+                    await sleep(100);
+
                     // There should be 4 cells and first cell is focused.
                     assert.equal(isCellSelected(wrapper, 'NativeCell', 0), false);
                     assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false);
@@ -956,6 +960,40 @@ for _ in range(50):
                     await update;
                     assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true);
 
+                    // Press 'ctrl+z'. This should do nothing
+                    update = waitForUpdate(wrapper, NativeEditor, 1);
+                    simulateKeyPressOnCell(0, { code: 'z', ctrlKey: true });
+                    await waitForPromise(update, 100);
+
+                    // There should be 4 cells and first cell is selected.
+                    assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true);
+                    assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false);
+                    assert.equal(isCellFocused(wrapper, 'NativeCell', 0), false);
+                    assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false);
+                    assert.equal(wrapper.find('NativeCell').length, 4);
+
+                    // Press 'z' to undo.
+                    update = waitForUpdate(wrapper, NativeEditor, 1);
+                    simulateKeyPressOnCell(0, { code: 'z' });
+                    await update;
+
+                    // There should be 3 cells and first cell is selected & nothing focused.
+                    assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true);
+                    assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false);
+                    assert.equal(wrapper.find('NativeCell').length, 3);
+
+                    // Press 'shift+z' to redo
+                    update = waitForUpdate(wrapper, NativeEditor, 1);
+                    simulateKeyPressOnCell(0, { code: 'z', shiftKey: true });
+                    await update;
+
+                    // There should be 4 cells and first cell is selected.
+                    assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true);
+                    assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false);
+                    assert.equal(isCellFocused(wrapper, 'NativeCell', 0), false);
+                    assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false);
+                    assert.equal(wrapper.find('NativeCell').length, 4);
+
                     // Press 'z' to undo.
                     update = waitForUpdate(wrapper, NativeEditor, 1);
                     simulateKeyPressOnCell(0, { code: 'z' });