From 3f76094d84135c638bdc10e6c0857b6ca3d24666 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 7 Dec 2023 14:39:54 -0800
Subject: [PATCH] macos: handle the "+" button automatically adding the window
 to the tabs

Fixes #1010
---
 .../Terminal/TerminalController.swift         | 29 ++++++-------------
 .../Features/Terminal/TerminalManager.swift   | 21 +++++++++++++-
 2 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift
index c4c2752a2..1b80adfff 100644
--- a/macos/Sources/Features/Terminal/TerminalController.swift
+++ b/macos/Sources/Features/Terminal/TerminalController.swift
@@ -167,15 +167,18 @@ class TerminalController: NSWindowController, NSWindowDelegate,
             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.
+        // In various situations, macOS automatically tabs new windows. Ghostty handles
+        // its own tabbing so we DONT want this behavior. This detects this scenario and undoes
+        // it.
+        //
+        // Example scenarios where this happens:
+        //   - When the system user tabbing preference is "always"
+        //   - When the "+" button in the tab bar is clicked
         //
         // 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 (!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).
@@ -189,21 +192,7 @@ class TerminalController: NSWindowController, NSWindowDelegate,
     override func newWindowForTab(_ sender: Any?) {
         // Trigger the ghostty core event logic for a new tab.
         guard let surface = self.focusedSurface?.surface else { return }
-        
-        // This is necessary due to a really strange issue on macOS that I was never
-        // able to figure out: if we create a new tab in this function directly, then
-        // it is incorrectly added to the wrong index in `tabGroup.windows`. The macOS
-        // `tabGroup.windows` documentation says that will always be in visual order
-        // of the tab but I was finding that not to be true. By introducing a small delay,
-        // I noticed everything works. I can't explain it and I'd rather not do this so
-        // if someone can figure this out that'd be great.
-        //
-        // See: https://github.com/mitchellh/ghostty/issues/1010
-        // Last reproduced on macOS 14.1
-        DispatchQueue.main.asyncAfter(deadline: .now() + 0.01) { [weak self] in
-            guard let s = self else { return }
-            s.ghostty.newTab(surface: surface)
-        }
+        ghostty.newTab(surface: surface)
     }
     
     //MARK: - NSWindowDelegate
diff --git a/macos/Sources/Features/Terminal/TerminalManager.swift b/macos/Sources/Features/Terminal/TerminalManager.swift
index 1720c4a10..6fce98e41 100644
--- a/macos/Sources/Features/Terminal/TerminalManager.swift
+++ b/macos/Sources/Features/Terminal/TerminalManager.swift
@@ -90,14 +90,33 @@ class TerminalManager {
     
     private func newTab(to parent: NSWindow, withBaseConfig base: Ghostty.SurfaceConfiguration?) {
         // Create a new window and add it to the parent
-        let window = createWindow(withBaseConfig: base).window!
+        let controller = createWindow(withBaseConfig: base)
+        let window = controller.window!
         
         // If the parent is miniaturized, then macOS exhibits really strange behaviors
         // so we have to bring it back out.
         if (parent.isMiniaturized) { parent.deminiaturize(self) }
         
+        // If our parent tab group already has this window, macOS added it and
+        // we need to remove it so we can set the correct order in the next line.
+        // If we don't do this, macOS gets really confused and the tabbedWindows
+        // state becomes incorrect.
+        //
+        // At the time of writing this code, the only known case this happens
+        // is when the "+" button is clicked in the tab bar.
+        if let tg = parent.tabGroup, tg.windows.firstIndex(of: window) != nil {
+            tg.removeWindow(window)
+        }
+        
+        // Add the window to the tab group and show it
         parent.addTabbedWindow(window, ordered: .above)
         window.makeKeyAndOrderFront(self)
+        
+        // It takes an event loop cycle until the macOS tabGroup state becomes
+        // consistent which causes our tab labeling to be off when the "+" button
+        // is used in the tab bar. This fixes that. If we can find a more robust
+        // solution we should do that.
+        DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { controller.relabelTabs() }
     }
     
     /// Creates a window controller, adds it to our managed list, and returns it.