Skip to content

Add vim-style search in diff pane - #9

Merged
umputun merged 3 commits into
masterfrom
diff-search
Apr 3, 2026
Merged

umputun merged 3 commits into
masterfrom
diff-search

Conversation

@umputun

@umputun umputun commented Apr 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • / opens search input in diff pane, case-insensitive substring match against diff line content
  • n/N navigate between matches with wrap-around (overrides next-file when search is active)
  • esc clears search results and highlights
  • ANSI-aware substring highlighting preserves syntax coloring
  • Status line shows match position as X/Y segment
  • Muted pipe separators in status line using raw ANSI fg sequences to preserve background
  • Default status bar foreground updated to #202020
  • Configurable search colors via --color-search-fg/--color-search-bg

umputun added 2 commits April 2, 2026 21:27
implement / to search, n/N to navigate matches, Esc to cancel.
highlights matching lines in diff view (expanded and collapsed).
adds search status indicator in status bar.
Improve search highlighting with ANSI-aware substring matching, use
muted pipe separators with raw ANSI sequences, add esc to clear search
results, update default status bar color, and fix status line formatting.
Copilot AI review requested due to automatic review settings April 3, 2026 03:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds interactive vim-style searching within the diff pane, including match navigation, highlighting, status display, and configurable match colors.

Changes:

  • Introduces / search mode with n/N match navigation and wrap-around behavior.
  • Adds ANSI-aware match highlighting during diff rendering (expanded + collapsed modes) and a X/Y status segment.
  • Adds configurable search colors (--color-search-fg/bg) and updates related docs/help text.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
ui/styles.go Adds search foreground/background colors and a SearchMatch style.
ui/styles_test.go Extends style tests to cover search color normalization and SearchMatch.
ui/search.go Implements search input lifecycle, match scanning, navigation, and match-set building.
ui/model.go Wires search key handling, status bar search segments, and search clearing on file load.
ui/model_test.go Adds extensive tests for search behavior, navigation, highlighting, and status bar output.
ui/diffview.go Integrates search match detection into rendering and implements ANSI-aware substring highlighting.
ui/collapsed.go Applies search highlighting in collapsed rendering and realigns search cursor on visibility changes.
cmd/revdiff/main.go Adds CLI flags/env/config wiring for search match colors and updates status fg default.
README.md Documents search feature and adds search color flags / keybindings.
docs/plans/completed/20260402-diff-search.md Marks the implementation plan as completed.
CLAUDE.md Documents the rendering/search pipeline changes at a high level.
.claude-plugin/skills/revdiff/references/usage.md Updates usage reference with search keybindings.
.claude-plugin/skills/revdiff/references/config.md Documents new search color flags.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ui/diffview.go Outdated
Comment on lines +207 to +209
hlOn := m.ansiFg(m.styles.colors.SearchFg) + m.ansiBg(m.styles.colors.SearchBg)
hlOff := "\033[39m\033[49m"

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

highlightSearchMatches inserts hlOff as \033[39m\033[49m, which resets to the terminal defaults (not the surrounding line/pane styling). When the line is rendered with lipgloss (e.g., add/remove background), this will cause everything after the first match to lose the original background/foreground. Consider restoring the previous SGR state (or at least reapplying the base line/pane bg/fg) instead of using 39/49 resets.

Copilot uses AI. Check for mistakes.
Comment thread ui/diffview.go Outdated
Comment on lines +207 to +208
hlOn := m.ansiFg(m.styles.colors.SearchFg) + m.ansiBg(m.styles.colors.SearchBg)
hlOff := "\033[39m\033[49m"

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

hlOn sets both foreground and background for matches. This overrides existing chroma/token foreground colors within the matched substring (e.g., \033[32m...), which conflicts with the goal of preserving syntax coloring. If preserving syntax fg is intended, limit the highlight to background-only (or only apply SearchFg when the content has no active ANSI fg).

Suggested change
hlOn := m.ansiFg(m.styles.colors.SearchFg) + m.ansiBg(m.styles.colors.SearchBg)
hlOff := "\033[39m\033[49m"
// Apply search highlight as background-only so any existing ANSI foreground
// styling (for example syntax/token colors) remains intact within matches.
hlOn := m.ansiBg(m.styles.colors.SearchBg)
hlOff := "\033[49m"

Copilot uses AI. Check for mistakes.
Comment thread ui/diffview.go

var content string
if dl.ChangeType == diff.ChangeDivider {
content = m.styles.LineNumber.Render(" " + lineContent)

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

Search matches are collected against all diffLines (including ChangeDivider), but divider lines never get match highlighting because the divider branch bypasses styleDiffContent (and thus highlightSearchMatches). This can lead to navigation landing on an un-highlighted match. Either exclude divider lines from matching, or apply search highlighting in the divider rendering path too.

Suggested change
content = m.styles.LineNumber.Render(" " + lineContent)
dividerContent := " " + lineContent
if isSearchMatch {
dividerContent = m.highlightSearchMatches(dividerContent)
}
content = m.styles.LineNumber.Render(dividerContent)

Copilot uses AI. Check for mistakes.
Comment thread README.md Outdated
@@ -174,6 +175,8 @@ All color options accept hex values (`#rrggbb`) and have corresponding `REVDIFF_
| `--color-diff-bg` | Diff pane background | terminal default |
| `--color-status-fg` | Status bar foreground | `#2D2D2D` |

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

The color options table still lists --color-status-fg default as #2D2D2D, but cmd/revdiff/main.go now defaults it to #202020. Please update the documented default to match the actual flag default.

Suggested change
| `--color-status-fg` | Status bar foreground | `#2D2D2D` |
| `--color-status-fg` | Status bar foreground | `#202020` |

Copilot uses AI. Check for mistakes.
@@ -55,6 +55,8 @@ All color options accept hex values (`#rrggbb`) and have corresponding `REVDIFF_
| `--color-diff-bg` | Diff pane background | terminal default |
| `--color-status-fg` | Status bar foreground | `#2D2D2D` |

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

This config reference still documents --color-status-fg default as #2D2D2D, but the CLI flag default in cmd/revdiff/main.go is now #202020. Please update the documented default to keep the reference accurate.

Suggested change
| `--color-status-fg` | Status bar foreground | `#2D2D2D` |
| `--color-status-fg` | Status bar foreground | `#202020` |

Copilot uses AI. Check for mistakes.
Comment thread ui/diffview.go Outdated
}

hlOn := m.ansiFg(m.styles.colors.SearchFg) + m.ansiBg(m.styles.colors.SearchBg)
hlOff := "\033[39m\033[49m"

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

highlightSearchMatches builds its highlight sequences from m.styles.colors.SearchFg/SearchBg. In --no-colors mode plainStyles() doesn't populate styles.colors, so hlOn becomes empty and search highlighting effectively disappears (and only the 39/49 resets get injected). Consider either populating styles.colors in plainStyles(), or using m.styles.SearchMatch (e.g., reverse video) for highlighting when color values are unavailable.

Suggested change
hlOff := "\033[39m\033[49m"
hlOff := "\033[39m\033[49m"
if hlOn == "" {
// In plain/no-colors mode search colors may be unset. Fall back to a
// visible non-color highlight so matches remain distinguishable.
hlOn = "\033[7m"
hlOff = "\033[27m"
}

Copilot uses AI. Check for mistakes.
Use background-only search highlight to preserve syntax colors, add
reverse video fallback for no-colors mode, show mode indicators
always (muted inactive / active fg) on right side with ≋ search icon,
darken default muted and search background colors, fix stale godoc,
update screenshot.
@umputun
umputun merged commit d565146 into master Apr 3, 2026
2 checks passed
@umputun
umputun deleted the diff-search branch April 3, 2026 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants