diff --git a/macos/Ghostty.xcodeproj/project.pbxproj b/macos/Ghostty.xcodeproj/project.pbxproj index 153ec8e6f..67f1784ac 100644 --- a/macos/Ghostty.xcodeproj/project.pbxproj +++ b/macos/Ghostty.xcodeproj/project.pbxproj @@ -62,8 +62,8 @@ A586365F2DEE6C2300E04A10 /* SplitTree.swift in Sources */ = {isa = PBXBuildFile; fileRef = A586365E2DEE6C2100E04A10 /* SplitTree.swift */; }; A58636662DEF964100E04A10 /* TerminalSplitTreeView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A58636652DEF963F00E04A10 /* TerminalSplitTreeView.swift */; }; A586366B2DF0A98C00E04A10 /* Array+Extension.swift in Sources */ = {isa = PBXBuildFile; fileRef = A586366A2DF0A98900E04A10 /* Array+Extension.swift */; }; - A586366D2DF25C2500E04A10 /* ExpiringTarget.swift in Sources */ = {isa = PBXBuildFile; fileRef = A586366C2DF25C1C00E04A10 /* ExpiringTarget.swift */; }; A586366F2DF25D8600E04A10 /* Duration+Extension.swift in Sources */ = {isa = PBXBuildFile; fileRef = A586366E2DF25D8300E04A10 /* Duration+Extension.swift */; }; + A58636712DF298FB00E04A10 /* ExpiringUndoManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = A58636702DF298F700E04A10 /* ExpiringUndoManager.swift */; }; A5874D992DAD751B00E83852 /* CGS.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5874D982DAD751A00E83852 /* CGS.swift */; }; A5874D9D2DAD786100E83852 /* NSWindow+Extension.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5874D9C2DAD785F00E83852 /* NSWindow+Extension.swift */; }; A59444F729A2ED5200725BBA /* SettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A59444F629A2ED5200725BBA /* SettingsView.swift */; }; @@ -170,8 +170,8 @@ A586365E2DEE6C2100E04A10 /* SplitTree.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SplitTree.swift; sourceTree = ""; }; A58636652DEF963F00E04A10 /* TerminalSplitTreeView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalSplitTreeView.swift; sourceTree = ""; }; A586366A2DF0A98900E04A10 /* Array+Extension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Array+Extension.swift"; sourceTree = ""; }; - A586366C2DF25C1C00E04A10 /* ExpiringTarget.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExpiringTarget.swift; sourceTree = ""; }; A586366E2DF25D8300E04A10 /* Duration+Extension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Duration+Extension.swift"; sourceTree = ""; }; + A58636702DF298F700E04A10 /* ExpiringUndoManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExpiringUndoManager.swift; sourceTree = ""; }; A5874D982DAD751A00E83852 /* CGS.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CGS.swift; sourceTree = ""; }; A5874D9C2DAD785F00E83852 /* NSWindow+Extension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NSWindow+Extension.swift"; sourceTree = ""; }; A59444F629A2ED5200725BBA /* SettingsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsView.swift; sourceTree = ""; }; @@ -294,7 +294,6 @@ A534263D2A7DCBB000EBB7A2 /* Helpers */ = { isa = PBXGroup; children = ( - A586366C2DF25C1C00E04A10 /* ExpiringTarget.swift */, A58636692DF0A98100E04A10 /* Extensions */, A5874D9B2DAD781100E83852 /* Private */, A5AEB1642D5BE7BF00513529 /* LastWindowPosition.swift */, @@ -303,6 +302,7 @@ A5333E1B2B5A1CE3008AEFF7 /* CrossKit.swift */, A5CBD0572C9F30860017A1AE /* Cursor.swift */, A5D0AF3C2B37804400D21823 /* CodableBridge.swift */, + A58636702DF298F700E04A10 /* ExpiringUndoManager.swift */, A52FFF582CAA4FF1000C6A5B /* Fullscreen.swift */, A59630962AEE163600D64628 /* HostingWindow.swift */, A5CA378B2D2A4DE800931030 /* KeyboardLayout.swift */, @@ -708,6 +708,7 @@ A53A29812DB44A6100B6E02C /* KeyboardShortcut+Extension.swift in Sources */, A5CBD0562C9E65B80017A1AE /* DraggableWindowView.swift in Sources */, C1F26EE92B76CBFC00404083 /* VibrantLayer.m in Sources */, + A58636712DF298FB00E04A10 /* ExpiringUndoManager.swift in Sources */, A59630972AEE163600D64628 /* HostingWindow.swift in Sources */, A59630A02AEF6AEB00D64628 /* TerminalManager.swift in Sources */, A51BFC2B2B30F6BE00E92F16 /* UpdateDelegate.swift in Sources */, @@ -720,7 +721,6 @@ A52FFF5B2CAA54B1000C6A5B /* FullscreenMode+Extension.swift in Sources */, A5333E222B5A2128008AEFF7 /* SurfaceView_AppKit.swift in Sources */, A5CA378E2D31D6C300931030 /* Weak.swift in Sources */, - A586366D2DF25C2500E04A10 /* ExpiringTarget.swift in Sources */, A5CDF1952AAFA19600513312 /* ConfigurationErrorsView.swift in Sources */, A55B7BBC29B6FC330055DE60 /* SurfaceView.swift in Sources */, A5333E1C2B5A1CE3008AEFF7 /* CrossKit.swift in Sources */, diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index d12b2efd2..eae8dd121 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -91,7 +91,7 @@ class AppDelegate: NSObject, let terminalManager: TerminalManager /// The global undo manager for app-level state such as window restoration. - lazy var undoManager = UndoManager() + lazy var undoManager = ExpiringUndoManager() /// Our quick terminal. This starts out uninitialized and only initializes if used. private var quickController: QuickTerminalController? = nil diff --git a/macos/Sources/Features/Terminal/BaseTerminalController.swift b/macos/Sources/Features/Terminal/BaseTerminalController.swift index cd7ceffbb..6cc6b2ec8 100644 --- a/macos/Sources/Features/Terminal/BaseTerminalController.swift +++ b/macos/Sources/Features/Terminal/BaseTerminalController.swift @@ -75,11 +75,25 @@ class BaseTerminalController: NSWindowController, /// The cancellables related to our focused surface. private var focusedSurfaceCancellables: Set = [] + /// The time that undo/redo operations that contain running ptys are valid for. + private var undoExpiration: Duration { + .seconds(5) + } + /// The undo manager for this controller is the undo manager of the window, /// which we set via the delegate method. - override var undoManager: UndoManager? { + override var undoManager: ExpiringUndoManager? { // This should be set via the delegate method windowWillReturnUndoManager - window?.undoManager + if let result = window?.undoManager as? ExpiringUndoManager { + return result + } + + // If the window one isn't set, we fallback to our global one. + if let appDelegate = NSApplication.shared.delegate as? AppDelegate { + return appDelegate.undoManager + } + + return nil } struct SavedFrame { @@ -173,7 +187,7 @@ class BaseTerminalController: NSWindowController, deinit { NotificationCenter.default.removeObserver(self) - + undoManager?.removeAllActions(withTarget: self) if let eventMonitor { NSEvent.removeMonitor(eventMonitor) } @@ -284,20 +298,20 @@ class BaseTerminalController: NSWindowController, // Setup our undo if let undoManager { undoManager.setActionName("Close Terminal") - undoManager.registerUndo(withTarget: ExpiringTarget( - with: .seconds(5), - in: undoManager, - )) { [weak self] v in - guard let self else { return } - self.surfaceTree = oldTree + undoManager.registerUndo( + withTarget: self, + expiresAfter: undoExpiration) { target in + target.surfaceTree = oldTree if let oldFocused { DispatchQueue.main.async { - Ghostty.moveFocus(to: oldFocused, from: self.focusedSurface) + Ghostty.moveFocus(to: oldFocused, from: target.focusedSurface) } } - undoManager.registerUndo(withTarget: NSObject()) { [weak self] _ in - self?.closeSurface( + undoManager.registerUndo( + withTarget: target, + expiresAfter: target.undoExpiration) { target in + target.closeSurface( node.leftmostLeaf(), withConfirmation: node.contains { $0.needsConfirmQuit @@ -446,9 +460,12 @@ class BaseTerminalController: NSWindowController, let newView = Ghostty.SurfaceView(ghostty_app, baseConfig: config) // Do the split + let newTree: SplitTree do { - let newTree = try surfaceTree.insert(view: newView, at: oldView, direction: splitDirection) - surfaceTree = newTree + newTree = try surfaceTree.insert( + view: newView, + at: oldView, + direction: splitDirection) } catch { // If splitting fails for any reason (it should not), then we just log // and return. The new view we created will be deinitialized and its @@ -457,9 +474,36 @@ class BaseTerminalController: NSWindowController, return } + // Keep track of the old tree for undo + let oldTree = surfaceTree - // Once we've split, we need to move focus to the new split - Ghostty.moveFocus(to: newView, from: oldView) + // 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) + } + } + } + } } @objc private func ghosttyDidEqualizeSplits(_ notification: Notification) { @@ -836,6 +880,9 @@ class BaseTerminalController: NSWindowController, // the view and the window so we had to nil this out to break it but I think this // may now be resolved. We should verify that no memory leaks and we can remove this. window.contentView = nil + + // Make sure we clean up all our undos + window.undoManager?.removeAllActions(withTarget: self) } func windowDidBecomeKey(_ notification: Notification) { diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 682efa947..6e35f40d1 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -287,6 +287,8 @@ extension Ghostty { if let surface = self.surface { ghostty_surface_free(surface) } + + Ghostty.logger.warning("WOW close") } func focusDidChange(_ focused: Bool) { diff --git a/macos/Sources/Helpers/ExpiringTarget.swift b/macos/Sources/Helpers/ExpiringTarget.swift deleted file mode 100644 index d24021495..000000000 --- a/macos/Sources/Helpers/ExpiringTarget.swift +++ /dev/null @@ -1,53 +0,0 @@ -import AppKit - -/// A target object for UndoManager that automatically expires after a specified duration. -/// -/// ExpiringTarget holds a reference to a target object and removes all undo actions -/// associated with itself from the UndoManager when the timer expires. This is useful -/// for creating temporary undo operations that should not persist beyond a certain time. -/// -/// The parameter T can be used to retain a reference to some target value -/// that can be used in the undo operation. The target is released when the timer expires. -/// -/// - Parameter T: The type of the target object, constrained to AnyObject -class ExpiringTarget { - private(set) var target: T? - private var timer: Timer? - private weak var undoManager: UndoManager? - - /// Creates an expiring target that will automatically remove undo actions after the specified duration. - /// - /// - Parameters: - /// - target: The target object to hold weakly. Defaults to nil. - /// - duration: The time after which the target should expire - /// - undoManager: The UndoManager from which to remove actions when expired - init(_ target: T? = nil, with duration: Duration, in undoManager: UndoManager) { - self.target = target - self.undoManager = undoManager - self.timer = Timer.scheduledTimer( - withTimeInterval: duration.timeInterval, - repeats: false) { _ in - self.expire() - } - } - - /// Manually expires the target, removing all associated undo actions and invalidating the timer. - /// - /// This method is called automatically when the timer fires, but can also be called manually - /// to expire the target before the timer duration has elapsed. - func expire() { - target = nil - undoManager?.removeAllActions(withTarget: self) - timer?.invalidate() - } - - deinit { - expire() - } -} - -extension ExpiringTarget where T == NSObject { - convenience init(with duration: Duration, in undoManager: UndoManager) { - self.init(nil, with: duration, in: undoManager) - } -} diff --git a/macos/Sources/Helpers/ExpiringUndoManager.swift b/macos/Sources/Helpers/ExpiringUndoManager.swift new file mode 100644 index 000000000..3eda56182 --- /dev/null +++ b/macos/Sources/Helpers/ExpiringUndoManager.swift @@ -0,0 +1,137 @@ +/// An UndoManager subclass that supports registering undo operations that automatically expire after a specified duration. +/// +/// This class extends the standard UndoManager to add time-based expiration for undo operations. +/// When an undo operation expires, it is automatically removed from the undo stack and cannot be invoked. +/// +/// Example usage: +/// ```swift +/// let undoManager = ExpiringUndoManager() +/// undoManager.registerUndo(withTarget: myObject, expiresAfter: .seconds(30)) { target in +/// // Undo operation that expires after 30 seconds +/// target.restorePreviousState() +/// } +/// ``` +class ExpiringUndoManager: UndoManager { + /// The set of expiring targets so we can properly clean them up when removeAllActions + /// is called with the real target. + private lazy var expiringTargets: Set = [] + + /// Registers an undo operation that automatically expires after the specified duration. + /// + /// - Parameters: + /// - target: The target object for the undo operation. The undo operation will be removed + /// if this object is deallocated before the operation is invoked. + /// - duration: The duration after which the undo operation should expire and be removed from the undo stack. + /// - handler: The closure to execute when the undo operation is invoked. The closure receives + /// the target object as its parameter. + func registerUndo( + withTarget target: TargetType, + expiresAfter duration: Duration, + handler: @escaping (TargetType) -> Void + ) { + let expiringTarget = ExpiringTarget( + target, + expiresAfter: duration, + in: self) + expiringTargets.insert(expiringTarget) + + super.registerUndo(withTarget: expiringTarget) { [weak self] expiringTarget in + self?.expiringTargets.remove(expiringTarget) + guard let target = expiringTarget.target as? TargetType else { return } + handler(target) + } + } + + /// Removes all undo and redo operations from the undo manager. + /// + /// This override ensures that all expiring targets are also cleared when + /// the undo manager is reset. + override func removeAllActions() { + super.removeAllActions() + expiringTargets = [] + } + + /// Removes all undo and redo operations involving the specified target. + /// + /// This override ensures that when actions are removed for a target, any associated + /// expiring targets are also properly cleaned up. + /// + /// - Parameter target: The target object whose actions should be removed. + override func removeAllActions(withTarget target: Any) { + // Call super to handle standard removal + super.removeAllActions(withTarget: target) + + if !(target is ExpiringTarget) { + // Find and remove any ExpiringTarget instances that wrap this target. + expiringTargets + .filter { $0.target == nil || $0.target === (target as AnyObject) } + .forEach { + // Technically they'll always expire when they get deinitialized + // but we want to make sure it happens right now. + $0.expire() + expiringTargets.remove($0) + } + } + } +} + +/// A target object for ExpiringUndoManager that removes itself from the +/// undo manager after it expires. +/// +/// This class acts as a proxy for the real target object in undo operations. +/// It holds a weak reference to the actual target and automatically removes +/// all associated undo operations when either: +/// - The specified duration expires +/// - The ExpiringTarget instance is deallocated +/// - The expire() method is called manually +private class ExpiringTarget { + /// The actual target object for the undo operation, held weakly to avoid retain cycles. + private(set) weak var target: AnyObject? + + /// Timer that triggers expiration after the specified duration. + private var timer: Timer? + + /// The undo manager from which to remove actions when this target expires. + private weak var undoManager: UndoManager? + + /// Creates an expiring target that will automatically remove undo actions after the specified duration. + /// + /// - Parameters: + /// - target: The target object to hold weakly. + /// - duration: The time after which the target should expire. + /// - undoManager: The UndoManager from which to remove actions when expired. + init(_ target: AnyObject? = nil, expiresAfter duration: Duration, in undoManager: UndoManager) { + self.target = target + self.undoManager = undoManager + self.timer = Timer.scheduledTimer( + withTimeInterval: duration.timeInterval, + repeats: false) { [weak self] _ in + self?.expire() + } + } + + /// Manually expires the target, removing all associated undo actions and invalidating the timer. + /// + /// This method is called automatically when the timer fires, but can also be called manually + /// to expire the target before the timer duration has elapsed. + func expire() { + target = nil + undoManager?.removeAllActions(withTarget: self) + timer?.invalidate() + timer = nil + } + + deinit { + expire() + } +} + +extension ExpiringTarget: Hashable, Equatable { + static func == (lhs: ExpiringTarget, rhs: ExpiringTarget) -> Bool { + return lhs === rhs + } + + func hash(into hasher: inout Hasher) { + hasher.combine(ObjectIdentifier(self)) + } +}