From de666ff05b0e46463fdd77384c489fd094bba8d0 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Thu, 19 Feb 2026 17:10:27 -0800 Subject: [PATCH] Fix split blackout race and stabilize focus handoff --- Sources/GhosttyTerminalView.swift | 133 +++++++++++++++++++----- Sources/Panels/BrowserPanelView.swift | 17 ++- Sources/Panels/TerminalPanel.swift | 3 + Sources/TerminalController.swift | 17 ++- Sources/TerminalWindowPortal.swift | 109 +++++++++++++++++-- Sources/Workspace.swift | 94 ++++++++++++++++- tests/test_split_flash_and_layout.py | 75 ++++++++++++- tests_v2/test_split_flash_and_layout.py | 72 +++++++++++++ 8 files changed, 476 insertions(+), 44 deletions(-) diff --git a/Sources/GhosttyTerminalView.swift b/Sources/GhosttyTerminalView.swift index bf360148..cca30dd6 100644 --- a/Sources/GhosttyTerminalView.swift +++ b/Sources/GhosttyTerminalView.swift @@ -1120,9 +1120,12 @@ final class TerminalSurface: Identifiable, ObservableObject { } func attachToView(_ view: GhosttyNSView) { - #if DEBUG - print("[TerminalSurface] attachToView: \(id) attachedView=\(attachedView != nil) surface=\(surface != nil)") - #endif +#if DEBUG + dlog( + "surface.attach surface=\(id.uuidString.prefix(5)) view=\(Unmanaged.passUnretained(view).toOpaque()) " + + "attached=\(attachedView != nil ? 1 : 0) hasSurface=\(surface != nil ? 1 : 0) inWindow=\(view.window != nil ? 1 : 0)" + ) +#endif // If already attached to this view, nothing to do. // Still re-assert the display id: during split close tree restructuring, the view can be @@ -1130,9 +1133,9 @@ final class TerminalSurface: Identifiable, ObservableObject { // Ghostty's vsync-driven renderer depends on having a valid display id; if it is missing // or stale, the surface can appear visually frozen until a focus/visibility change. if attachedView === view && surface != nil { - #if DEBUG - print("[TerminalSurface] attachToView: same view and surface exists") - #endif +#if DEBUG + dlog("surface.attach.reuse surface=\(id.uuidString.prefix(5)) view=\(Unmanaged.passUnretained(view).toOpaque())") +#endif if let screen = view.window?.screen ?? NSScreen.main, let displayID = screen.displayID, displayID != 0, @@ -1144,9 +1147,12 @@ final class TerminalSurface: Identifiable, ObservableObject { } if let attachedView, attachedView !== view { - #if DEBUG - print("[TerminalSurface] attachToView: different view, returning") - #endif +#if DEBUG + dlog( + "surface.attach.skip surface=\(id.uuidString.prefix(5)) reason=alreadyAttachedToDifferentView " + + "current=\(Unmanaged.passUnretained(attachedView).toOpaque()) new=\(Unmanaged.passUnretained(view).toOpaque())" + ) +#endif return } @@ -1155,20 +1161,31 @@ final class TerminalSurface: Identifiable, ObservableObject { // If surface doesn't exist yet, create it once the view is in a real window so // content scale and pixel geometry are derived from the actual backing context. if surface == nil { - guard view.window != nil else { return } - #if DEBUG - print("[TerminalSurface] attachToView: creating surface for \(id)") - #endif + guard view.window != nil else { +#if DEBUG + dlog( + "surface.attach.defer surface=\(id.uuidString.prefix(5)) reason=noWindow " + + "bounds=\(String(format: "%.1fx%.1f", view.bounds.width, view.bounds.height))" + ) +#endif + return + } +#if DEBUG + dlog("surface.attach.create surface=\(id.uuidString.prefix(5))") +#endif createSurface(for: view) - #if DEBUG - print("[TerminalSurface] attachToView: after createSurface, surface=\(surface != nil)") - #endif +#if DEBUG + dlog("surface.attach.create.done surface=\(id.uuidString.prefix(5)) hasSurface=\(surface != nil ? 1 : 0)") +#endif } else if let screen = view.window?.screen ?? NSScreen.main, let displayID = screen.displayID, displayID != 0, let s = surface { // Surface exists but we're (re)attaching after a view hierarchy move; ensure display id. ghostty_surface_set_display_id(s, displayID) +#if DEBUG + dlog("surface.attach.displayId surface=\(id.uuidString.prefix(5)) display=\(displayID)") +#endif } } @@ -1536,6 +1553,9 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { private var lastScrollEventTime: CFTimeInterval = 0 private var visibleInUI: Bool = true private var pendingSurfaceSize: CGSize? +#if DEBUG + private var lastSizeSkipSignature: String? +#endif // Visibility is used for focus gating, not for libghostty occlusion. fileprivate var isVisibleInUI: Bool { visibleInUI } @@ -1640,6 +1660,13 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { NotificationCenter.default.removeObserver(windowObserver) self.windowObserver = nil } +#if DEBUG + dlog( + "surface.view.windowMove surface=\(terminalSurface?.id.uuidString.prefix(5) ?? "nil") " + + "inWindow=\(window != nil ? 1 : 0) bounds=\(String(format: "%.1fx%.1f", bounds.width, bounds.height)) " + + "pending=\(String(format: "%.1fx%.1f", pendingSurfaceSize?.width ?? 0, pendingSurfaceSize?.height ?? 0))" + ) +#endif guard let window else { return } // If the surface creation was deferred while detached, create/attach it now. @@ -1700,14 +1727,62 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { private func updateSurfaceSize(size: CGSize? = nil) { guard let terminalSurface = terminalSurface else { return } let size = size ?? bounds.size - guard size.width > 0 && size.height > 0 else { return } + guard size.width > 0 && size.height > 0 else { +#if DEBUG + let signature = "nonPositive-\(Int(size.width))x\(Int(size.height))" + if lastSizeSkipSignature != signature { + dlog( + "surface.size.defer surface=\(terminalSurface.id.uuidString.prefix(5)) " + + "reason=nonPositive size=\(String(format: "%.1fx%.1f", size.width, size.height)) " + + "inWindow=\(window != nil ? 1 : 0)" + ) + lastSizeSkipSignature = signature + } +#endif + return + } pendingSurfaceSize = size - guard let window else { return } + guard let window else { +#if DEBUG + let signature = "noWindow-\(Int(size.width))x\(Int(size.height))" + if lastSizeSkipSignature != signature { + dlog( + "surface.size.defer surface=\(terminalSurface.id.uuidString.prefix(5)) reason=noWindow " + + "size=\(String(format: "%.1fx%.1f", size.width, size.height))" + ) + lastSizeSkipSignature = signature + } +#endif + return + } // First principles: derive pixel size from AppKit's backing conversion for the current // window/screen. Avoid updating Ghostty while detached from a window. let backingSize = convertToBacking(NSRect(origin: .zero, size: size)).size - guard backingSize.width > 0, backingSize.height > 0 else { return } + guard backingSize.width > 0, backingSize.height > 0 else { +#if DEBUG + let signature = "zeroBacking-\(Int(backingSize.width))x\(Int(backingSize.height))" + if lastSizeSkipSignature != signature { + dlog( + "surface.size.defer surface=\(terminalSurface.id.uuidString.prefix(5)) reason=zeroBacking " + + "size=\(String(format: "%.1fx%.1f", size.width, size.height)) " + + "backing=\(String(format: "%.1fx%.1f", backingSize.width, backingSize.height))" + ) + lastSizeSkipSignature = signature + } +#endif + return + } +#if DEBUG + if lastSizeSkipSignature != nil { + dlog( + "surface.size.resume surface=\(terminalSurface.id.uuidString.prefix(5)) " + + "size=\(String(format: "%.1fx%.1f", size.width, size.height)) " + + "backing=\(String(format: "%.1fx%.1f", backingSize.width, backingSize.height))" + ) + lastSizeSkipSignature = nil + } +#endif let xScale = backingSize.width / size.width let yScale = backingSize.height / size.height let layerScale = max(1.0, window.backingScaleFactor) @@ -3939,16 +4014,21 @@ struct GhosttyTerminalView: NSViewRepresentable { coordinator.desiredIsVisibleInUI = false coordinator.desiredPortalZPriority = 0 coordinator.lastBoundHostId = nil + let hostedView = coordinator.hostedView #if DEBUG - if let hostedView = coordinator.hostedView { + if let hostedView { if let snapshot = AppDelegate.shared?.tabManager?.debugCurrentWorkspaceSwitchSnapshot() { let dtMs = (CACurrentMediaTime() - snapshot.startedAt) * 1000 dlog( "ws.swiftui.dismantle id=\(snapshot.id) dt=\(String(format: "%.2fms", dtMs)) " + - "surface=\(hostedView.debugSurfaceId?.uuidString.prefix(5) ?? "nil")" + "surface=\(hostedView.debugSurfaceId?.uuidString.prefix(5) ?? "nil") " + + "inWindow=\(hostedView.window != nil ? 1 : 0)" ) } else { - dlog("ws.swiftui.dismantle id=none surface=\(hostedView.debugSurfaceId?.uuidString.prefix(5) ?? "nil")") + dlog( + "ws.swiftui.dismantle id=none surface=\(hostedView.debugSurfaceId?.uuidString.prefix(5) ?? "nil") " + + "inWindow=\(hostedView.window != nil ? 1 : 0)" + ) } } #endif @@ -3958,9 +4038,12 @@ struct GhosttyTerminalView: NSViewRepresentable { host.onGeometryChanged = nil } - coordinator.hostedView?.setVisibleInUI(false) - coordinator.hostedView?.setActive(false) - coordinator.hostedView?.setInactiveOverlay(color: .clear, opacity: 0, visible: false) + // SwiftUI can transiently dismantle/rebuild NSViewRepresentable instances during split + // tree updates. Do not force visible/active false here; that causes avoidable blackouts + // when the same hosted view is rebound moments later. + hostedView?.setFocusHandler(nil) + hostedView?.setTriggerFlashHandler(nil) + hostedView?.setDropZoneOverlay(zone: nil) coordinator.hostedView = nil nsView.subviews.forEach { $0.removeFromSuperview() } diff --git a/Sources/Panels/BrowserPanelView.swift b/Sources/Panels/BrowserPanelView.swift index 7c4105a3..76e5e9a1 100644 --- a/Sources/Panels/BrowserPanelView.swift +++ b/Sources/Panels/BrowserPanelView.swift @@ -185,6 +185,14 @@ struct BrowserPanelView: View { guard addressBarFocused else { return } refreshSuggestions() } + .onReceive(NotificationCenter.default.publisher(for: .browserDidBlurAddressBar).filter { note in + guard let panelId = note.object as? UUID else { return false } + return panelId == panel.id + }) { _ in + if addressBarFocused { + addressBarFocused = false + } + } } private var addressBar: some View { @@ -353,7 +361,8 @@ struct BrowserPanelView: View { WebViewRepresentable( panel: panel, shouldAttachWebView: isVisibleInUI, - shouldFocusWebView: isFocused && !addressBarFocused + shouldFocusWebView: isFocused && !addressBarFocused, + isPanelFocused: isFocused ) // Keep the representable identity stable across bonsplit structural updates. // This reduces WKWebView reparenting churn (and the associated WebKit crashes). @@ -2413,6 +2422,7 @@ struct WebViewRepresentable: NSViewRepresentable { let panel: BrowserPanel let shouldAttachWebView: Bool let shouldFocusWebView: Bool + let isPanelFocused: Bool final class Coordinator { weak var webView: WKWebView? @@ -2576,7 +2586,10 @@ struct WebViewRepresentable: NSViewRepresentable { } window.makeFirstResponder(webView) } else { - if Self.responderChainContains(window.firstResponder, target: webView) { + // Only force-resign WebView focus when this panel itself is not focused. + // If the panel is focused but the omnibar-focus state is briefly stale, aggressively + // clearing first responder here can undo programmatic webview focus (socket tests). + if !isPanelFocused && Self.responderChainContains(window.firstResponder, target: webView) { window.makeFirstResponder(nil) } } diff --git a/Sources/Panels/TerminalPanel.swift b/Sources/Panels/TerminalPanel.swift index 8989d622..1a133228 100644 --- a/Sources/Panels/TerminalPanel.swift +++ b/Sources/Panels/TerminalPanel.swift @@ -114,6 +114,9 @@ final class TerminalPanel: Panel, ObservableObject { func focus() { surface.setFocus(true) + // `unfocus()` force-disables active state to stop stale retries from stealing focus. + // Re-enable it immediately for explicit focus requests (socket/UI) so ensureFocus can run. + hostedView.setActive(true) hostedView.ensureFocus(for: workspaceId, surfaceId: id) } diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 80c0a7f9..96166d03 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -8403,8 +8403,14 @@ class TerminalController { return } + // Programmatic WebView focus should win over stale omnibar focus state, especially + // after workspace switches where the blank-page omnibar auto-focus can re-trigger. + browserPanel.endSuppressWebViewFocusForAddressBar() + browserPanel.clearWebViewFocusSuppression() + NotificationCenter.default.post(name: .browserDidBlurAddressBar, object: panelId) + // Prevent omnibar auto-focus from immediately stealing first responder back. - browserPanel.suppressOmnibarAutofocus(for: 1.0) + browserPanel.suppressOmnibarAutofocus(for: 1.5) let webView = browserPanel.webView guard let window = webView.window else { @@ -8418,6 +8424,15 @@ class TerminalController { window.makeFirstResponder(webView) if Self.responderChainContains(window.firstResponder, target: webView) { + // Some focus churn paths (workspace handoff / omnibar blur) can race this call. + // Reassert on the next runloop if another responder steals focus immediately. + DispatchQueue.main.async { [weak window, weak webView] in + guard let window, let webView else { return } + guard webView.window === window else { return } + if !Self.responderChainContains(window.firstResponder, target: webView) { + window.makeFirstResponder(webView) + } + } result = "OK" } else { result = "ERROR: Focus did not move into web view" diff --git a/Sources/TerminalWindowPortal.swift b/Sources/TerminalWindowPortal.swift index 5904a7aa..e5ea21bc 100644 --- a/Sources/TerminalWindowPortal.swift +++ b/Sources/TerminalWindowPortal.swift @@ -1,9 +1,24 @@ import AppKit import ObjectiveC +#if DEBUG +import Bonsplit +#endif private var cmuxWindowTerminalPortalKey: UInt8 = 0 private var cmuxWindowTerminalPortalCloseObserverKey: UInt8 = 0 +#if DEBUG +private func portalDebugToken(_ view: NSView?) -> String { + guard let view else { return "nil" } + let ptr = Unmanaged.passUnretained(view).toOpaque() + return String(describing: ptr) +} + +private func portalDebugFrame(_ rect: NSRect) -> String { + String(format: "%.1f,%.1f %.1fx%.1f", rect.origin.x, rect.origin.y, rect.size.width, rect.size.height) +} +#endif + final class WindowTerminalHostView: NSView { override var isOpaque: Bool { false } @@ -120,6 +135,13 @@ final class WindowTerminalPortal: NSObject { if let anchor = entry.anchorView { hostedByAnchorId.removeValue(forKey: ObjectIdentifier(anchor)) } +#if DEBUG + let hadSuperview = (entry.hostedView?.superview === hostView) ? 1 : 0 + dlog( + "portal.detach hosted=\(portalDebugToken(entry.hostedView)) " + + "anchor=\(portalDebugToken(entry.anchorView)) hadSuperview=\(hadSuperview)" + ) +#endif if let hostedView = entry.hostedView, hostedView.superview === hostView { hostedView.removeFromSuperview() } @@ -133,6 +155,15 @@ final class WindowTerminalPortal: NSObject { let previousEntry = entriesByHostedId[hostedId] if let previousHostedId = hostedByAnchorId[anchorId], previousHostedId != hostedId { +#if DEBUG + let previousToken = entriesByHostedId[previousHostedId] + .map { portalDebugToken($0.hostedView) } + ?? String(describing: previousHostedId) + dlog( + "portal.bind.replace anchor=\(portalDebugToken(anchorView)) " + + "oldHosted=\(previousToken) newHosted=\(portalDebugToken(hostedView))" + ) +#endif detachHostedView(withId: previousHostedId) } @@ -156,15 +187,37 @@ final class WindowTerminalPortal: NSObject { }() let becameVisible = (previousEntry?.visibleInUI ?? false) == false && visibleInUI let priorityIncreased = zPriority > (previousEntry?.zPriority ?? Int.min) +#if DEBUG + if previousEntry == nil || didChangeAnchor || becameVisible || priorityIncreased || hostedView.superview !== hostView { + dlog( + "portal.bind hosted=\(portalDebugToken(hostedView)) " + + "anchor=\(portalDebugToken(anchorView)) prevAnchor=\(portalDebugToken(previousEntry?.anchorView)) " + + "visible=\(visibleInUI ? 1 : 0) prevVisible=\((previousEntry?.visibleInUI ?? false) ? 1 : 0) " + + "z=\(zPriority) prevZ=\(previousEntry?.zPriority ?? Int.min)" + ) + } +#endif if hostedView.superview !== hostView { - hostedView.removeFromSuperview() - hostView.addSubview(hostedView) - } else if (didChangeAnchor || becameVisible || priorityIncreased), hostView.subviews.last !== hostedView { - // Refresh z-order only on meaningful transitions. Reordering on every bind call - // creates expensive reparent loops during SwiftUI update/layout churn. - hostedView.removeFromSuperview() - hostView.addSubview(hostedView) +#if DEBUG + dlog( + "portal.reparent hosted=\(portalDebugToken(hostedView)) " + + "reason=attach super=\(portalDebugToken(hostedView.superview))" + ) +#endif + hostView.addSubview(hostedView, positioned: .above, relativeTo: nil) + } else if (becameVisible || priorityIncreased), hostView.subviews.last !== hostedView { + // Refresh z-order only when a view becomes visible or gets a higher priority. + // Anchor-only churn is common during split tree updates; forcing remove/add there + // causes transient inWindow=0 -> 1 bounces that can flash black. +#if DEBUG + dlog( + "portal.reparent hosted=\(portalDebugToken(hostedView)) reason=raise " + + "didChangeAnchor=\(didChangeAnchor ? 1 : 0) becameVisible=\(becameVisible ? 1 : 0) " + + "priorityIncreased=\(priorityIncreased ? 1 : 0)" + ) +#endif + hostView.addSubview(hostedView, positioned: .above, relativeTo: nil) } synchronizeHostedView(withId: hostedId) @@ -185,23 +238,52 @@ final class WindowTerminalPortal: NSObject { return } guard let anchorView = entry.anchorView, let window else { +#if DEBUG + if !hostedView.isHidden { + dlog("portal.hidden hosted=\(portalDebugToken(hostedView)) value=1 reason=missingAnchorOrWindow") + } +#endif hostedView.isHidden = true return } guard anchorView.window === window else { +#if DEBUG + if !hostedView.isHidden { + dlog( + "portal.hidden hosted=\(portalDebugToken(hostedView)) value=1 " + + "reason=anchorWindowMismatch anchorWindow=\(portalDebugToken(anchorView.window?.contentView))" + ) + } +#endif hostedView.isHidden = true return } let frameInWindow = anchorView.convert(anchorView.bounds, to: nil) let frameInHost = hostView.convert(frameInWindow, from: nil) + let anchorHidden = Self.isHiddenOrAncestorHidden(anchorView) + let tinyFrame = frameInHost.width <= 1 || frameInHost.height <= 1 let shouldHide = !entry.visibleInUI || - Self.isHiddenOrAncestorHidden(anchorView) || - frameInHost.width <= 1 || - frameInHost.height <= 1 + anchorHidden || + tinyFrame let oldFrame = hostedView.frame +#if DEBUG + let collapsedToTiny = oldFrame.width > 1 && oldFrame.height > 1 && tinyFrame + let restoredFromTiny = (oldFrame.width <= 1 || oldFrame.height <= 1) && !tinyFrame + if collapsedToTiny { + dlog( + "portal.frame.collapse hosted=\(portalDebugToken(hostedView)) anchor=\(portalDebugToken(anchorView)) " + + "old=\(portalDebugFrame(oldFrame)) new=\(portalDebugFrame(frameInHost))" + ) + } else if restoredFromTiny { + dlog( + "portal.frame.restore hosted=\(portalDebugToken(hostedView)) anchor=\(portalDebugToken(anchorView)) " + + "old=\(portalDebugFrame(oldFrame)) new=\(portalDebugFrame(frameInHost))" + ) + } +#endif if !Self.rectApproximatelyEqual(oldFrame, frameInHost) { CATransaction.begin() CATransaction.setDisableActions(true) @@ -215,6 +297,13 @@ final class WindowTerminalPortal: NSObject { } if hostedView.isHidden != shouldHide { +#if DEBUG + dlog( + "portal.hidden hosted=\(portalDebugToken(hostedView)) value=\(shouldHide ? 1 : 0) " + + "visibleInUI=\(entry.visibleInUI ? 1 : 0) anchorHidden=\(anchorHidden ? 1 : 0) " + + "tiny=\(tinyFrame ? 1 : 0) frame=\(portalDebugFrame(frameInHost))" + ) +#endif hostedView.isHidden = shouldHide } } diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index d24da118..9709f541 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -1331,7 +1331,14 @@ extension Workspace: BonsplitDelegate { } func splitTabBar(_ controller: BonsplitController, didMoveTab tab: Bonsplit.Tab, fromPane source: PaneID, toPane destination: PaneID) { - _ = source +#if DEBUG + let movedPanel = panelIdFromSurfaceId(tab.id)?.uuidString.prefix(5) ?? "unknown" + dlog( + "split.moveTab panel=\(movedPanel) " + + "from=\(source.id.uuidString.prefix(5)) to=\(destination.id.uuidString.prefix(5)) " + + "sourceTabs=\(controller.tabs(inPane: source).count) destTabs=\(controller.tabs(inPane: destination).count)" + ) +#endif applyTabSelection(tabId: tab.id, inPane: destination) scheduleTerminalGeometryReconcile() scheduleFocusReconcile() @@ -1375,6 +1382,13 @@ extension Workspace: BonsplitDelegate { } func splitTabBar(_ controller: BonsplitController, didSplitPane originalPane: PaneID, newPane: PaneID, orientation: SplitOrientation) { +#if DEBUG + dlog( + "split.didSplit original=\(originalPane.id.uuidString.prefix(5)) new=\(newPane.id.uuidString.prefix(5)) " + + "orientation=\(orientation) programmatic=\(isProgrammaticSplit ? 1 : 0) " + + "originalTabs=\(controller.tabs(inPane: originalPane).count) newTabs=\(controller.tabs(inPane: newPane).count)" + ) +#endif // Only auto-create a terminal if the split came from bonsplit UI. // Programmatic splits via newTerminalSplit() set isProgrammaticSplit and handle their own panels. guard !isProgrammaticSplit else { @@ -1393,11 +1407,68 @@ extension Workspace: BonsplitDelegate { if !controller.tabs(inPane: newPane).isEmpty { let originalTabs = controller.tabs(inPane: originalPane) let hasRealSurface = originalTabs.contains { panelIdFromSurfaceId($0.id) != nil } +#if DEBUG + dlog( + "split.didSplit.drag original=\(originalPane.id.uuidString.prefix(5)) " + + "new=\(newPane.id.uuidString.prefix(5)) originalTabs=\(originalTabs.count) " + + "newTabs=\(controller.tabs(inPane: newPane).count) hasRealSurface=\(hasRealSurface ? 1 : 0)" + ) +#endif if !hasRealSurface { - _ = newTerminalSurface(inPane: originalPane, focus: false) - for tab in controller.tabs(inPane: originalPane) { - if panelIdFromSurfaceId(tab.id) == nil { - bonsplitController.closeTab(tab.id) + let placeholderTabs = originalTabs.filter { panelIdFromSurfaceId($0.id) == nil } +#if DEBUG + dlog( + "split.placeholderRepair pane=\(originalPane.id.uuidString.prefix(5)) " + + "action=reusePlaceholder placeholderCount=\(placeholderTabs.count)" + ) +#endif + if let replacementTab = placeholderTabs.first { + // Keep the existing placeholder tab identity and replace only the panel mapping. + // This avoids an extra create+close tab churn that can transiently render an + // empty pane during drag-to-split of a single-tab pane. + let inheritedConfig: ghostty_surface_config_s? = { + for panel in panels.values { + if let terminalPanel = panel as? TerminalPanel, + let surface = terminalPanel.surface.surface { + return ghostty_surface_inherited_config(surface, GHOSTTY_SURFACE_CONTEXT_SPLIT) + } + } + return nil + }() + + let replacementPanel = TerminalPanel( + workspaceId: id, + context: GHOSTTY_SURFACE_CONTEXT_SPLIT, + configTemplate: inheritedConfig + ) + panels[replacementPanel.id] = replacementPanel + surfaceIdToPanelId[replacementTab.id] = replacementPanel.id + + bonsplitController.updateTab( + replacementTab.id, + title: replacementPanel.displayTitle, + icon: .some(replacementPanel.displayIcon), + iconImageData: .some(nil), + isDirty: replacementPanel.isDirty, + showsNotificationBadge: false, + isLoading: false + ) + + for extraPlaceholder in placeholderTabs.dropFirst() { + bonsplitController.closeTab(extraPlaceholder.id) + } + } else { +#if DEBUG + dlog( + "split.placeholderRepair pane=\(originalPane.id.uuidString.prefix(5)) " + + "fallback=createTerminalAndDropPlaceholders" + ) +#endif + _ = newTerminalSurface(inPane: originalPane, focus: false) + for tab in controller.tabs(inPane: originalPane) { + if panelIdFromSurfaceId(tab.id) == nil { + bonsplitController.closeTab(tab.id) + } } } } @@ -1410,6 +1481,13 @@ extension Workspace: BonsplitDelegate { let sourcePanelId = panelIdFromSurfaceId(sourceTabId), let sourcePanel = terminalPanel(for: sourcePanelId) else { return } +#if DEBUG + dlog( + "split.didSplit.autoCreate pane=\(newPane.id.uuidString.prefix(5)) " + + "fromPane=\(originalPane.id.uuidString.prefix(5)) sourcePanel=\(sourcePanelId.uuidString.prefix(5))" + ) +#endif + let inheritedConfig: ghostty_surface_config_s? = if let existing = sourcePanel.surface.surface { ghostty_surface_inherited_config(existing, GHOSTTY_SURFACE_CONTEXT_SPLIT) } else { @@ -1434,6 +1512,12 @@ extension Workspace: BonsplitDelegate { } surfaceIdToPanelId[newTabId] = newPanel.id +#if DEBUG + dlog( + "split.didSplit.autoCreate.done pane=\(newPane.id.uuidString.prefix(5)) " + + "panel=\(newPanel.id.uuidString.prefix(5))" + ) +#endif // `createTab` selects the new tab but does not emit didSelectTab; schedule an explicit // selection so our focus/unfocus logic runs after this delegate callback returns. diff --git a/tests/test_split_flash_and_layout.py b/tests/test_split_flash_and_layout.py index 0dacb027..254d31a4 100644 --- a/tests/test_split_flash_and_layout.py +++ b/tests/test_split_flash_and_layout.py @@ -85,8 +85,47 @@ def _assert_selected_panels_healthy(payload: dict, *, min_wh: float = 80.0) -> N ) +def _assert_no_transient_detach_or_hide( + c: cmux, + *, + duration_s: float = 1.0, + cadence_s: float = 0.005, + max_false_samples: int = 2, +) -> None: + false_in_window: dict[str, int] = {} + hidden_true: dict[str, int] = {} + deadline = time.time() + duration_s + + while time.time() < deadline: + rows = c.surface_health() + for row in rows: + if row.get("type") != "terminal": + continue + panel_id = (row.get("id") or "").lower() + if not panel_id: + continue + if row.get("in_window") is False: + false_in_window[panel_id] = false_in_window.get(panel_id, 0) + 1 + if row.get("hidden") is True: + hidden_true[panel_id] = hidden_true.get(panel_id, 0) + 1 + time.sleep(cadence_s) + + detached = {k: v for k, v in false_in_window.items() if v > max_false_samples} + hidden = {k: v for k, v in hidden_true.items() if v > max_false_samples} + if detached or hidden: + raise cmuxError( + f"Transient detach/hide during split exceeds tolerance " + f"(detached={detached}, hidden={hidden})" + ) + + def main() -> int: with cmux(SOCKET_PATH) as c: + # Run on a fresh workspace to avoid state carry-over from restored sessions. + test_workspace = c.new_workspace() + c.select_workspace(test_workspace) + time.sleep(0.2) + # Baseline: a fresh counter, no flashes just from connecting. c.reset_empty_panel_count() @@ -108,6 +147,38 @@ def main() -> int: raise cmuxError(f"Expected >= 2 panes after split, got {len(panes)}") _assert_selected_panels_healthy(after) + # Drag-to-split from a single-surface pane should also avoid EmptyPanelView flashes. + drag_workspace = c.new_workspace() + c.select_workspace(drag_workspace) + time.sleep(0.2) + drag_before = c.layout_debug() + _assert_selected_panels_healthy(drag_before) + drag_selected = drag_before.get("selectedPanels") or [] + if not drag_selected: + raise cmuxError("layout_debug returned no selectedPanels for drag split setup") + drag_panel_id = drag_selected[0].get("panelId") + if not drag_panel_id: + raise cmuxError("drag split setup selected panel has no panelId") + drag_panes_before = len(drag_before.get("layout", {}).get("panes") or []) + + c.reset_empty_panel_count() + response = c._send_command(f"drag_surface_to_split {drag_panel_id} right") + if not response.startswith("OK "): + raise cmuxError(response) + _assert_no_transient_detach_or_hide(c) + time.sleep(0.4) + flashes = c.empty_panel_count() + if flashes != 0: + raise cmuxError(f"EmptyPanelView appeared during drag split (count={flashes})") + + drag_after = c.layout_debug() + drag_panes_after = len(drag_after.get("layout", {}).get("panes") or []) + if drag_panes_after < drag_panes_before + 1: + raise cmuxError( + f"Expected drag split to add a pane: before={drag_panes_before} after={drag_panes_after}" + ) + _assert_selected_panels_healthy(drag_after) + # Browser split should also avoid EmptyPanelView flashes. c.reset_empty_panel_count() browser_id = c._send_command("open_browser https://example.com") @@ -121,10 +192,12 @@ def main() -> int: after_browser = c.layout_debug() _assert_selected_panels_healthy(after_browser) + c.close_workspace(test_workspace) + time.sleep(0.1) + print("PASS: split flash + layout bounds checks") return 0 if __name__ == "__main__": raise SystemExit(main()) - diff --git a/tests_v2/test_split_flash_and_layout.py b/tests_v2/test_split_flash_and_layout.py index e7262277..df082633 100644 --- a/tests_v2/test_split_flash_and_layout.py +++ b/tests_v2/test_split_flash_and_layout.py @@ -85,8 +85,47 @@ def _assert_selected_panels_healthy(payload: dict, *, min_wh: float = 80.0) -> N ) +def _assert_no_transient_detach_or_hide( + c: cmux, + *, + duration_s: float = 1.0, + cadence_s: float = 0.005, + max_false_samples: int = 2, +) -> None: + false_in_window: dict[str, int] = {} + hidden_true: dict[str, int] = {} + deadline = time.time() + duration_s + + while time.time() < deadline: + rows = c.surface_health() + for row in rows: + if row.get("type") != "terminal": + continue + panel_id = (row.get("id") or "").lower() + if not panel_id: + continue + if row.get("in_window") is False: + false_in_window[panel_id] = false_in_window.get(panel_id, 0) + 1 + if row.get("hidden") is True: + hidden_true[panel_id] = hidden_true.get(panel_id, 0) + 1 + time.sleep(cadence_s) + + detached = {k: v for k, v in false_in_window.items() if v > max_false_samples} + hidden = {k: v for k, v in hidden_true.items() if v > max_false_samples} + if detached or hidden: + raise cmuxError( + f"Transient detach/hide during split exceeds tolerance " + f"(detached={detached}, hidden={hidden})" + ) + + def main() -> int: with cmux(SOCKET_PATH) as c: + # Run on a fresh workspace to avoid state carry-over from restored sessions. + test_workspace = c.new_workspace() + c.select_workspace(test_workspace) + time.sleep(0.2) + # Baseline: a fresh counter, no flashes just from connecting. c.reset_empty_panel_count() @@ -108,6 +147,36 @@ def main() -> int: raise cmuxError(f"Expected >= 2 panes after split, got {len(panes)}") _assert_selected_panels_healthy(after) + # Drag-to-split from a single-surface pane should also avoid EmptyPanelView flashes. + drag_workspace = c.new_workspace() + c.select_workspace(drag_workspace) + time.sleep(0.2) + drag_before = c.layout_debug() + _assert_selected_panels_healthy(drag_before) + drag_selected = drag_before.get("selectedPanels") or [] + if not drag_selected: + raise cmuxError("layout_debug returned no selectedPanels for drag split setup") + drag_panel_id = drag_selected[0].get("panelId") + if not drag_panel_id: + raise cmuxError("drag split setup selected panel has no panelId") + drag_panes_before = len(drag_before.get("layout", {}).get("panes") or []) + + c.reset_empty_panel_count() + c.drag_surface_to_split(drag_panel_id, "right") + _assert_no_transient_detach_or_hide(c) + time.sleep(0.4) + flashes = c.empty_panel_count() + if flashes != 0: + raise cmuxError(f"EmptyPanelView appeared during drag split (count={flashes})") + + drag_after = c.layout_debug() + drag_panes_after = len(drag_after.get("layout", {}).get("panes") or []) + if drag_panes_after < drag_panes_before + 1: + raise cmuxError( + f"Expected drag split to add a pane: before={drag_panes_before} after={drag_panes_after}" + ) + _assert_selected_panels_healthy(drag_after) + # Browser split should also avoid EmptyPanelView flashes. c.reset_empty_panel_count() _browser_id = c.open_browser("https://example.com") @@ -119,6 +188,9 @@ def main() -> int: after_browser = c.layout_debug() _assert_selected_panels_healthy(after_browser) + c.close_workspace(test_workspace) + time.sleep(0.1) + print("PASS: split flash + layout bounds checks") return 0