Skip to content

Add debug logs for Cmd+F find bar refocus #840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lawrencecchen merged 10 commits into from
Mar 4, 2026
Merged

Add debug logs for Cmd+F find bar refocus #840

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
603df79
Add debug logs for Cmd+F find bar focus/refocus state machine
lawrencecchen Mar 4, 2026
e5cb51c
Fix Cmd+F find bar focus loss after window switch
lawrencecchen Mar 4, 2026
67bd247
Fix multi-surface focus stealing and NSHostingView responder issue
lawrencecchen Mar 4, 2026
3be380d
Use AppKit NSTextField focus instead of SwiftUI @FocusState for searc…
lawrencecchen Mar 4, 2026
3d41035
Two-phase focus restore: AppKit + SwiftUI sync, click-to-terminal fix
lawrencecchen Mar 4, 2026
996133f
Replace SwiftUI TextField with NSViewRepresentable for find bar
lawrencecchen Mar 4, 2026
b2b6247
Explicitly unfocus terminal surface when find bar takes focus
lawrencecchen Mar 4, 2026
98b2d0a
Address review findings: field-editor guard, NSLog→dlog, stale focus
lawrencecchen Mar 4, 2026
7bc4809
Guard against re-focusing already-focused search field
lawrencecchen Mar 4, 2026
f5754e8
Address review findings: stale focus target, IME guard, tab/pane gating
lawrencecchen Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Sources/Find/SurfaceSearchOverlay.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,20 @@ struct SurfaceSearchOverlay: View {
}
}
.onExitCommand {
#if DEBUG
dlog("find.overlay.escape surface=\(surfaceId.uuidString.prefix(5)) needleEmpty=\(searchState.needle.isEmpty)")
#endif
if searchState.needle.isEmpty {
onClose()
} else {
onMoveFocusToTerminal()
}
}
.onChange(of: isSearchFieldFocused) { focused in
#if DEBUG
dlog("find.overlay.focusChanged surface=\(surfaceId.uuidString.prefix(5)) focused=\(focused)")
#endif

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update search focus target when field focus changes

The new focus-change hook only logs and never updates searchFocusTarget, so clicking from the find field back into the terminal leaves the target stale as .searchField. On the next didBecomeKey, restoreSearchFocus will re-focus the search UI even though the user's last in-window focus was the terminal, which regresses expected Cmd+F workflows that keep the find bar open while returning to terminal input.

Useful? React with 👍 / 👎.

}
.backport.onKeyPress(.return) { modifiers in
let action = modifiers.contains(.shift)
? "navigate_search:previous"
Expand Down
139 changes: 133 additions & 6 deletions Sources/GhosttyTerminalView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4669,6 +4669,14 @@ final class GhosttySurfaceScrollView: NSView {
private var dropZoneOverlayAnimationGeneration: UInt64 = 0
// Intentionally no focus retry loops: rely on AppKit first-responder and bonsplit selection.

/// Tracks whether keyboard focus should go to the search field or the terminal
/// when the window becomes key while the find bar is open.
enum SearchFocusTarget {
case searchField
case terminal
}
private(set) var searchFocusTarget: SearchFocusTarget = .searchField

private static func panelBackgroundFillColor(for terminalBackgroundColor: NSColor) -> NSColor {
// The Ghostty renderer already draws translucent terminal backgrounds. If we paint an
// additional translucent layer here, alpha stacks and appears effectively opaque.
Expand Down Expand Up @@ -4954,6 +4962,17 @@ final class GhosttySurfaceScrollView: NSView {
self?.handleScrollbarUpdate(notification)
})

observers.append(NotificationCenter.default.addObserver(
forName: .ghosttySearchFocus,
object: nil,
queue: .main
) { [weak self] notification in
guard let self,
let surface = notification.object as? TerminalSurface,
surface === self.surfaceView.terminalSurface else { return }
self.searchFocusTarget = .searchField
})

observers.append(NotificationCenter.default.addObserver(
forName: .ghosttyDidUpdateCellSize,
object: surfaceView,
Expand Down Expand Up @@ -5055,19 +5074,32 @@ final class GhosttySurfaceScrollView: NSView {
object: window,
queue: .main
) { [weak self] _ in
self?.applyFirstResponderIfNeeded()
guard let self else { return }
let searchActive = self.surfaceView.terminalSurface?.searchState != nil
#if DEBUG
dlog("find.window.didBecomeKey surface=\(self.surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil") searchActive=\(searchActive) focusTarget=\(self.searchFocusTarget) firstResponder=\(String(describing: self.window?.firstResponder))")
#endif
self.applyFirstResponderIfNeeded()
})
windowObservers.append(NotificationCenter.default.addObserver(
forName: NSWindow.didResignKeyNotification,
object: window,
queue: .main
) { [weak self] _ in
guard let self, let window = self.window else { return }
let searchActive = self.surfaceView.terminalSurface?.searchState != nil
// Losing key window does not always trigger first-responder resignation, so force
// the focused terminal view to yield responder to keep Ghostty cursor/focus state in sync.
if let fr = window.firstResponder as? NSView,
fr === self.surfaceView || fr.isDescendant(of: self.surfaceView) {
#if DEBUG
dlog("find.window.didResignKey surface=\(self.surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil") searchActive=\(searchActive) resigningFirstResponder")
#endif
window.makeFirstResponder(nil)
} else {
#if DEBUG
dlog("find.window.didResignKey surface=\(self.surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil") searchActive=\(searchActive) firstResponder=\(String(describing: window.firstResponder)) (not terminal, skipping)")
#endif
}
})
if window.isKeyWindow { applyFirstResponderIfNeeded() }
Expand All @@ -5078,7 +5110,14 @@ final class GhosttySurfaceScrollView: NSView {
}

func setFocusHandler(_ handler: (() -> Void)?) {
surfaceView.onFocus = handler
surfaceView.onFocus = { [weak self] in
// When the terminal surface gains focus (click, tab, etc.), update the
Comment on lines 5121 to +5127

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear onFocus when handler is nil

setFocusHandler now always installs a closure, even when callers pass nil, so teardown paths like dismantleNSView no longer truly unregister focus callbacks. In this state, incidental terminal focus events can still mutate searchFocusTarget to .terminal while the find bar is open, which changes later didBecomeKey restoration behavior away from the search field despite no user focus action.

Useful? React with 👍 / 👎.

// search focus target so window reactivation restores terminal focus.
if self?.surfaceView.terminalSurface?.searchState != nil {
self?.searchFocusTarget = .terminal
}
handler?()
}
}

func setTriggerFlashHandler(_ handler: (() -> Void)?) {
Expand Down Expand Up @@ -5131,18 +5170,29 @@ final class GhosttySurfaceScrollView: NSView {
// SwiftUI panel-level overlays can fall behind portal-hosted terminal surfaces.
guard let terminalSurface = surfaceView.terminalSurface,
let searchState else {
let hadOverlay = searchOverlayHostingView != nil
#if DEBUG
dlog("find.setSearchOverlay REMOVE surface=\(surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil") hadOverlay=\(hadOverlay)")
#endif
searchOverlayHostingView?.removeFromSuperview()
searchOverlayHostingView = nil
searchFocusTarget = .searchField
return
}

let hadOverlay = searchOverlayHostingView != nil
#if DEBUG
dlog("find.setSearchOverlay MOUNT surface=\(terminalSurface.id.uuidString.prefix(5)) existingOverlay=\(hadOverlay ? "yes(update)" : "no(create)")")
#endif

let tabId = terminalSurface.tabId
let surfaceId = terminalSurface.id
let rootView = SurfaceSearchOverlay(
tabId: tabId,
surfaceId: surfaceId,
searchState: searchState,
onMoveFocusToTerminal: { [weak self] in
self?.searchFocusTarget = .terminal
self?.moveFocus()
Comment on lines 5207 to 5209

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update search focus target when field is manually refocused

Setting searchFocusTarget to .terminal on Escape is not paired with any update when the user later clicks back into the search field, so the target can become stale. In that state, switching to another app and back will run restoreSearchFocus and force focus to the terminal even though the search field was the last focused control, which is a user-visible regression for find-bar workflows after using Escape once.

Useful? React with 👍 / 👎.

},
onNavigateSearch: { [weak terminalSurface] action in
Expand Down Expand Up @@ -5172,6 +5222,7 @@ final class GhosttySurfaceScrollView: NSView {
return
}

searchFocusTarget = .searchField
let overlay = NSHostingView(rootView: rootView)
overlay.translatesAutoresizingMaskIntoConstraints = false
addSubview(overlay)
Expand Down Expand Up @@ -5476,7 +5527,9 @@ final class GhosttySurfaceScrollView: NSView {

func moveFocus(from previous: GhosttySurfaceScrollView? = nil, delay: TimeInterval? = nil) {
#if DEBUG
dlog("focus.moveFocus to=\(self.surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil")")
let surfaceShort = self.surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil"
let searchActive = self.surfaceView.terminalSurface?.searchState != nil
dlog("find.moveFocus to=\(surfaceShort) searchState=\(searchActive ? "active" : "nil")")
#endif
let work = { [weak self] in
guard let self else { return }
Expand Down Expand Up @@ -5626,7 +5679,11 @@ final class GhosttySurfaceScrollView: NSView {
let isHiddenForFocus = isHiddenOrHasHiddenAncestor || surfaceView.isHiddenOrHasHiddenAncestor

guard isActive else { return }
guard surfaceView.terminalSurface?.searchState == nil else { return }
if surfaceView.terminalSurface?.searchState != nil {
guard let window else { return }
restoreSearchFocus(window: window)
return

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve tab/pane gating before restoring search focus

In ensureFocus, the new early return for active search calls restoreSearchFocus before the existing selectedTabId/focused-pane guards run. That means a delayed retry can now re-focus a stale surface with an open find bar even after selection moved elsewhere, because the function no longer reaches the checks that previously prevented cross-tab/pane focus steals. This is user-visible during rapid workspace/pane switches with find open.

Useful? React with 👍 / 👎.

}
guard let window else { return }
Comment on lines 5698 to 5699

@coderabbitai coderabbitai Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Keep the search-active ensureFocus path inside the existing guard/retry flow.

At Line [5691], active search returns before visibility/key-window checks and retry logic. In transient reparent/reactivation states, this can drop focus restoration instead of retrying.

♻️ Proposed adjustment 
-        guard isActive else { return }
-        if surfaceView.terminalSurface?.searchState != nil {
-            guard let window else { return }
-            restoreSearchFocus(window: window)
-            return
-        }
-        guard let window else { return }
+        guard isActive else { return }
+        guard let window else { return }
         guard surfaceView.isVisibleInUI else {
             retry()
             return
         }
         guard !isHiddenForFocus, hasUsablePortalGeometry else {
@@
             retry()
             return
         }
+
+        if surfaceView.terminalSurface?.searchState != nil {
+            if !window.isKeyWindow {
+                window.makeKeyAndOrderFront(nil)
+            }
+            restoreSearchFocus(window: window)
+            if searchFocusTarget == .terminal, !isSurfaceViewFirstResponder() {
+                retry()
+            }
+            return
+        }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 5690 - 5696, The current
early return for an active search bypasses the existing guard/retry flow and can
skip retrying visibility/key-window checks; move the search-active path so it
executes after the existing "guard let window else { return }" and within the
retry logic block (i.e., keep the check of
surfaceView.terminalSurface?.searchState and the call to
restoreSearchFocus(window:) inside the same guarded/retry flow used for
ensureFocus) so that restoreSearchFocus(window:) is only attempted when window
is available and will participate in the retry behavior when
reparenting/reactivation occurs; adjust the control flow around isActive and the
guard-let window to nest the search-active branch rather than returning early.

guard surfaceView.isVisibleInUI else {
retry()
Expand Down Expand Up @@ -5705,27 +5762,97 @@ final class GhosttySurfaceScrollView: NSView {
return size.width > 1 && size.height > 1
}()
let isHiddenForFocus = isHiddenOrHasHiddenAncestor || surfaceView.isHiddenOrHasHiddenAncestor
let surfaceShort = surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil"

guard isActive else { return }
guard surfaceView.isVisibleInUI else { return }
guard !isHiddenForFocus, hasUsablePortalGeometry else {
#if DEBUG
dlog(
"focus.apply.skip surface=\(surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil") " +
"focus.apply.skip surface=\(surfaceShort) " +
"reason=hidden_or_tiny hidden=\(isHiddenForFocus ? 1 : 0) frame=\(String(format: "%.1fx%.1f", bounds.width, bounds.height))"
)
#endif
return
}
guard surfaceView.terminalSurface?.searchState == nil else { return }
guard let window, window.isKeyWindow else { return }
if surfaceView.terminalSurface?.searchState != nil {
// Find bar is open. Restore focus based on what the user last intended.
restoreSearchFocus(window: window)
return
}
if let fr = window.firstResponder as? NSView,
fr === surfaceView || fr.isDescendant(of: surfaceView) {
return
}
// Don't steal focus from a search overlay on another surface in this window.
if let fr = window.firstResponder as? NSView, isSearchOverlayOrDescendant(fr) {
#if DEBUG
dlog("find.applyFirstResponder SKIP surface=\(surfaceShort) reason=searchOverlayFocused")
#endif
return
}
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
#if DEBUG
dlog("find.applyFirstResponder APPLY surface=\(surfaceShort) prevFirstResponder=\(String(describing: window.firstResponder))")
#endif
window.makeFirstResponder(surfaceView)
}

/// Restore focus when window becomes key and the find bar is open.
/// Respects `searchFocusTarget` so Escape-to-terminal intent is preserved across window switches.
private func restoreSearchFocus(window: NSWindow) {
let surfaceShort = surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil"
switch searchFocusTarget {
case .searchField:
// Two-phase focus restore:
// 1. AppKit: make the backing NSTextField first responder so typing works immediately.
// (notification alone fails when first responder is NSWindow)
// 2. SwiftUI: post .ghosttySearchFocus so @FocusState syncs and .onExitCommand/onKeyPress work.
// (AppKit alone breaks SwiftUI event handlers like Escape and Return)
var usedAppKit = false
if let overlay = searchOverlayHostingView,
let textField = findTextField(in: overlay) {
window.makeFirstResponder(textField)
usedAppKit = true
}
if let terminalSurface = surfaceView.terminalSurface {
NotificationCenter.default.post(name: .ghosttySearchFocus, object: terminalSurface)
}
#if DEBUG
dlog("find.restoreSearchFocus surface=\(surfaceShort) target=searchField via=\(usedAppKit ? "appkit+notification" : "notification_only")")
#endif
case .terminal:
window.makeFirstResponder(surfaceView)
#if DEBUG
dlog("find.restoreSearchFocus surface=\(surfaceShort) target=terminal")
#endif
}
}

/// Find the first NSTextField descendant in a view hierarchy.
/// Used to locate the SwiftUI TextField's backing NSTextField inside the search overlay.
private func findTextField(in view: NSView) -> NSTextField? {
for subview in view.subviews {
if let textField = subview as? NSTextField, textField.isEditable {
return textField
}
if let found = findTextField(in: subview) {
return found
}
}
return nil
}

/// Check if a view is a search overlay hosting view or a descendant of one.
private func isSearchOverlayOrDescendant(_ view: NSView) -> Bool {
var current: NSView? = view
while let v = current {
if v is NSHostingView<SurfaceSearchOverlay> { return true }
current = v.superview
Comment on lines +5856 to +5859

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Treat field-editor responders as search overlay focus

isSearchOverlayOrDescendant only walks superview and matches NSHostingView<SurfaceSearchOverlay>, but when the find field is actively editing AppKit sets window.firstResponder to the shared NSTextView field editor instead of a descendant of the hosting view. In that common state this helper returns false, so another surface in the same window can still pass applyFirstResponderIfNeeded and call makeFirstResponder(surfaceView), stealing focus from an open find bar after didBecomeKey/surface focus churn.

Useful? React with 👍 / 👎.

}
return false
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#if DEBUG
struct DebugRenderStats {
let drawCount: Int
Expand Down
14 changes: 11 additions & 3 deletions Sources/TabManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,11 +726,18 @@ class TabManager: ObservableObject {
}

func startSearch() {
guard let panel = selectedTerminalPanel else { return }
if panel.searchState == nil {
guard let panel = selectedTerminalPanel else {
NSLog("Find: startSearch SKIPPED no selectedTerminalPanel")
return
}
let wasNil = panel.searchState == nil
if wasNil {
panel.searchState = TerminalSurface.SearchState()
}
NSLog("Find: startSearch workspace=%@ panel=%@", panel.workspaceId.uuidString, panel.id.uuidString)
NSLog("Find: startSearch workspace=%@ panel=%@ created=%@ firstResponder=%@",
panel.workspaceId.uuidString, panel.id.uuidString,
wasNil ? "yes" : "no(reuse)",
String(describing: panel.surface.hostedView.window?.firstResponder))
NotificationCenter.default.post(name: .ghosttySearchFocus, object: panel.surface)
_ = panel.performBindingAction("start_search")
}
Expand Down Expand Up @@ -760,6 +767,7 @@ class TabManager: ObservableObject {
}

func hideFind() {
NSLog("Find: hideFind panel=%@", selectedTerminalPanel?.id.uuidString ?? "nil")
selectedTerminalPanel?.searchState = nil
}

Expand Down
1 change: 1 addition & 0 deletions Sources/cmuxApp.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ struct cmuxApp: App {
CommandGroup(after: .textEditing) {
Menu("Find") {
Button("Find…") {
NSLog("Find: menu Cmd+F fired")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
activeTabManager.startSearch()
}
.keyboardShortcut("f", modifiers: .command)
Expand Down