feat(config): add ui.show_preview to open preview pane at launch - #48
Merged
Conversation
The dashboard preview pane always starts hidden, and the only way to get it back is pressing 'p' every single session. If you live in preview mode — say, reading reddit megathreads — that's a pointless ritual. The reason is that preview visibility was a runtime-only bool, hard-coded to false in App::new with no config behind it. Let's fix that: add ui.show_preview (default false, so nobody's setup changes), read once at startup. 'p' still toggles at runtime, and the new key shows up in `feedr config list`, get/set, and the TUI config editor like every other setting. While at it, fix test_toggle_preview_pane: it asserted the launch default, which now depends on whatever config.toml happens to be on the developer's machine. Tests that read your personal dotfiles are not tests, they're surprises. Fixes #40.
Feedr has been treating every feed item as a bag of text, which is fine for blogs and useless for everything else. YouTube channels, podcasts, and web comics all ship their actual payload as <media:content>, <enclosure>, or Atom <content src> — and we were throwing all of it away. So, three things, which honestly should have been three commits: Parse media attachments and thumbnails into FeedItem, and expose them to macros as %m (primary media URL), %M (its MIME type) and %i (thumbnail). These are *orthogonal* to %u — no silent fallback to the page URL, because "mpv %m" quietly playing a web page instead of the episode is exactly the kind of helpfulness nobody asked for. Absent media expands to "". Render thumbnails inline in the detail view via the Kitty graphics protocol (kitty, Ghostty, WezTerm; silent no-op elsewhere, including tmux, which eats APC sequences). The trick: the detail view reserves a blank strip and the escapes are emitted *after* terminal.draw() returns, so ratatui's diff never fights us over those cells. Fetches run on background threads gated by is_safe_auto_url AND a redirect policy that re-validates every hop — thumbnails are hostile feed content fetched with no user action, so a public-looking URL that 302s into your router does not get to win. Decoding runs under explicit limits (a 5 MB PNG that inflates to gigabytes is treated as the attack it is), the decoded cache is LRU-capped, and frames with a visible modal suppress the image so it can't paint over the help overlay. Stop rendering RSS summaries through html2text's defaults. Reddit-style layout tables came out as box-drawn | columns that mangled on rewrap, and every anchor grew [N] markers plus a footnote dump of URLs nobody can click in a TUI. Flatten table markup before conversion and use a decorator that keeps emphasis but drops link annotations. The article URL is already in the header; we don't need it forty more times at the bottom.
The image crate bumped its rust-version to 1.88 somewhere around 0.25.7, while the rest of our dependency graph tops out at 1.81. Shipping 0.25.10 would have quietly made feedr require a months-old-at-best compiler for exactly zero features we use. Pin to 0.25.6 in the lockfile, same pattern as the dom_smoothie pin: the manifest stays at "0.25", the committed lockfile enforces the real version, and the manifest comment makes it clear that bumping past this is a toolchain-floor decision, not routine maintenance. For the record: the CI job that claims to test the 1.75 MSRV has been silently running stable all along, because rust-toolchain.toml pins "stable" and overrides the matrix toolchain. So nothing would have caught this. That's a pre-existing problem, and it's not getting fixed from a feature branch.
The detail and dashboard views render descriptions through CleanDecorator — no [N] link markers, no footnote dump, tables flattened. But the cached plain_text, which feeds pipe-to payloads and the content-length filter, was still going through the stock html2text decorator. So what you piped to your script was not what you were reading on screen. This is not great. Move CleanDecorator and render_clean_html from ui/utils.rs into feed.rs and build plain_text with them at parse time. They live in feed.rs not because they're prettier there, but because the parse path needs them and ui already depends on feed — the reverse dependency would have been a layering violation waiting to breed. One text pipeline. What you see, search, filter, and pipe is now the same string.
A batch of lifecycle bugs in the Kitty integration, all found in review, and all variations of "the protocol is keyed differently than you think". First, clear_terminal used d=a, which deletes *placements* but keeps the transmitted pixel data in the terminal's registry — and then threw away our own kitty_images map. So every view re-entry re-encoded and re-transmitted the PNG under a fresh id while the old data sat orphaned in the terminal, bounded only by the terminal's own quota. The comment claiming delete-all wiped the registry was simply wrong. Now kitty_images survives clears: re-entering a view is one cheap placement escape, no re-encode, no re-transmit. Terminal-side data is freed exactly when a URL falls out of our LRU — eviction queues the id and the next write flushes a d=I (uppercase: placements *and* data), so terminal image memory is bounded by MAX_CACHED_IMAGES instead of by hope. Second, placements are keyed by (image id, placement id), so re-using p=1 across *different* image ids stacks both images in the strip. Nothing hits that today because every image-to-image transition happens to pass through an imageless frame, but "happens to" is not an invariant — track last_placed_id and delete the old placement when the id changes. While at it: spawn fetch workers via thread::Builder so an OS thread-creation failure releases the in-flight slot instead of panicking the TUI (the fulltext workers already did this; the image path just forgot), and suppress the 10-row image strip in compact mode or on detail areas under 30 rows, where header plus strip would squeeze the article down to one visible line. A thumbnail is decoration. The article is the point.
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
Adds a
ui.show_previewconfig option (bool, defaultfalse) so the dashboard preview pane can start open at launch instead of requiringpevery session. Requested in #40, where the use case is reddit megathread feeds that are basically unreadable without the preview pane.show_preview = trueunder[ui]inconfig.tomlopens the preview pane at startup;pstill toggles it at runtimefeedr config list/get/set(withtrue/falsevalidation) and the TUI config editor (feedr config --tui, Enter toggles it)#[serde(default)]keeps existing config files loading unchanged — omitted key meansfalse, i.e. exactly today's behaviorAlso fixes
test_toggle_preview_pane, which asserted the launch default ofApp::new()— that default now depends on the developer's real~/.config/feedr/config.toml, so the test now sets its initial state explicitly and asserts only toggle behavior.Fixes #40
Test plan
cargo test— 215 tests pass, including two new unit tests: back-compat parse of a[ui]table without the key, andget_value/validate_and_setround-trip with invalid-value rejectioncargo clippy --all-targets --all-features -- -D warningscleancargo fmt --all -- --checkcleanshow_preview = true, launch opens the dashboard with the preview pane visible;phides it; relaunch shows it again