Add in-terminal search (Cmd+F) with floating overlay#27
Merged
Conversation
Handles GHOSTTY_ACTION_START/END_SEARCH and SEARCH_TOTAL/SELECTED actions in the runtime. A floating TerminalSearchOverlay appears over the active pane with a debounced needle field, match count, and prev/next navigation. Cmd+F triggers search; Escape or the X button dismisses it.
Replace the floating card with a compact full-width bar pinned to the top of the terminal area. Uses rounded top corners, a bottom separator, and small icon-only nav buttons to match Ghostty's native search look.
Full-width bar covered terminal content. Switch to a compact panel anchored to the top-right of the terminal (matching Ghostty's layout) with all-round corner radius, thin border, and a subtle drop shadow.
Add fixedSize(horizontal: true, vertical: false) so the overlay uses its natural compact width before the positioning frame expands.
📝 WalkthroughWalkthroughAdds in-terminal search: new observable SurfaceSearchState, a TerminalSearchOverlay UI, GhosttyRuntime search APIs and callbacks, GhosttyTerminalView wiring, keyboard/command palette triggers, and surface search lifecycle handling. Changes
Sequence DiagramsequenceDiagram
actor User
participant View as GhosttyTerminalView
participant Overlay as TerminalSearchOverlay
participant Runtime as GhosttyRuntime
participant Engine as Terminal Engine
User->>View: Press Cmd+F
View->>Runtime: startSearch(surfaceId, needle: "")
Runtime->>Runtime: create/update SurfaceSearchState
Runtime->>View: onSearchStateChange(state)
View->>Overlay: render with state
User->>Overlay: Type needle
Overlay->>Runtime: update needle (binding)
Runtime->>Engine: dispatch search query
Engine->>Runtime: SEARCH_TOTAL / SEARCH_SELECTED actions
Runtime->>View: onSearchStateChange(updated state)
View->>Overlay: update counts/selection
User->>Overlay: Next/Previous
Overlay->>Runtime: navigate_next / navigate_previous
Runtime->>Engine: dispatch navigation
User->>Overlay: Press Escape / Close
Overlay->>Runtime: endSearch(surfaceId)
Runtime->>View: onSearchStateChange(nil)
View->>Overlay: dismiss
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift (1)
78-80: Consider usingAppThemefor the background color.The hardcoded color value could make future theming difficult. Other components like
CommandPaletteViewuseAppTheme.panelGradientfor similar overlay backgrounds.♻️ Suggested change
.background( - Color(nsColor: NSColor(calibratedRed: 0.16, green: 0.17, blue: 0.22, alpha: 0.97)) + AppTheme.panelGradient )Alternatively, if this specific color is intentional for search overlays, consider adding a dedicated
AppTheme.searchOverlayBackgroundproperty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift` around lines 78 - 80, The background is using a hardcoded Color in TerminalSearchOverlay (the .background(Color(nsColor: ...)) call) which prevents consistent theming; replace that hardcoded value with the project theme value (use AppTheme.panelGradient as used by CommandPaletteView) or add and use a new AppTheme.searchOverlayBackground if this overlay needs a distinct color, and update the .background call in TerminalSearchOverlay to reference the AppTheme property instead.Sources/Shellraiser/Infrastructure/Ghostty/GhosttyRuntime.swift (1)
238-255: Review the debounce logic and redundant main queue dispatch.The debounce implementation has two observations:
Redundant dispatch: Since
GhosttyRuntimeis@MainActorand the delay scheduler isDispatchQueue.main, theDispatchQueue.main.asyncinside the sink may be unnecessary.Debounce threshold: Short needles (< 3 chars) get a 300ms delay while longer needles fire immediately. This is a reasonable UX choice, though consider whether 2-char searches might benefit from less delay.
♻️ Potential simplification
.switchToLatest() .sink { [weak self] needle in - DispatchQueue.main.async { [weak self] in - _ = self?.performBindingAction(surfaceId: surfaceId, action: "search:\(needle)") - } + _ = self?.performBindingAction(surfaceId: surfaceId, action: "search:\(needle)") }The publisher already operates on the main queue via the delay scheduler, so the explicit dispatch should be unnecessary. However, verify this doesn't cause issues with Combine's threading guarantees.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Shellraiser/Features/WorkspaceDetail/PaneLeafView.swift`:
- Around line 258-284: The callbacks for TerminalSearchOverlay are calling
GhosttyRuntime.shared.performBindingAction with action strings
"navigate_search:next" and "navigate_search:previous", but Ghostty expects the
single action "navigate_search"; update the onNavigateNext and
onNavigatePrevious closures to call
GhosttyRuntime.shared.performBindingAction(surfaceId: action:) using the action
string "navigate_search" (same as onClose uses GhosttyRuntime.shared.endSearch)
so both navigation callbacks match the API; locate these changes in the
TerminalSearchOverlay block where activeSurface?.id is used and adjust the
action parameter passed to .performBindingAction accordingly.
---
Nitpick comments:
In `@Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift`:
- Around line 78-80: The background is using a hardcoded Color in
TerminalSearchOverlay (the .background(Color(nsColor: ...)) call) which prevents
consistent theming; replace that hardcoded value with the project theme value
(use AppTheme.panelGradient as used by CommandPaletteView) or add and use a new
AppTheme.searchOverlayBackground if this overlay needs a distinct color, and
update the .background call in TerminalSearchOverlay to reference the AppTheme
property instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5385a42-6042-4ea5-bea9-535dd528db87
📒 Files selected for processing (9)
Shellraiser.xcodeproj/project.pbxprojSources/Shellraiser/Features/Terminal/GhosttyTerminalView.swiftSources/Shellraiser/Features/WorkspaceDetail/PaneLeafView.swiftSources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swiftSources/Shellraiser/Infrastructure/Ghostty/GhosttyRuntime.swiftSources/Shellraiser/Models/SurfaceSearchState.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager+CommandPalette.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager+Shortcuts.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager+SurfaceOperations.swift
Comment on lines
+258
to
+284
| if let searchState { | ||
| TerminalSearchOverlay( | ||
| searchState: searchState, | ||
| onNavigateNext: { | ||
| guard let surfaceId = activeSurface?.id else { return } | ||
| GhosttyRuntime.shared.performBindingAction( | ||
| surfaceId: surfaceId, | ||
| action: "navigate_search:next" | ||
| ) | ||
| }, | ||
| onNavigatePrevious: { | ||
| guard let surfaceId = activeSurface?.id else { return } | ||
| GhosttyRuntime.shared.performBindingAction( | ||
| surfaceId: surfaceId, | ||
| action: "navigate_search:previous" | ||
| ) | ||
| }, | ||
| onClose: { | ||
| guard let surfaceId = activeSurface?.id else { return } | ||
| GhosttyRuntime.shared.endSearch(surfaceId: surfaceId) | ||
| } | ||
| ) | ||
| .fixedSize(horizontal: true, vertical: false) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topTrailing) | ||
| .padding(.trailing, 20) | ||
| .padding(.top, 14) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ghostty terminal navigate_search binding action format
💡 Result:
In Ghostty, the keybind action is just navigate_search (no :parameter part). (ghostty.org)
So the binding format is the normal:
keybind = <trigger>=navigate_searchTo match the defaults mentioned in the 1.3.0 release notes (next vs previous result), you bind both keys to the same action (the “shift” variant is used for previous): (ghostty.org)
# macOS
keybind = cmd+g=navigate_search
keybind = shift+cmd+g=navigate_search
# GTK (Linux)
keybind = enter=navigate_search
keybind = shift+enter=navigate_searchCitations:
- 1: https://ghostty.org/docs/config/keybind/reference?utm_source=openai
- 2: https://ghostty.org/docs/install/release-notes/1-3-0?utm_source=openai
Fix navigate_search action strings to match Ghostty's API format.
The callbacks use "navigate_search:next" and "navigate_search:previous", but Ghostty's binding action is simply navigate_search without parameter variants. The distinction between next/previous is made via the trigger key binding (shift modifier), not action parameters. Use "navigate_search" for both actions to match the documented API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Shellraiser/Features/WorkspaceDetail/PaneLeafView.swift` around lines
258 - 284, The callbacks for TerminalSearchOverlay are calling
GhosttyRuntime.shared.performBindingAction with action strings
"navigate_search:next" and "navigate_search:previous", but Ghostty expects the
single action "navigate_search"; update the onNavigateNext and
onNavigatePrevious closures to call
GhosttyRuntime.shared.performBindingAction(surfaceId: action:) using the action
string "navigate_search" (same as onClose uses GhosttyRuntime.shared.endSearch)
so both navigation callbacks match the API; locate these changes in the
TerminalSearchOverlay block where activeSurface?.id is used and adjust the
action parameter passed to .performBindingAction accordingly.
Replaces hardcoded NSColor with AppTheme.panelGradient for theme consistency with ChromeCardModifier and other chrome elements.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift (3)
105-105: Add a concise doc comment forSearchNavButtonStyle.
SearchNavButtonStyleis a non-trivial type and should be documented like the other types in this file.📝 Suggested change
+/// Button style used by search navigation controls for pressed/idle states. private struct SearchNavButtonStyle: ButtonStyle {As per coding guidelines,
Add concise doc comments (///) for non-trivial types and functions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift` at line 105, Add a concise doc comment (///) above the SearchNavButtonStyle declaration that briefly explains its purpose and usage (e.g., that it customizes the appearance/interaction of search navigation buttons in the terminal overlay), mention important behavior or parameters if relevant, and follow the same style/format as other type comments in the file; place the comment immediately before the `private struct SearchNavButtonStyle: ButtonStyle` declaration so IDEs and generated docs surface it.
92-103: RenameSearchNavButtonto include aViewsuffix for guideline consistency.This type conforms to
View; naming it with a*Viewsuffix keeps UI type naming consistent and easier to scan.♻️ Suggested rename
-private struct SearchNavButton: View { +private struct SearchNavButtonView: View { let systemImage: String let action: () -> Void var body: some View { Button(action: action) { Image(systemName: systemImage) .font(.system(size: 11, weight: .medium)) } .buttonStyle(SearchNavButtonStyle()) } } ... - SearchNavButton(systemImage: "chevron.up", action: onNavigatePrevious) + SearchNavButtonView(systemImage: "chevron.up", action: onNavigatePrevious) .help("Previous match") - SearchNavButton(systemImage: "chevron.down", action: onNavigateNext) + SearchNavButtonView(systemImage: "chevron.down", action: onNavigateNext) .help("Next match") ... - SearchNavButton(systemImage: "xmark", action: onClose) + SearchNavButtonView(systemImage: "xmark", action: onClose) .help("Close search")As per coding guidelines,
Keep model and manager names explicit with suffixes (*Model, *Manager, *View).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift` around lines 92 - 103, Rename the SwiftUI view type SearchNavButton to include the View suffix (e.g., SearchNavButtonView) to follow UI naming guidelines; update the struct declaration and all references/usages (including the initializer signature that takes systemImage and action, and any places using SearchNavButtonStyle) so the type name matches, and ensure any file-private/test references or previews are also updated to the new SearchNavButtonView identifier.
62-73: Add explicit accessibility labels to icon-only buttons.
.help(...)is good for tooltips, but explicitaccessibilityLabelmakes VoiceOver output clearer and task-oriented.♿ Suggested accessibility tweak
- SearchNavButton(systemImage: "chevron.up", action: onNavigatePrevious) + SearchNavButton(systemImage: "chevron.up", action: onNavigatePrevious) .help("Previous match") + .accessibilityLabel("Previous match") - SearchNavButton(systemImage: "chevron.down", action: onNavigateNext) + SearchNavButton(systemImage: "chevron.down", action: onNavigateNext) .help("Next match") + .accessibilityLabel("Next match") ... - SearchNavButton(systemImage: "xmark", action: onClose) + SearchNavButton(systemImage: "xmark", action: onClose) .help("Close search") + .accessibilityLabel("Close search")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift` around lines 62 - 73, Add explicit accessibility labels to the icon-only buttons in TerminalSearchOverlay by setting accessibilityLabel on each SearchNavButton instance (the ones using systemImage "chevron.up", "chevron.down", and "xmark") so VoiceOver reads task-oriented text like "Previous match", "Next match", and "Close search" instead of relying only on .help tooltips; update the SearchNavButton usages in TerminalSearchOverlay.swift to call .accessibilityLabel(...) with the matching strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift`:
- Line 105: Add a concise doc comment (///) above the SearchNavButtonStyle
declaration that briefly explains its purpose and usage (e.g., that it
customizes the appearance/interaction of search navigation buttons in the
terminal overlay), mention important behavior or parameters if relevant, and
follow the same style/format as other type comments in the file; place the
comment immediately before the `private struct SearchNavButtonStyle:
ButtonStyle` declaration so IDEs and generated docs surface it.
- Around line 92-103: Rename the SwiftUI view type SearchNavButton to include
the View suffix (e.g., SearchNavButtonView) to follow UI naming guidelines;
update the struct declaration and all references/usages (including the
initializer signature that takes systemImage and action, and any places using
SearchNavButtonStyle) so the type name matches, and ensure any file-private/test
references or previews are also updated to the new SearchNavButtonView
identifier.
- Around line 62-73: Add explicit accessibility labels to the icon-only buttons
in TerminalSearchOverlay by setting accessibilityLabel on each SearchNavButton
instance (the ones using systemImage "chevron.up", "chevron.down", and "xmark")
so VoiceOver reads task-oriented text like "Previous match", "Next match", and
"Close search" instead of relying only on .help tooltips; update the
SearchNavButton usages in TerminalSearchOverlay.swift to call
.accessibilityLabel(...) with the matching strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a7fc5f1-2b72-41f7-ad08-826d50149e09
📒 Files selected for processing (1)
Sources/Shellraiser/Features/WorkspaceDetail/TerminalSearchOverlay.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit