From 7d9eaddeb313c5039dce731956d820c747ddc426 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 18 Nov 2023 21:09:08 -0800 Subject: [PATCH] macos: alternate solution to ignoring "always" userTabbingPreference Related to #642 Fixes #910 See #642 for why we want to ignore the "always" userTabbingPreference. To do that, we'd set tabbingMode to "disallow" because we manually (in code) handled it all. Unfortunately, setting the tabbingMode to "disallow" introduce #910. I still believe this is a macOS bug at heart, so I'm going to submit an Apple Feedback item for it. However, I've found a workaround which I also feel is the better solution, implemented here. Instead of setting tabbingMode to "disallow" I now detect if we're in the scenario where the user has their system tabbing preference set to "always". In that case, we detect if the new window has been automatically put into a tab group by macOS, and if so we remove it. This all happens in the `windowDidLoad` controller callback. At this phase, our Ghostty-managed windows should NEVER be in a tab group, because "new tab" adds them to a tab _after_. So we can be certain that if we're already in a tab group it was from the macOS system setting. This happens to fix #910. --- .../Terminal/TerminalController.swift | 42 ++++++++----------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index 517b4becb..2f5772324 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -111,10 +111,6 @@ class TerminalController: NSWindowController, NSWindowDelegate, // If window decorations are disabled, remove our title if (!ghostty.windowDecorations) { window.styleMask.remove(.titled) } - // If we aren't in full screen, then we want to disable tabbing (see comment - // in the delegate function) - if (!window.styleMask.contains(.fullScreen)) { disableTabbing() } - // Terminals typically operate in sRGB color space and macOS defaults // to "native" which is typically P3. There is a lot more resources // covered in thie GitHub issue: https://github.com/mitchellh/ghostty/pull/376 @@ -130,6 +126,23 @@ class TerminalController: NSWindowController, NSWindowDelegate, viewModel: self, delegate: self )) + + // If the user tabbing preference is always, then macOS automatically tabs + // all new windows. Ghostty handles its own tabbing so we DONT want this behavior. + // This detects this scenario and undoes it. + // + // We don't run this logic in fullscreen because in fullscreen this will end up + // removing the window and putting it into its own dedicated fullscreen, which is not + // the expected or desired behavior of anyone I've found. + if (NSWindow.userTabbingPreference == .always && + !window.styleMask.contains(.fullScreen)) { + // If we have more than 1 window in our tab group we know we're a new window. + // Since Ghostty manages tabbing manually this will never be more than one + // at this point in the AppKit lifecycle (we add to the group after this). + if let tabGroup = window.tabGroup, tabGroup.windows.count > 1 { + window.tabGroup?.removeWindow(window) + } + } } // Shows the "+" button in the tab bar, responds to that click. @@ -193,27 +206,6 @@ class TerminalController: NSWindowController, NSWindowDelegate, func windowDidBecomeKey(_ notification: Notification) { self.relabelTabs() } - - func windowWillExitFullScreen(_ notification: Notification) { - // See comment in this function - disableTabbing() - } - - func windowWillEnterFullScreen(_ notification: Notification) { - // We re-enable the automatic tabbing mode when we enter full screen otherwise - // every new tab also enters a new screen. - guard let window = self.window else { return } - window.tabbingMode = .automatic - } - - private func disableTabbing() { - // For new windows, explicitly disallow tabbing with other windows. - // This overrides the value of userTabbingPreference. Rationale: - // Ghostty provides separate "New Tab" and "New Window" actions so - // there's no reason to make "New Window" open in a tab. - guard let window = self.window else { return } - window.tabbingMode = .disallowed; - } //MARK: - First Responder