Add search functionality to preview pane with highlight and navigation #388
Conversation
There was a problem hiding this comment.
Pull request overview
Adds in-preview search so users can search within the preview pane, highlight matches, and navigate results.
Changes:
- Render a preview search status/input bar when preview search is active.
- Add preview search handling (focus, regex search, highlighting, next/prev navigation).
- Extend the model to store preview content and preview-search state, and initialize/reset it on preview open.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| view.go | Shows the preview search bar when it’s active instead of the normal preview status bar. |
| preview.go | Implements preview search input handling, regex searching, highlighting, and n/N navigation. |
| main.go | Adds model state for preview search and resets it when opening preview. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
preview.go
Outdated
| // Re-highlight with current match emphasized | ||
| m.rehighlightPreview() | ||
|
|
||
| // Scroll to the line | ||
| m.preview.SetYOffset(lineNum) | ||
| } |
There was a problem hiding this comment.
Navigating search results (n/N) calls rehighlightPreview(), which recompiles the regex and reprocesses the entire previewContent every time. For large previews this is O(total_lines) per keypress and can noticeably lag. Consider caching the compiled regexp + per-line match indexes from the initial search, and only updating the emphasis for the currently selected result (or highlighting during rendering for visible lines only).
preview.go
Outdated
| func (m *model) highlightLine(line string, matches [][]int, currentMatchLine int) string { | ||
| if len(matches) == 0 { | ||
| return line | ||
| } | ||
|
|
||
| var result strings.Builder | ||
| lastEnd := 0 | ||
|
|
||
| for matchIdx, match := range matches { | ||
| start, end := match[0], match[1] | ||
|
|
||
| // Add text before the match | ||
| if start > lastEnd { | ||
| result.WriteString(line[lastEnd:start]) | ||
| } | ||
|
|
||
| // Highlight the match | ||
| matchText := line[start:end] | ||
| if currentMatchLine >= 0 && matchIdx == 0 { | ||
| // Current search result - use cursor style | ||
| result.WriteString(theme.CurrentTheme.Cursor(matchText)) | ||
| } else { |
There was a problem hiding this comment.
highlightLine’s currentMatchLine parameter is effectively used as a boolean (only checked for >= 0) and the actual line number value is unused, which is confusing. Consider changing it to a bool like isCurrentResultLine (or pass the selected match index if you intend to highlight a specific match).
| case key.Matches(msg, keyMap.Search): | ||
| m.previewSearchInput.CursorEnd() | ||
| m.previewSearchInput.Width = m.termWidth - 2 | ||
| m.previewSearchInput.Focus() | ||
| return m, nil | ||
|
|
||
| case key.Matches(msg, keyMap.SearchNext): | ||
| m.selectPreviewSearchResult(m.previewSearchCursor + 1) | ||
| return m, nil | ||
|
|
||
| case key.Matches(msg, keyMap.SearchPrev): | ||
| m.selectPreviewSearchResult(m.previewSearchCursor - 1) | ||
| return m, nil |
There was a problem hiding this comment.
This PR adds substantial new preview search behavior (focus handling, regex parsing, highlighting, and n/N navigation), but there are no tests covering it. There are existing Bubble Tea integration tests in main_test.go; consider adding a test that opens preview, performs a search, and verifies that the status bar and navigation update as expected (including invalid-regex handling).
… line numbers and simplify content handling in preview rendering
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| m.previewContent = value | ||
| m.previewSearchInput.SetValue("") | ||
| m.previewSearchResults = nil | ||
| m.previewSearchCursor = -1 | ||
| m.preview.SetContent(view) |
There was a problem hiding this comment.
m.previewContent is set to the raw node value, but the viewport content is set to view (which may be wrapped/padded by lipgloss or even an ASCII image). Since preview search uses m.previewContent for matching and line-offset calculation, and Escape/not-found restores m.previewContent, this can (a) search the wrong text, (b) scroll to incorrect Y offsets, and (c) revert the preview to the raw/base64 string instead of the rendered preview. Store the exact base content shown in the preview viewport (e.g., assign m.previewContent = view), or split into separate fields for ‘display content’ and ‘search text’ and restore the display content on cancel.
| case msg.Type == tea.KeyEscape: | ||
| m.previewSearchInput.Blur() | ||
| m.previewSearchInput.SetValue("") | ||
| m.previewSearchResults = nil | ||
| m.previewSearchCursor = -1 | ||
| m.preview.SetContent(m.previewContent) | ||
| return m, nil | ||
|
|
||
| case msg.Type == tea.KeyEnter: | ||
| m.previewSearchInput.Blur() | ||
| found := m.doPreviewSearch(m.previewSearchInput.Value()) | ||
| if !found { | ||
| m.previewSearchResults = nil | ||
| m.previewSearchCursor = -1 | ||
| m.preview.SetContent(m.previewContent) | ||
| } |
There was a problem hiding this comment.
The preview search cancel / not-found paths call m.preview.SetContent(m.previewContent), but previewContent is intended to represent the un-highlighted preview content. With the current setup it can diverge from what was actually rendered into the viewport (wrapping/padding, images), so canceling a search may restore the wrong content and break scrolling offsets. Use a stored ‘base preview display’ string (the same one initially passed to preview.SetContent) when restoring, rather than the raw node value.
| func (m *model) doPreviewSearch(pattern string) bool { | ||
| if pattern == "" { | ||
| return false | ||
| } | ||
|
|
There was a problem hiding this comment.
New preview search behavior (/, Enter, n/N navigation, highlighting) isn’t covered by automated tests. There are existing Bubble Tea integration tests in main_test.go using teatest; adding a similar test that opens preview, performs a search, and asserts the rendered output / scroll position would help prevent regressions in this interactive flow.
No description provided.