mirrors/ghostty
1
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2025-09-05 19:08:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

macos: disambiguate close tab vs close window for confirmation (#7618)

Browse Source 
This fixes an issue where pressing the red close button in a window or
the "x" button on a tab couldn't differentiate and would always close
the tab or close the window (depending on tab counts).

It seems like in both cases, AppKit triggers the `windowShouldClose`
delegate method on the controller, but for the close window case it
triggers this on ALL the windows in the group, not just the one that was
clicked.

I implemented a kind of silly coordinator that debounces
`windowShouldClose` calls over 100ms and uses that to differentiate
between the two cases.
This commit is contained in:
Mitchell Hashimoto
2025-06-17 16:24:46 -07:00
committed by GitHub
parent 7d01332574 51b9fa751a
commit 30a8ba1bf6
4 changed files with 148 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
macos/Ghostty.xcodeproj/project.pbxproj
View File

@@ -119,6 +119,7 @@
A5E112932AF73E6E00C6E0C2 /* ClipboardConfirmation.xib in Resources */ = {isa = PBXBuildFile; fileRef = A5E112922AF73E6E00C6E0C2 /* ClipboardConfirmation.xib */; };
A5E112952AF73E8A00C6E0C2 /* ClipboardConfirmationController.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5E112942AF73E8A00C6E0C2 /* ClipboardConfirmationController.swift */; };
A5E112972AF7401B00C6E0C2 /* ClipboardConfirmationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5E112962AF7401B00C6E0C2 /* ClipboardConfirmationView.swift */; };
A5E4082A2E022E9E0035FEAC /* TabGroupCloseCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5E408292E022E9B0035FEAC /* TabGroupCloseCoordinator.swift */; };
A5FEB3002ABB69450068369E /* main.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5FEB2FF2ABB69450068369E /* main.swift */; };
AEE8B3452B9AA39600260C5E /* NSPasteboard+Extension.swift in Sources */ = {isa = PBXBuildFile; fileRef = AEE8B3442B9AA39600260C5E /* NSPasteboard+Extension.swift */; };
C159E81D2B66A06B00FDFE9C /* OSColor+Extension.swift in Sources */ = {isa = PBXBuildFile; fileRef = C159E81C2B66A06B00FDFE9C /* OSColor+Extension.swift */; };
@@ -238,6 +239,7 @@
A5E112922AF73E6E00C6E0C2 /* ClipboardConfirmation.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = ClipboardConfirmation.xib; sourceTree = "<group>"; };
A5E112942AF73E8A00C6E0C2 /* ClipboardConfirmationController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipboardConfirmationController.swift; sourceTree = "<group>"; };
A5E112962AF7401B00C6E0C2 /* ClipboardConfirmationView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipboardConfirmationView.swift; sourceTree = "<group>"; };
A5E408292E022E9B0035FEAC /* TabGroupCloseCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabGroupCloseCoordinator.swift; sourceTree = "<group>"; };
A5FEB2FF2ABB69450068369E /* main.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = main.swift; sourceTree = "<group>"; };
AEE8B3442B9AA39600260C5E /* NSPasteboard+Extension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NSPasteboard+Extension.swift"; sourceTree = "<group>"; };
C159E81C2B66A06B00FDFE9C /* OSColor+Extension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OSColor+Extension.swift"; sourceTree = "<group>"; };
@@ -320,12 +322,13 @@
A5333E1B2B5A1CE3008AEFF7 /* CrossKit.swift */,
A5CBD0572C9F30860017A1AE /* Cursor.swift */,
A5D0AF3C2B37804400D21823 /* CodableBridge.swift */,
A5CBD0552C9E65A50017A1AE /* DraggableWindowView.swift */,
A58636702DF298F700E04A10 /* ExpiringUndoManager.swift */,
A52FFF582CAA4FF1000C6A5B /* Fullscreen.swift */,
A59630962AEE163600D64628 /* HostingWindow.swift */,
A5CA378B2D2A4DE800931030 /* KeyboardLayout.swift */,
A59FB5D02AE0DEA7009128F3 /* MetalView.swift */,
A5CBD0552C9E65A50017A1AE /* DraggableWindowView.swift */,
A5E408292E022E9B0035FEAC /* TabGroupCloseCoordinator.swift */,
A5CA378D2D31D6C100931030 /* Weak.swift */,
C1F26EE72B76CBFC00404083 /* VibrantLayer.h */,
C1F26EE82B76CBFC00404083 /* VibrantLayer.m */,
@@ -792,6 +795,7 @@
A599CDB02CF103F60049FA26 /* NSAppearance+Extension.swift in Sources */,
A52FFF572CA90484000C6A5B /* QuickTerminalScreen.swift in Sources */,
A5CC36132C9CD72D004D6760 /* SecureInputOverlay.swift in Sources */,
A5E4082A2E022E9E0035FEAC /* TabGroupCloseCoordinator.swift in Sources */,
A535B9DA299C569B0017E2E4 /* ErrorView.swift in Sources */,
A53A29882DB69D2F00B6E02C /* TerminalCommandPalette.swift in Sources */,
A51BFC202B2FB64F00E92F16 /* AboutController.swift in Sources */,

21
macos/Sources/Features/Terminal/TerminalController.swift
View File

@@ -5,7 +5,7 @@ import Combine
import GhosttyKit
/// A classic, tabbed terminal experience.
class TerminalController: BaseTerminalController {
class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Controller {
override var windowNibName: NSNib.Name? {
let defaultValue = "Terminal"
@@ -882,14 +882,20 @@ class TerminalController: BaseTerminalController {
ghostty.newTab(surface: surface)
}
//MARK: - NSWindowDelegate
// MARK: NSWindowDelegate
// TabGroupCloseCoordinator.Controller
lazy private(set) var tabGroupCloseCoordinator = TabGroupCloseCoordinator()
override func windowShouldClose(_ sender: NSWindow) -> Bool {
// If we have tabs, then this should only close the tab.
if window?.tabGroup?.windows.count ?? 0 > 1 {
closeTab(sender)
} else {
closeWindow(sender)
tabGroupCloseCoordinator.windowShouldClose(sender) { [weak self] scope in
guard let self else { return }
switch (scope) {
case .tab: closeTab(nil)
case .window:
guard self.window?.isFirstWindowInTabGroup ?? false else { return }
closeWindow(nil)
}
}
// We will always explicitly close the window using the above
@@ -1264,4 +1270,3 @@ extension TerminalController: NSMenuItemValidation {
}
}
}

6
macos/Sources/Helpers/Extensions/NSWindow+Extension.swift
View File

@@ -9,4 +9,10 @@ extension NSWindow {
guard windowNumber > 0 else { return nil }
return CGWindowID(windowNumber)
}
/// True if this is the first window in the tab group.
var isFirstWindowInTabGroup: Bool {
guard let firstWindow = tabGroup?.windows.first else { return true }
return firstWindow === self
}
}

124
macos/Sources/Helpers/TabGroupCloseCoordinator.swift Normal file
View File

@@ -0,0 +1,124 @@
import AppKit
/// Coordinates close operations for windows that are part of a tab group.
///
/// This coordinator helps distinguish between closing a single tab versus closing
/// an entire window (with all its tabs). When macOS native tabs are used, close
/// operations can be ambiguous - this coordinator tracks close requests across
/// multiple windows in a tab group to determine the user's intent.
class TabGroupCloseCoordinator {
/// The scope of a close operation.
enum CloseScope {
case tab
case window
}
/// Protocol that window controllers must implement to use the coordinator.
protocol Controller {
/// The tab group close coordinator instance for this controller.
var tabGroupCloseCoordinator: TabGroupCloseCoordinator { get }
}
/// Callback type for close operations.
typealias Callback = (CloseScope) -> Void
// We use weak vars and ObjectIdentifiers below because we don't want to
// create any strong reference cycles during coordination.
/// The tab group being coordinated. Weak reference to avoid cycles.
private weak var tabGroup: NSWindowTabGroup?
/// Map of window identifiers to their close callbacks.
private var closeRequests: [ObjectIdentifier: Callback] = [:]
/// Timer used to debounce close requests and determine intent.
private var debounceTimer: Timer?
deinit {
trigger(.tab)
}
/// Call this from the windowShouldClose override in order to track whether
/// a window close event is from a tab or a window. If this window already
/// requested a close then only the latest will be called.
func windowShouldClose(
_ window: NSWindow,
callback: @escaping Callback
) {
// If this window isn't part of a tab group we assume its a window
// close for the window and let our timer keep running for the rest.
guard let tabGroup = window.tabGroup else {
callback(.window)
return
}
// Forward to the proper coordinator
if let firstController = tabGroup.windows.first?.windowController as? Controller,
firstController.tabGroupCloseCoordinator !== self {
let coordinator = firstController.tabGroupCloseCoordinator
coordinator.windowShouldClose(window, callback: callback)
return
}
// If our tab group is nil then we either are seeing this for the first
// time or our weak ref expired and we should fire our callbacks.
if self.tabGroup == nil {
self.tabGroup = tabGroup
debounceTimer?.fire()
debounceTimer = nil
}
// No matter what, we cancel our debounce and restart this. This opens
// us up to a DoS if close requests are looped but this would only
// happen in hostile scenarios that are self-inflicted.
debounceTimer?.invalidate()
debounceTimer = nil
// If this tab group doesn't match then I don't really know what to
// do. This shouldn't happen. So we just assume it's a tab close
// and trigger the rest. No right answer here as far as I know.
if self.tabGroup != tabGroup {
callback(.tab)
trigger(.tab)
return
}
// Add the request
closeRequests[ObjectIdentifier(window)] = callback
// If close requests matches all our windows then we are done.
if closeRequests.count == tabGroup.windows.count {
let allWindows = Set(tabGroup.windows.map { ObjectIdentifier($0) })
if Set(closeRequests.keys) == allWindows {
trigger(.window)
return
}
}
// Setup our new timer
debounceTimer = Timer.scheduledTimer(
withTimeInterval: Duration.milliseconds(100).timeInterval,
repeats: false
) { [weak self] _ in
self?.trigger(.tab)
}
}
/// Triggers all pending close callbacks with the given scope.
///
/// This method is called when the coordinator has determined the user's intent
/// (either closing a tab or the entire window). It executes all pending callbacks
/// and resets the coordinator's state.
///
/// - Parameter scope: The determined scope of the close operation.
private func trigger(_ scope: CloseScope) {
// Reset our state
tabGroup = nil
debounceTimer?.invalidate()
debounceTimer = nil
// Trigger all of our callbacks
closeRequests.forEach { $0.value(scope) }
closeRequests = [:]
}
}