Show search matches as scrollbar tick marks, colored like the text highlights#176
Merged
Merged
Conversation
…llow) libghostty's search only reports match counts (SEARCH_TOTAL/SELECTED), never positions, so tick rows are recovered Macterm-side: on each total update (coalesced 100ms) the scrollback text is re-read via ghostty_surface_read_text and scanned with the same ASCII-case- insensitive literal match ghostty uses, re-wrapping logical lines at the surface width to map matches onto visual rows. The match list stays 1:1 with ghostty's so the SEARCH_SELECTED index (counted from the newest match) picks the orange tick; a count mismatch just drops the orange highlight rather than mislabeling a tick. Ticks draw in an overlay pinned to the scroller track in tile(), above the scroller so they survive overlay-scroller fade, with hit-testing disabled so the scroller keeps full click/drag behavior. Colors come from MactermTheme: ANSI yellow for matches; orange has no ANSI slot, so the selected tick is the one fixed system color.
The ticks previously used ANSI yellow + systemOrange, which didn't match what the renderer actually paints. Ghostty highlights matches with search-background / search-selected-background (defaults: golden yellow #FFE082, soft peach #F2A57E). Those keys are TerminalColor unions with no cval, so ghostty_config_get can't return them — the user's config text is scanned directly (same pattern and config-file-include limitation as ShellIntegrationFeatures), falling back to ghostty's documented defaults. Ticks grow 2x4pt to 4x2pt-inset for visibility.
github-actions
Bot
added
area:ui
📝 WalkthroughWalkthroughAdds themed search-match ticks to the terminal scrollbar. Search queries trigger debounced scrollback scans, matches are mapped to wrapped visual rows, selected results are tracked, and a non-interactive overlay renders normalized tick positions. ChangesSearch scrollbar ticks
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalPane
participant SurfaceScrollView
participant ghostty_surface_read_text
participant SearchTicks
participant SearchTickOverlay
TerminalPane->>SurfaceScrollView: noteSearchNeedle(query)
TerminalPane->>SurfaceScrollView: refreshSearchTicks()
SurfaceScrollView->>ghostty_surface_read_text: read scrollback text
SurfaceScrollView->>SearchTicks: compute match rows and selected row
SurfaceScrollView->>SearchTickOverlay: push match rows and total rows
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
Author
|
Here's an example of what it looks like: |
Contributor
Window-state benchmark
Reported value is the median of 3×10s windows per state (splitting the window and taking the median keeps one co-scheduled spike from skewing a state); CPU % is the process CPU-time delta over a window. Runs land on different shared runners, so treat small deltas as noise — 🔺/🔻 marks changes ≥25% that also clear the metric's absolute noise floor (CPU % ≥0.5, Memory (RSS MB) ≥25, CPU ms/s ≥5, Wakeups/s ≥50); CPU deltas off a noise-dominated baseline aren't flagged (CPU % baseline ≥1.5, CPU ms/s baseline ≥15). Flagged changes add the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Macterm/Views/Terminal/SurfaceScrollView.swift (1)
243-261: 🚀 Performance & Scalability | 🔵 TrivialFull scrollback rescan runs synchronously on the main thread up to ~10×/sec during active search.
scanSearchTicksreads the entire scrollback (readText(scrollback: true)) and runsSearchTicks.matchRowson it every time the 100ms debounce fires, driven by everySEARCH_TOTALupdate while ghostty streams match counts. For typical scrollback sizes this is likely negligible, but with a largescrollback-limitconfigured, this repeated synchronous main-thread work could cause visible stutter while the user types/edits a search query.Worth verifying against a large-scrollback benchmark, and if it's real, considering either a longer/adaptive debounce or offloading the read+scan to a background queue (contingent on
ghostty_surface_read_textbeing safe to call off the main actor).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Macterm/Views/Terminal/SurfaceScrollView.swift` around lines 243 - 261, Update scanSearchTicks and its 100ms debounce path to avoid repeatedly performing the full scrollback read and SearchTicks.matchRows synchronously on the main thread during active search. Verify the behavior with a large scrollback benchmark, then use a longer or adaptive debounce or safely offload the read-and-scan work from the main actor, confirming ghostty_surface_read_text is thread-safe before doing so; preserve existing tick clearing and result updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Macterm/Views/Terminal/SurfaceScrollView.swift`:
- Around line 243-261: Update scanSearchTicks and its 100ms debounce path to
avoid repeatedly performing the full scrollback read and SearchTicks.matchRows
synchronously on the main thread during active search. Verify the behavior with
a large scrollback benchmark, then use a longer or adaptive debounce or safely
offload the read-and-scan work from the main actor, confirming
ghostty_surface_read_text is thread-safe before doing so; preserve existing tick
clearing and result updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 784f963d-dd7a-43ff-b7af-9829840944a4
📒 Files selected for processing (9)
Macterm/Config/SearchHighlightColors.swiftMacterm/Ghostty/Theme.swiftMacterm/Model/SearchTicks.swiftMacterm/Model/SplitNode.swiftMacterm/Views/Terminal/SearchTickOverlay.swiftMacterm/Views/Terminal/SurfaceScrollView.swiftMacterm/Views/TerminalPane.swiftMactermTests/Config/SearchHighlightColorsTests.swiftMactermTests/Model/SearchTicksTests.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
2 participants
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
Cmd+F now marks every match's position in the scrollbar track (Xcode-style ticks), so you can see where matches live in the scrollback at a glance. The selected match's tick is drawn last and in the selected-highlight color, so it stands out from candidate matches.
Design
libghostty never reports match positions — its search API only emits counts (
SEARCH_TOTAL/SEARCH_SELECTED). So tick rows are recovered Macterm-side:SearchTicks(pure, unit-tested): reads the scrollback via the existingghostty_surface_read_textpath and scans it with the same ASCII-case-insensitive literal match ghostty's search uses (std.ascii.indexOfIgnoreCaseinsliding_window.zig), re-wrapping logical lines at the surface's column count to map matches to visual rows. The match list stays 1:1 with ghostty's, so theSEARCH_SELECTEDindex (counted from the newest match) picks the selected tick; if the counts ever disagree, the selected highlight is dropped rather than drawn on the wrong tick.SearchTickOverlay: anNSViewpinned to the scroller track intile(), above the overlay scroller so ticks stay visible while it fades out, with hit-testing disabled so scrollbar click/drag is untouched.SurfaceScrollViewre-scans onSEARCH_TOTALupdates (coalesced 100ms — totals stream while ghostty walks history), recolors the selected tick onSEARCH_SELECTED, clears on search end. The scan runs against the needle actually sent to the core, not the live (possibly newer) field text.Tick colors mirror the renderer's search highlight backgrounds (
search-background/search-selected-background, defaults golden yellow#FFE082/ soft peach#F2A57E), so ticks read as the same colors as the highlighted text. Those keys areTerminalColorunions with nocval, soghostty_config_getcan't return them — a new pureSearchHighlightColorshelper scans the user's config text (same pattern and config-file-include limitation asShellIntegrationFeatures), honoring hex overrides and falling back to the defaults for named-X11 /cell-*values a tick can't reproduce.Known approximations (noted in code comments)
Testing
SearchTicks(wrap arithmetic, case-insensitivity, non-overlap, selected-from-end mapping, multibyte counting) andSearchHighlightColors(defaults, hex/quoted parsing, last-wins, fallback semantics).mise run format,lint, and the full suite pass. Overlay drawing is AppKit view code, intentionally not unit-tested per repo conventions.Summary by CodeRabbit
New Features
Bug Fixes