feat(detail): in-article find with smart-case matching#44
Merged
Conversation
Full-text extraction landed in v0.7.0 (#43), which is great right up until you open a 5000-word article and want to find a specific phrase. Then it's just you, j/k, and your patience. Before this commit `/` in the detail view opened the *cross-feed* search modal, which made approximately zero sense — you're staring at one article and pressing slash drops a full-screen overlay that filters your entire bookmark list. Let's fix that. Add a real find-in-page: `/` enters a new `ArticleSearch` input mode, live-highlights matches as you type, `n`/`N` cycle through them with wrap-around, and `ESC` clears. The query persists across `Shift+F` toggling between summary and extracted body, so the same search works whether you're reading the feed-provided HTML or the Readability-extracted plain text. Focus change to a different article drops the query — a stale `/foo` following you to the next article is the kind of UX that makes people uninstall things. Matching is ripgrep-style smart-case: case-sensitive when the query has any uppercase character, case-insensitive (ASCII fold) otherwise. Highlights render as styled spans inside a `Vec<Line>` fed to the existing `Paragraph` widget, so soft-wrap continues to work for free. The current match (the one `n` lands on) gets the filled-block treatment; non-current matches stay subtle so the page doesn't look like a Christmas tree. Jump-to-match uses a sibling helper to `count_wrapped_lines` that returns the cumulative wrapped-row index for a given source line, so the viewport scrolls exactly where the wrapped output puts the match — no drift, no math at three different abstraction layers. Body and content-width are cached on `App` at render time so the event handler can resolve match → scroll position without re-deriving the body. State lives only in `App`; nothing persisted. 22 new tests cover the helpers, the state machine, and the event dispatch.
Self-review of the in-article-find diff turned up four things worth fixing before someone else trips on them. None are user-visible today; they're all latent. *exit_detail_view* cleared the search query but left *input_mode* alone. Today no code path triggers the bad case — but the day someone wires a macro or remote-trigger that swaps views, the user gets stranded in ArticleSearch mode in a view that has no footer. One line, fixed. The detail renderer was cloning the entire rendered body into *article_body_cache* every frame regardless of whether search was active — about 500KB/s of churn on a 50KB full-text article for a feature nobody was using. Gate it on "search footer visible" and the steady state goes to zero allocations. The doc on *find_article_matches* called the ASCII-only fold a "v1 limitation." It is not a limitation, it is a load-bearing *invariant*: the highlight render slices the original line at the byte offsets the search reported, which only works because *to_ascii_lowercase* is byte-length-preserving. *str::to_lowercase* — which someone will reach for when they want proper Unicode — is *not* length-preserving (\`"İ".to_lowercase()\` is two code points) and would silently invalidate the offset model. Reframe the comment so a future maintainer can't miss it. While at it: *build_styled_body* had a dead *'a* lifetime and was *to_string()*-ing every unmatched line. Borrow from *body* via *Line::from(&str)* and *Span::raw(&str)*, replace the HashMap bucketing with a linear walk over already-sorted matches. Tests added for the invariant, multi-byte offset correctness, *input_mode* reset, and *build_styled_body* coverage.
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
/-style find-in-page to the article detail view: live highlights as you type,n/Nto cycle matches with wrap-around,ESCto clear.Shift+Ftoggling, dies on article focus change./in the detail view from cross-feed search (which made little sense from inside an article) to article search.KeyActions:OpenArticleSearch(/),NextMatch(n),PrevMatch(N). All remappable via the existing[keybindings]config.Design notes
Vec<Line>soParagraph::wrapkeeps doing soft-wrap for free; match ranges are emitted as styled spans within each source line.ncursor sits on) gets the bold filled block; non-current matches stay subtle (foreground tint, no fill) so a heavily-matched page doesn't look like a Christmas tree.count_wrapped_lines(wrapped_row_of_line) that returns the cumulative wrapped-row index for a given source line, so scrolling lands exactly where the wrapped output puts the match.Appat render time son/Nevent handling can resolve match → scroll without re-deriving the body source. State is in-memory only — nothing persisted.Test plan
cargo build --release— cleancargo fmt --all -- --check— cleancargo clippy --all-targets --all-features -- -D warnings— cleancargo test --all-features— 199 lib tests + 4 integration tests pass (22 new tests added: pure helpers, scroll math, state machine, and event-dispatch mode transitions)/foohighlights,n/Ncycle and viewport scrolls,ESCclears,Shift+Fkeeps the query alive on the same article, navigating to a different article drops the queryFoo) only matches case-sensitively; all-lowercase matches case-insensitively