macos: make sure we're not registering unnecessary undos

macos/Sources/Features/Terminal/BaseTerminalController.swift

@@ -260,8 +260,8 @@ class BaseTerminalController: NSWindowController,
         self.alert = alert
     }
 
-    // MARK: Focus Management
-    
+    // MARK: Split Tree Management
+
     /// Find the next surface to focus when a node is being closed.
     /// Goes to previous split unless we're the leftmost leaf, then goes to next.
     private func findNextFocusTargetAfterClosing(node: SplitTree<Ghostty.SurfaceView>.Node) -> Ghostty.SurfaceView? {
@@ -282,45 +282,63 @@ class BaseTerminalController: NSWindowController,
     ///
     /// This does no confirmation and assumes confirmation is already done.
     private func removeSurfaceNode(_ node: SplitTree<Ghostty.SurfaceView>.Node) {
-        let nextTarget = findNextFocusTargetAfterClosing(node: node)
-        let oldFocused = focusedSurface
-        let focused = node.contains { $0 == focusedSurface }
-
-        // Keep track of the old tree for undo management.
-        let oldTree = surfaceTree
-
-        // Remove the node from the tree
-        surfaceTree = surfaceTree.remove(node)
-        
         // Move focus if the closed surface was focused and we have a next target
-        if let nextTarget, focused {
+        let nextFocus: Ghostty.SurfaceView? = if node.contains(
+            where: { $0 == focusedSurface }
+        ) {
+            findNextFocusTargetAfterClosing(node: node)
+        } else {
+            nil
+        }
+
+        replaceSurfaceTree(
+            surfaceTree.remove(node),
+            moveFocusTo: nextFocus,
+            moveFocusFrom: focusedSurface,
+            undoAction: "Close Terminal"
+        )
+    }
+
+    private func replaceSurfaceTree(
+        _ newTree: SplitTree<Ghostty.SurfaceView>,
+        moveFocusTo newView: Ghostty.SurfaceView? = nil,
+        moveFocusFrom oldView: Ghostty.SurfaceView? = nil,
+        undoAction: String? = nil
+    ) {
+        // Setup our new split tree
+        let oldTree = surfaceTree
+        surfaceTree = newTree
+        if let newView {
             DispatchQueue.main.async {
-                Ghostty.moveFocus(to: nextTarget, from: oldFocused)
+                Ghostty.moveFocus(to: newView, from: oldView)
             }
         }
 
         // Setup our undo
         if let undoManager {
-            undoManager.setActionName("Close Terminal")
+            if let undoAction {
+                undoManager.setActionName(undoAction)
+            }
             undoManager.registerUndo(
                 withTarget: self,
-                expiresAfter: undoExpiration) { target in
+                expiresAfter: undoExpiration
+            ) { target in
                 target.surfaceTree = oldTree
-                if let oldFocused {
+                if let oldView {
                     DispatchQueue.main.async {
-                        Ghostty.moveFocus(to: oldFocused, from: target.focusedSurface)
+                        Ghostty.moveFocus(to: oldView, from: target.focusedSurface)
                     }
                 }
 
                 undoManager.registerUndo(
                     withTarget: target,
-                    expiresAfter: target.undoExpiration) { target in
-                    target.closeSurfaceNode(
-                        node,
-                        withConfirmation: node.contains {
-                            $0.needsConfirmQuit
-                        }
-                    )
+                    expiresAfter: target.undoExpiration
+                ) { target in
+                    target.replaceSurfaceTree(
+                        newTree,
+                        moveFocusTo: newView,
+                        moveFocusFrom: target.focusedSurface,
+                        undoAction: undoAction)
                 }
             }
         }
@@ -478,36 +496,11 @@ class BaseTerminalController: NSWindowController,
             return
         }
 
-        // Keep track of the old tree for undo
-        let oldTree = surfaceTree
-
-        // Setup our new split tree
-        surfaceTree = newTree
-        DispatchQueue.main.async {
-            Ghostty.moveFocus(to: newView, from: oldView)
-        }
-
-        // Setup our undo
-        if let undoManager {
-            undoManager.setActionName("New Split")
-            undoManager.registerUndo(
-                withTarget: self,
-                expiresAfter: undoExpiration) { target in
-                target.surfaceTree = oldTree
-                DispatchQueue.main.async {
-                    Ghostty.moveFocus(to: oldView, from: target.focusedSurface)
-                }
-                
-                undoManager.registerUndo(
-                    withTarget: target,
-                    expiresAfter: target.undoExpiration) { target in
-                    target.surfaceTree = newTree
-                    DispatchQueue.main.async {
-                        Ghostty.moveFocus(to: newView, from: target.focusedSurface)
-                    }
-                }
-            }
-        }
+        replaceSurfaceTree(
+            newTree,
+            moveFocusTo: newView,
+            moveFocusFrom: oldView,
+            undoAction: "New Split")
     }
 
     @objc private func ghosttyDidEqualizeSplits(_ notification: Notification) {

macos/Sources/Features/Terminal/TerminalController.swift

@@ -210,14 +210,18 @@ class TerminalController: BaseTerminalController {
             undoManager.setActionName("New Window")
             undoManager.registerUndo(
                 withTarget: c,
-                expiresAfter: c.undoExpiration) { target in
+                expiresAfter: c.undoExpiration
+            ) { target in
                 // Close the window when undoing
-                target.closeWindow(nil)
+                undoManager.disableUndoRegistration {
+                    target.closeWindow(nil)
+                }
 
                 // Register redo action
                 undoManager.registerUndo(
                     withTarget: ghostty,
-                    expiresAfter: target.undoExpiration) { ghostty in
+                    expiresAfter: target.undoExpiration
+                ) { ghostty in
                     _ = TerminalController.newWindow(
                         ghostty,
                         withBaseConfig: baseConfig,
@@ -314,14 +318,18 @@ class TerminalController: BaseTerminalController {
             undoManager.setActionName("New Tab")
             undoManager.registerUndo(
                 withTarget: controller,
-                expiresAfter: controller.undoExpiration) { target in
+                expiresAfter: controller.undoExpiration
+            ) { target in
                 // Close the tab when undoing
-                target.closeTab(nil)
-                
+                undoManager.disableUndoRegistration {
+                    target.closeTab(nil)
+                }
+
                 // Register redo action
                 undoManager.registerUndo(
                     withTarget: ghostty,
-                    expiresAfter: target.undoExpiration) { ghostty in
+                    expiresAfter: target.undoExpiration
+                ) { ghostty in
                     _ = TerminalController.newTab(
                         ghostty,
                         from: parent,
@@ -617,14 +625,16 @@ class TerminalController: BaseTerminalController {
             undoManager.setActionName("Close Tab")
             undoManager.registerUndo(
                 withTarget: ghostty,
-                expiresAfter: undoExpiration) { ghostty in
+                expiresAfter: undoExpiration
+            ) { ghostty in
                 let newController = TerminalController(ghostty, with: undoState)
                 
                 // Register redo action
                 undoManager.registerUndo(
                     withTarget: newController,
-                    expiresAfter: newController.undoExpiration) { target in
-                    target.closeTab(nil)
+                    expiresAfter: newController.undoExpiration
+                ) { target in
+                    target.closeTabImmediately()
                 }
             }
         }
@@ -654,7 +664,7 @@ class TerminalController: BaseTerminalController {
                 undoManager.registerUndo(
                     withTarget: newController,
                     expiresAfter: newController.undoExpiration) { target in
-                    target.closeWindow(nil)
+                    target.closeWindowImmediately()
                 }
             }
         }

macos/Sources/Helpers/ExpiringUndoManager.swift

@@ -32,6 +32,11 @@ class ExpiringUndoManager: UndoManager {
         // Ignore instantly expiring undos
         guard duration.timeInterval > 0 else { return }
 
+        // Ignore when undo registration is disabled. UndoManager still lets
+        // registration happen then cancels later but I was seeing some
+        // weird behavior with this so let's just guard on it.
+        guard self.isUndoRegistrationEnabled else { return }
+
         let expiringTarget = ExpiringTarget(
             target,
             expiresAfter: duration,
@@ -64,7 +69,10 @@ class ExpiringUndoManager: UndoManager {
         // Call super to handle standard removal
         super.removeAllActions(withTarget: target)
 
-        if !(target is ExpiringTarget) {
+        // If the target is an expiring target, remove it.
+        if let expiring = target as? ExpiringTarget {
+            expiringTargets.remove(expiring)
+        } else {
             // Find and remove any ExpiringTarget instances that wrap this target.
             expiringTargets
                 .filter { $0.target == nil || $0.target === (target as AnyObject) }

macos/Sources/Helpers/Extensions/UndoManager+Extension.swift

@@ -0,0 +1,20 @@
+import Foundation
+
+extension UndoManager {
+    /// A Boolean value that indicates whether the undo manager is currently performing
+    /// either an undo or redo operation.
+    var isUndoingOrRedoing: Bool {
+        isUndoing || isRedoing
+    }
+
+    /// Temporarily disables undo registration while executing the provided handler.
+    ///
+    /// This method provides a convenient way to perform operations without recording them
+    /// in the undo stack. It ensures that undo registration is properly re-enabled even
+    /// if the handler throws an error.
+    func disableUndoRegistration(handler: () -> Void) {
+        disableUndoRegistration()
+        handler()
+        enableUndoRegistration()
+    }
+}