From 98b819a4e18983af110c335fa909525eb772466e Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Thu, 11 Jun 2026 20:35:07 +0530 Subject: [PATCH 1/5] feat(config): add ui.show_preview to open preview pane at launch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- README.md | 4 +++- src/app.rs | 8 ++++++-- src/config.rs | 39 +++++++++++++++++++++++++++++++++++++++ src/config_cli.rs | 4 ++++ src/config_tui.rs | 7 +++++++ 5 files changed, 59 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e4d4784..c8e7ad7 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Feedr is a feature-rich terminal-based RSS feed reader written in Rust. It provi - **Summary View**: "What's New" screen shows articles added since your last session with per-feed stats - **Read/Unread Tracking**: Persistent read state tracking across sessions - **Mark All Read**: Quickly mark all visible items as read with `m` -- **Article Preview**: Toggle an inline preview pane in the dashboard view +- **Article Preview**: Toggle an inline preview pane in the dashboard view, or have it open at launch with `show_preview = true` - **Link Extraction**: Extract and browse all links from an article with `l` - **Full-Text Extraction**: Strip away summaries and read the actual article content inline via Mozilla Readability — manual on `Shift+F`, or auto-extract on refresh per feed with `fulltext = true` - **Help Overlay**: Press `?` for a scrollable keybinding reference overlay @@ -246,6 +246,7 @@ tick_rate = 100 # UI update rate in milliseconds error_display_timeout = 3000 # Error message duration in milliseconds theme = "dark" # Theme: "dark" (cyberpunk) or "light" (zen) compact_mode = "auto" # Compact layout: "auto", "always", or "never" +show_preview = false # Start with the dashboard preview pane open # Optional: Define default feeds to load on first run [[default_feeds]] @@ -276,6 +277,7 @@ Authorization = "Bearer your_token_here" - **error_display_timeout**: How long error messages are displayed in milliseconds - **theme**: Choose between `"dark"` (cyberpunk aesthetic with neon colors) or `"light"` (zen minimalist with organic colors). Can also be toggled at runtime with `t`. - **compact_mode**: Controls the compact layout for small terminals. `"auto"` (default) enables compact mode when terminal height is ≤30 rows, `"always"` forces compact mode, and `"never"` disables it. Compact mode uses single-line items, a minimal title bar, and an abbreviated help bar to maximize screen real estate. +- **show_preview**: When `true`, the dashboard preview pane is open at launch (default: `false`). The `p` key still toggles it at runtime. Note that the preview pane is always hidden while compact mode is active. #### Background Refresh Example To enable automatic refresh every 5 minutes with rate limiting: diff --git a/src/app.rs b/src/app.rs index c2e3257..c0a8221 100644 --- a/src/app.rs +++ b/src/app.rs @@ -433,6 +433,8 @@ impl App { } }); + let show_preview = config.ui.show_preview; + let mut app = Self { config, feeds: Vec::new(), @@ -468,7 +470,7 @@ impl App { color_scheme, last_session_time, show_summary, - preview_pane: false, + preview_pane: show_preview, preview_scroll: 0, preview_max_scroll: 0, feed_headers, @@ -2661,7 +2663,9 @@ mod tests { #[test] fn test_toggle_preview_pane() { let mut app = make_test_app(); - assert!(!app.preview_pane); + // Initial state depends on ui.show_preview in the loaded config, so set + // it explicitly — this test asserts toggle behavior, not the default. + app.preview_pane = false; app.preview_scroll = 5; app.preview_max_scroll = 10; diff --git a/src/config.rs b/src/config.rs index de91143..5d60905 100644 --- a/src/config.rs +++ b/src/config.rs @@ -100,6 +100,9 @@ pub struct UiConfig { /// Compact mode for small terminals (auto, always, never) #[serde(default)] pub compact_mode: CompactMode, + /// Show the dashboard preview pane on launch + #[serde(default)] + pub show_preview: bool, } #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] @@ -187,6 +190,7 @@ impl Default for UiConfig { error_display_timeout: default_error_timeout(), theme: Theme::default(), compact_mode: CompactMode::default(), + show_preview: false, } } } @@ -226,6 +230,7 @@ impl Config { "ui.error_display_timeout" => Ok(self.ui.error_display_timeout.to_string()), "ui.theme" => Ok(self.ui.theme.to_string()), "ui.compact_mode" => Ok(self.ui.compact_mode.to_string()), + "ui.show_preview" => Ok(self.ui.show_preview.to_string()), k if k.starts_with("default_feeds") => { bail!("Feed management is not supported via CLI. Use 'feedr config --tui' instead.") } @@ -302,6 +307,10 @@ impl Config { value ), }, + "ui.show_preview" => { + let v: bool = value.parse().context("Expected 'true' or 'false'")?; + self.ui.show_preview = v; + } k if k.starts_with("default_feeds") => { bail!("Feed management is not supported via CLI. Use 'feedr config --tui' instead.") } @@ -384,6 +393,8 @@ impl Config { # UI Theme Settings:\n\ # - theme: Choose between \"light\" or \"dark\" theme (default: dark)\n\ # You can also toggle the theme in the app by pressing 't'\n\ + # - show_preview: Start with the dashboard preview pane open (default: false)\n\ + # You can still toggle the preview pane in the app by pressing 'p'\n\ #\n\ # Example configuration for auto-refresh every 5 minutes:\n\ # [general]\n\ @@ -394,6 +405,7 @@ impl Config { # [ui]\n\ # theme = \"light\"\n\ # compact_mode = \"auto\" # auto (default), always, or never\n\ + # show_preview = false # start with the dashboard preview pane open\n\ #\n\ # Example default feeds configuration:\n\ # [[default_feeds]]\n\ @@ -576,6 +588,33 @@ mod tests { assert_eq!(config.macro_options.pipe_default_stdin, "body"); } + #[test] + fn test_show_preview_back_compat_and_default() { + // An old config with a [ui] table but no show_preview key must + // still load, defaulting to false. + let toml_str = r#" + [ui] + theme = "dark" + "#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!(!config.ui.show_preview); + assert!(!Config::default().ui.show_preview); + } + + #[test] + fn test_show_preview_get_set() { + let mut config = Config::default(); + assert_eq!(config.get_value("ui.show_preview").unwrap(), "false"); + + config.validate_and_set("ui.show_preview", "true").unwrap(); + assert!(config.ui.show_preview); + assert_eq!(config.get_value("ui.show_preview").unwrap(), "true"); + + assert!(config.validate_and_set("ui.show_preview", "yes").is_err()); + // Failed set must not clobber the previous value + assert!(config.ui.show_preview); + } + #[test] fn test_config_serialization() { let config = Config::default(); diff --git a/src/config_cli.rs b/src/config_cli.rs index 80de8c4..52c4c02 100644 --- a/src/config_cli.rs +++ b/src/config_cli.rs @@ -48,6 +48,10 @@ pub fn list() -> Result<()> { ), ("ui.theme", "Color theme (light, dark)"), ("ui.compact_mode", "Compact mode (auto, always, never)"), + ( + "ui.show_preview", + "Show preview pane on launch (true/false)", + ), ]; for (key, desc) in keys { diff --git a/src/config_tui.rs b/src/config_tui.rs index 34700f6..33c8281 100644 --- a/src/config_tui.rs +++ b/src/config_tui.rs @@ -146,6 +146,13 @@ pub fn get_fields(section: ConfigSection, config: &Config) -> Vec { kind: FieldKind::Enum, description: "auto, always, never".into(), }, + FieldInfo { + key: "ui.show_preview".into(), + label: "Show Preview Pane".into(), + value: config.ui.show_preview.to_string(), + kind: FieldKind::Bool, + description: "true, false — preview pane open at launch".into(), + }, ], ConfigSection::DefaultFeeds => { if config.default_feeds.is_empty() { From ce53aee0b8bdcfed5cd2850ae6f07aa54df81f91 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Thu, 11 Jun 2026 21:16:24 +0530 Subject: [PATCH 2/5] feat: media attachments, inline Kitty images, and clean HTML rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 , , or Atom — 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. --- CLAUDE.md | 3 +- Cargo.lock | 140 ++++++++- Cargo.toml | 6 + README.md | 12 + examples/dump_media.rs | 48 +++ src/app.rs | 121 +++++++- src/events.rs | 6 + src/feed.rs | 642 ++++++++++++++++++++++++++++++++++++++- src/image.rs | 659 +++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/tui.rs | 85 ++++++ src/ui/dashboard.rs | 4 +- src/ui/detail.rs | 116 ++++++-- src/ui/mod.rs | 22 ++ src/ui/utils.rs | 143 +++++++++ 15 files changed, 1968 insertions(+), 40 deletions(-) create mode 100644 examples/dump_media.rs create mode 100644 src/image.rs diff --git a/CLAUDE.md b/CLAUDE.md index 7eea16f..a3809e7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,8 +64,9 @@ MSRV: 1.75.0. CI runs tests on stable, beta, and 1.75.0. - **Rate limiting**: `last_domain_fetch: HashMap` throttles per-domain HTTP requests. - **Authenticated feeds**: `feed_headers: HashMap>` in `App` maps feed URLs to custom HTTP headers. Built from `config.default_feeds` entries that have `headers`. Passed to `Feed::fetch_url()` at all fetch call sites. - **Compact mode**: `app.compact` bool is updated each frame by `update_compact_mode(terminal_height)`. Rendering in `ui.rs` branches on `app.compact` for layout, title bar, help bar, and dashboard item format. Controlled by `config.ui.compact_mode` (`Auto`/`Always`/`Never`). Dialog modals use `centered_rect_with_min()` to enforce minimum dimensions regardless of compact mode. -- **External-command hooks (macros + `exec_on_new`)**: Commands are run **without a shell**. Templates are tokenized once at config load via `shlex`, then `expand_argv_template` substitutes `%X` placeholders into individual argv tokens (no re-expansion), so feed content cannot break out of an argument. The macro engine queues steps into `app.pending_macro_steps` from `events.rs` and the TUI loop drains them in `tui.rs::drain_macro_steps` — drain lives at the loop level because `pipe-to` needs the terminal handle to suspend the TUI. Chains halt on the first step error (tracked via a `pre_error` guard so a stale `app.error` doesn't spuriously abort). The macro prefix (default `,`) is checked at the top of `handle_key_event` and only when `input_mode == Normal`, so text-input modes are not disturbed; an idle prefix times out via the existing success-message timeout. +- **External-command hooks (macros + `exec_on_new`)**: Commands are run **without a shell**. Templates are tokenized once at config load via `shlex`, then `expand_argv_template` substitutes `%X` placeholders (`%t %u %a %d %f %F %m %M %i %%`) into individual argv tokens (no re-expansion), so feed content cannot break out of an argument. `%m` (primary media URL), `%M` (its MIME), and `%i` (thumbnail URL) come from `` / `` / Atom `` parsed in `feed::extract_media` and routed through `FeedItem::primary_media`; they expand to `""` when absent (no fallback to `%u`). The macro engine queues steps into `app.pending_macro_steps` from `events.rs` and the TUI loop drains them in `tui.rs::drain_macro_steps` — drain lives at the loop level because `pipe-to` needs the terminal handle to suspend the TUI. Chains halt on the first step error (tracked via a `pre_error` guard so a stale `app.error` doesn't spuriously abort). The macro prefix (default `,`) is checked at the top of `handle_key_event` and only when `input_mode == Normal`, so text-input modes are not disturbed; an idle prefix times out via the existing success-message timeout. - **`exec_on_new` crash semantics**: AT-MOST-ONCE. `flush_exec_on_new` persists the `seen_items` / `feeds_seeded` sets *before* spawning any child, so a kill mid-fire loses a notification rather than re-firing on the next launch. `mark_feed_seen` only flips `feeds_seeded` on a fetch that returned items (transiently-empty first fetches don't seed), and the first observation of a feed seeds the seen-set silently to avoid a firehose. Children are spawned detached with stdio nulled; a reaper thread waits on each so they don't linger as zombies. The seen-set is pruned in `remove_current_feed` to prevent monotonic growth across feed churn. **Single-shared mark per feed**: `mark_feed_seen` is hoisted to the feed-drain call site in `tui.rs` (gated on `exec_on_new_template.is_some() || fulltext_feeds.contains(&feed.url)`) so multiple consumers (currently exec_on_new and fulltext) share one mark per feed arrival — calling it twice would double-mark and the second consumer would see an empty `newly_seen` list. +- **Inline images (Kitty protocol)**: `src/image.rs` owns an `ImageCache` (hung on `App.image_cache`) that fetches `` URLs in background threads (`MAX_CONCURRENT_FETCHES = 4`), decodes via the `image` crate, downscales to `MAX_SOURCE_DIM`, re-encodes as PNG, and transmits to the terminal via the Kitty graphics protocol in 4096-byte base64 chunks. Protocol detection (`detect_protocol`) checks `KITTY_WINDOW_ID`, `TERM=*kitty/ghostty*`, `TERM_PROGRAM=ghostty|wezterm` — but returns `None` first when `$TMUX` is set (tmux swallows APC sequences, no passthrough support); everything else is a silent no-op. **Integration shape**: the detail-view renderer (`ui::detail`) reserves a 10-row strip below the header by adding a `Constraint::Length(10)` to the vertical Layout — but **renders nothing into those cells**. Instead it stores `(url, col, row, cols, rows)` in `App.pending_image_render`, derived from the strip rect the layout solver *actually* produced (which can be squeezed below 10 rows on short terminals — the image must not spill past it). After `terminal.draw()` returns in `tui::run_app`, `emit_inline_image` drains that slot and writes the Kitty escapes directly to stdout via `io::stdout().lock()`. ratatui's diff sees the strip cells as "unchanged blank" on subsequent frames, so it emits no escapes there and the image persists; placements use a fixed placement id (`p=1`) so per-frame re-placement replaces rather than accumulates. **Modal z-order**: because the escapes land after the draw, `ui::render` nulls `pending_image_render` on any frame where a modal is visible (error, input modes, filter, link overlay, help overlay) so the image can't paint over it — regression-locked by `test_modal_suppresses_pending_image_render`. On view-change (no pending image this frame but `last_frame_had_image` is true), `emit_inline_image` writes `\x1b_Ga=d,d=a\x1b\\` (Kitty delete-all) so the image doesn't bleed into the next view. SSRF: the upfront URL is gated through `feed::is_safe_auto_url` AND the fetch uses `Feed::build_safe_redirect_client` so every redirect hop is re-validated (thumbnails are hostile feed content fetched with no user action). Body size is hard-capped at `MAX_IMAGE_BYTES = 5 MB`, and decoding runs under explicit `image::Limits` (`MAX_DECODE_DIM`, `MAX_DECODE_ALLOC`) so a small PNG can't inflate to gigabytes. The decoded cache is LRU-capped at `MAX_CACHED_IMAGES = 32` via `insert_image` (eviction also drops the matching Kitty payload). Worker panics are caught so a hostile image can't strand an `in_flight` slot. The summary-`` thumbnail fallback (`feed::extract_first_image_url`) skips declared-tiny images (width/height ≤ 2) so FeedBurner-style 1×1 beacons don't win as "the" thumbnail. - **Full-text extraction**: `feed::extract_article` fetches an article URL with the existing `reqwest::blocking::Client` and runs Mozilla Readability via the `dom_smoothie` crate. **Sync, no tokio.** Per-feed `Authorization`/auth headers are intentionally NOT forwarded to article URLs (they're third-party hosts — propagating would be a credential leak). The response body is read via `Response::take(FULLTEXT_MAX_BYTES+1).read_to_end(...)` so peak allocation is hard-capped at ~5 MB regardless of what the server sends, and the response charset is honored (`Content-Type charset=` → `` sniff → UTF-8) via `encoding_rs` so non-UTF8 pages don't mojibake. Each extraction runs on a `std::thread::spawn` worker wrapped in `catch_unwind` (so a `dom_smoothie` panic on hostile HTML surfaces as `Failed("…panicked…")` instead of stranding the slot on `Pending` forever). The TUI loop maintains a `Arc` `extract_inflight` budget capped at `EXTRACTION_MAX_INFLIGHT = 4`; queued requests beyond that budget — or whose domain was last fetched less than `refresh_rate_limit_delay` ago — are pushed back onto `pending_extraction_requests` to retry on the next loop tick. State lives only in `App::extracted: HashMap` (`Pending` / `Ready(ExtractedArticle)` / `Failed(String)`) — **in-memory only**, never persisted; LRU-capped at `EXTRACTED_CACHE_CAP = 500` with insertion-order tracking via `extracted_order: VecDeque`. The cap is **hard**: `insert_extraction` always evicts the deque head when full, and `record_extraction_result` rejects results for slots that aren't currently `Pending` (so a late worker for an evicted / removed id is dropped rather than resurrecting dead state). `Shift+F` (`KeyAction::FetchFullText`) toggles between summary and extracted text when `Ready`, queues a new request when absent, and re-queues on `Failed` (so the user can retry). Per-feed `fulltext = true` in config auto-extracts newly-seen items on refresh (same `mark_feed_seen` "newly seen" semantics as `exec_on_new` — first fetch seeds silently, no firehose); the auto path additionally filters via `feed::is_safe_auto_url` (http/https only, rejects RFC1918 / loopback / link-local / CGNAT / multicast / 6to4 / NAT64 / `localhost`-style names) to prevent a hostile feed from probing internal hosts. The auto path's worker also uses `Feed::build_safe_redirect_client`, whose `redirect::Policy::custom` re-runs `is_safe_auto_url` on every hop, so a public-looking `` that 302s into an internal target is rejected mid-chain instead of slipping past the upfront URL check. Each `ExtractionRequest` carries a `safe_redirects` flag (true for auto, false for manual `Shift+F`); the spawn loop picks the matching client per-request. Manual `Shift+F` bypasses both the URL allowlist and the safe-redirect client (it's the user's explicit action, same trust model as opening the article in a browser). The spawn loop also gates each pop on the slot still being `Pending`, so requests whose `extracted` entry was evicted by LRU or pruned by `remove_current_feed` get dropped without spawning a worker, and uses `std::thread::Builder::new().spawn()` so an OS thread-creation failure releases the inflight slot and re-queues the request instead of crashing the TUI. The detail-view lookup uses `current_article_indices()` (same resolver as the action handler) so they stay in lockstep. Extracted entries are pruned alongside `seen_items` in `remove_current_feed` via the `remove_extraction(&id)` helper. ## Commit Conventions diff --git a/Cargo.lock b/Cargo.lock index f1aa11c..b544a90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -168,6 +168,12 @@ version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" +[[package]] +name = "base64" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" + [[package]] name = "bit-set" version = "0.8.0" @@ -222,12 +228,24 @@ version = "3.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1628fb46dfa0b37568d12e5edd512553eccf6a22a78e8bde00bb4aed84d5bdbf" +[[package]] +name = "bytemuck" +version = "1.25.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec" + [[package]] name = "byteorder" version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" +[[package]] +name = "byteorder-lite" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f1fe948ff07f4bd06c30984e69f5b4899c516a3ef74f34df92a2df2ab535495" + [[package]] name = "bytes" version = "1.10.1" @@ -310,6 +328,12 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b94f61472cee1439c0b966b47e3aca9ae07e45d070759512cd390ea2bebc6675" +[[package]] +name = "color_quant" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b" + [[package]] name = "colorchoice" version = "1.0.4" @@ -578,6 +602,15 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "fdeflate" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e6853b52649d4ac5c0bd02320cddc5ba956bdb407c4b75a2c6b75bf51500f8c" +dependencies = [ + "simd-adler32", +] + [[package]] name = "feed-rs" version = "2.3.1" @@ -600,6 +633,7 @@ name = "feedr" version = "0.8.0" dependencies = [ "anyhow", + "base64 0.22.1", "chrono", "clap", "crossterm", @@ -608,6 +642,7 @@ dependencies = [ "encoding_rs", "feed-rs", "html2text", + "image", "open", "opml", "ratatui", @@ -773,6 +808,16 @@ dependencies = [ "wasi 0.14.2+wasi-0.2.4", ] +[[package]] +name = "gif" +version = "0.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee8cfcc411d9adbbaba82fb72661cc1bcca13e8bba98b364e62b2dba8f960159" +dependencies = [ + "color_quant", + "weezl", +] + [[package]] name = "gimli" version = "0.31.1" @@ -1127,6 +1172,34 @@ dependencies = [ "icu_properties", ] +[[package]] +name = "image" +version = "0.25.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85ab80394333c02fe689eaf900ab500fbd0c2213da414687ebf995a65d5a6104" +dependencies = [ + "bytemuck", + "byteorder-lite", + "color_quant", + "gif", + "image-webp", + "moxcms", + "num-traits", + "png", + "zune-core", + "zune-jpeg", +] + +[[package]] +name = "image-webp" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "525e9ff3e1a4be2fbea1fdf0e98686a6d98b4d8f937e1bf7402245af1909e8c3" +dependencies = [ + "byteorder-lite", + "quick-error", +] + [[package]] name = "indexmap" version = "2.8.0" @@ -1295,6 +1368,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e3e04debbb59698c15bacbb6d93584a8c0ca9cc3213cb423d31f760d8843ce5" dependencies = [ "adler2", + "simd-adler32", ] [[package]] @@ -1320,6 +1394,16 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "moxcms" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb85c154ba489f01b25c0d36ae69a87e4a1c73a72631fc6c0eb6dde34a73e44b" +dependencies = [ + "num-traits", + "pxfm", +] + [[package]] name = "native-tls" version = "0.2.14" @@ -1638,6 +1722,19 @@ version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" +[[package]] +name = "png" +version = "0.18.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "60769b8b31b2a9f263dae2776c37b1b28ae246943cf719eb6946a1db05128a61" +dependencies = [ + "bitflags 2.9.0", + "crc32fast", + "fdeflate", + "flate2", + "miniz_oxide", +] + [[package]] name = "ppv-lite86" version = "0.2.21" @@ -1668,6 +1765,18 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "pxfm" +version = "0.1.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0c5ccf5294c6ccd63a74f1565028353830a9c2f5eb0c682c355c471726a6e3f" + +[[package]] +name = "quick-error" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" + [[package]] name = "quick-xml" version = "0.37.4" @@ -1796,7 +1905,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd67538700a17451e7cba03ac727fb961abb7607553461627b97de0b89cf4a62" dependencies = [ "async-compression", - "base64", + "base64 0.21.7", "bytes", "encoding_rs", "futures-core", @@ -1871,7 +1980,7 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1c74cae0a4cf6ccbbf5f359f08efdf8ee7e1dc532573bf0db71968cb56b1448c" dependencies = [ - "base64", + "base64 0.21.7", ] [[package]] @@ -2091,6 +2200,12 @@ dependencies = [ "libc", ] +[[package]] +name = "simd-adler32" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "703d5c7ef118737c72f1af64ad2f6f8c5e1921f818cdcb97b8fe6fc69bf66214" + [[package]] name = "siphasher" version = "0.3.11" @@ -2670,6 +2785,12 @@ dependencies = [ "string_cache_codegen 0.6.1", ] +[[package]] +name = "weezl" +version = "0.1.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a28ac98ddc8b9274cb41bb4d9d4d5c425b6020c50c46f25559911905610b4a88" + [[package]] name = "winapi" version = "0.3.9" @@ -3179,3 +3300,18 @@ dependencies = [ "quote", "syn 2.0.100", ] + +[[package]] +name = "zune-core" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb8a0807f7c01457d0379ba880ba6322660448ddebc890ce29bb64da71fb40f9" + +[[package]] +name = "zune-jpeg" +version = "0.5.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "27bc9d5b815bc103f142aa054f561d9187d191692ec7c2d1e2b4737f8dbd7296" +dependencies = [ + "zune-core", +] diff --git a/Cargo.toml b/Cargo.toml index c188e6f..843d1c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,12 @@ shlex = "1.3" # the lockfile. dom_smoothie = "~0.17" encoding_rs = "0.8" +# Image decoding for inline rendering (Kitty graphics protocol). Default +# features disabled to avoid pulling in formats we don't surface — we only +# need to *decode* whatever the network sends and *encode* PNG to hand to +# the Kitty protocol. +image = { version = "0.25", default-features = false, features = ["png", "jpeg", "gif", "webp"] } +base64 = "0.22" [profile.release] codegen-units = 1 diff --git a/README.md b/README.md index c8e7ad7..4b7ea8b 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ Feedr is a feature-rich terminal-based RSS feed reader written in Rust. It provi - **Article Preview**: Toggle an inline preview pane in the dashboard view, or have it open at launch with `show_preview = true` - **Link Extraction**: Extract and browse all links from an article with `l` - **Full-Text Extraction**: Strip away summaries and read the actual article content inline via Mozilla Readability — manual on `Shift+F`, or auto-extract on refresh per feed with `fulltext = true` +- **Inline Images**: When an article has a `` (e.g. YouTube channel feeds, web-comic RSS), Feedr renders it inline in the detail view via the Kitty graphics protocol. Auto-detected on Ghostty, Kitty, and WezTerm; silently no-ops elsewhere. Images are fetched in the background — read or open in browser without waiting. - **Help Overlay**: Press `?` for a scrollable keybinding reference overlay - **OPML Import**: Bulk import feeds from OPML files via `feedr --import ` - **Browser Integration**: Open articles in your default browser @@ -361,8 +362,13 @@ Expanded in every `argv` token of macro and hook commands: | `%d` | Formatted publish date | | `%f` | Feed title | | `%F` | Feed URL | +| `%m` | Primary media URL (``, ``, or Atom ``) | +| `%M` | MIME type of the primary media | +| `%i` | First `` URL on the item | | `%%` | Literal `%` | +`%m`/`%M`/`%i` expand to the empty string when an item has no media — they're **orthogonal** to `%u` (no fallback). For YouTube channel feeds the watch-page URL is `%u`; the in-feed video URL is `%m`. For RSS podcasts the episode page is `%u`; the audio file is `%m`. + #### Macros A macro binds a key to an ordered chain of steps. Trigger with `` (default prefix is `,`). Steps are separated by `;`. An optional trailing ` -- "description"` overrides the help-overlay label. @@ -373,6 +379,12 @@ y = 'open-in-browser ; pipe-to "yt-dlp %u"' w = 'pipe-to "wallabag-cli add %u" -- "Save to Wallabag"' n = 'pipe-to "tee /tmp/out.txt" stdin=metadata' +# Media-rich feeds (YouTube channels, web comics, podcasts): +v = 'exec "mpv %u" -- "Play in mpv"' # YouTube: mpv resolves the watch URL via yt-dlp +d = 'exec "yt-dlp -o ~/Videos/%(title)s.%(ext)s %u" -- "Download with yt-dlp"' +p = 'exec "mpv %m" -- "Play media file"' # Podcasts: direct .mp3 URL is %m +i = 'exec "feh %i" -- "View thumbnail"' # Web comics / YT thumbnails + [macro_options] prefix = "," # the macro-prefix key pipe_default_stdin = "body" # body | title | url | metadata | none diff --git a/examples/dump_media.rs b/examples/dump_media.rs new file mode 100644 index 0000000..2b3a0a4 --- /dev/null +++ b/examples/dump_media.rs @@ -0,0 +1,48 @@ +//! Fetch a few real feeds and dump the media data feedr would surface. +//! Run with: cargo run --example dump_media + +use feedr::feed::{Feed, FeedFetchResult}; + +fn main() { + let urls = [ + "https://www.youtube.com/feeds/videos.xml?channel_id=UCsBjURrPoezykLs9EqgamOA", + "https://xkcd.com/atom.xml", + "https://www.smbc-comics.com/comic/rss", + ]; + + let client = reqwest::blocking::Client::builder() + .timeout(std::time::Duration::from_secs(15)) + .build() + .unwrap(); + + for url in urls { + let t0 = std::time::Instant::now(); + println!("\n=== {url} ==="); + match Feed::fetch_url(url, &client, None, None) { + Ok(FeedFetchResult::Feed(feed)) => { + println!("[feed] {} ({} items)", feed.title, feed.items.len()); + for item in feed.items.iter().take(3) { + println!(" - {}", item.title); + println!(" link = {:?}", item.link); + println!(" thumbnail = {:?}", item.thumbnail); + for m in &item.media { + println!( + " media = url={} kind={:?} mime={:?} {}x{} dur={:?}s size={:?}", + m.url, m.kind, m.mime, + m.width.unwrap_or(0), m.height.unwrap_or(0), + m.duration_secs, m.size_bytes, + ); + } + if let Some(primary) = item.primary_media() { + println!(" primary = {} ({:?})", primary.url, primary.kind); + } + } + } + Ok(FeedFetchResult::DiscoveredFeeds { .. }) => { + println!(" (discovered feeds, not a direct feed URL)"); + } + Err(e) => println!(" ERROR: {e}"), + } + println!(" (fetch took {:.1}s)", t0.elapsed().as_secs_f32()); + } +} diff --git a/src/app.rs b/src/app.rs index c0a8221..4782743 100644 --- a/src/app.rs +++ b/src/app.rs @@ -68,7 +68,7 @@ pub enum View { Starred, } -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct App { pub config: Config, pub feeds: Vec, @@ -195,6 +195,35 @@ pub struct App { /// Cached content width (post-border, post-padding) used to compute /// wrapped-row positions when jumping to a match. pub article_body_width_cache: usize, + /// Inline-image cache + Kitty protocol driver. Owned by `App` so the + /// detail-view renderer can call `start_fetch` from inside ratatui's + /// draw closure (it has `&mut App`). The actual escape emission lives + /// in `run_app` after `terminal.draw()` returns — see + /// `pending_image_render`. + pub image_cache: crate::image::ImageCache, + /// Set by the detail-view renderer each frame when an image is loaded + /// and ready to draw. Consumed once by `run_app` immediately after + /// `terminal.draw()` returns — the run loop emits Kitty placement + /// escapes for this rect, then clears the slot. None on every frame + /// where no image should be shown (no thumbnail, not on detail view, + /// fetch in-flight, fetch failed). + pub pending_image_render: Option, + /// True iff the previous frame *was* showing an inline image. Used by + /// `run_app` to detect view-change → emit Kitty delete-all so the + /// image doesn't bleed onto the next view's cells. + pub last_frame_had_image: bool, +} + +/// Detail-view → run_app message: "after the next `terminal.draw()` returns, +/// place image `url` inside the rect `(col, row, cols, rows)`." Coordinates +/// are 0-based terminal cells (matching ratatui's `Rect` semantics). +#[derive(Clone, Debug)] +pub struct PendingImage { + pub url: String, + pub col: u16, + pub row: u16, + pub cols: u16, + pub rows: u16, } /// A single in-article search match, indexed against the rendered body's @@ -220,6 +249,13 @@ pub struct ArticleContext<'a> { pub feed_title: &'a str, pub feed_url: &'a str, pub plain_text: Option<&'a str>, + /// Primary media URL — picked by `FeedItem::primary_media`. None if the + /// item has no media attachments. Exposed to macros as `%m`. + pub media_url: Option<&'a str>, + /// MIME type of the primary media attachment, if known. `%M`. + pub media_mime: Option<&'a str>, + /// First `` URI on the item, if any. `%i`. + pub thumbnail_url: Option<&'a str>, } #[derive(Clone, Debug)] @@ -507,6 +543,9 @@ impl App { article_search_matches: Vec::new(), article_body_cache: String::new(), article_body_width_cache: 0, + image_cache: crate::image::ImageCache::new(), + pending_image_render: None, + last_frame_had_image: false, }; app.update_dashboard(); @@ -1904,6 +1943,7 @@ impl App { let (feed_idx, item_idx) = self.current_article_indices()?; let feed = self.feeds.get(feed_idx)?; let item = feed.items.get(item_idx)?; + let primary = item.primary_media(); Some(ArticleContext { title: &item.title, url: item.link.as_deref(), @@ -1912,6 +1952,9 @@ impl App { feed_title: &feed.title, feed_url: &feed.url, plain_text: item.plain_text.as_deref(), + media_url: primary.map(|m| m.url.as_str()), + media_mime: primary.and_then(|m| m.mime.as_deref()), + thumbnail_url: item.thumbnail.as_deref(), }) } @@ -2325,6 +2368,7 @@ pub fn wrapped_row_of_line(body: &str, target_line_idx: usize, width: usize) -> /// the path used by `exec_on_new`, where the new item may not yet live /// inside `App::feeds`. pub fn article_context_from<'a>(feed: &'a Feed, item: &'a FeedItem) -> ArticleContext<'a> { + let primary = item.primary_media(); ArticleContext { title: &item.title, url: item.link.as_deref(), @@ -2333,6 +2377,9 @@ pub fn article_context_from<'a>(feed: &'a Feed, item: &'a FeedItem) -> ArticleCo feed_title: &feed.title, feed_url: &feed.url, plain_text: item.plain_text.as_deref(), + media_url: primary.map(|m| m.url.as_str()), + media_mime: primary.and_then(|m| m.mime.as_deref()), + thumbnail_url: item.thumbnail.as_deref(), } } @@ -2376,7 +2423,14 @@ pub fn mark_feed_seen(app: &mut App, feed: &Feed) -> Vec { /// verbatim into individual argv elements. /// /// Variables: %t title, %u url, %a author, %d formatted-date, -/// %f feed-title, %F feed-url, %% literal %. +/// %f feed-title, %F feed-url, +/// %m primary-media-url, %M primary-media-mime, %i thumbnail-url, +/// %% literal %. +/// +/// Placeholders that resolve to no value (e.g. `%m` on a plain text item) +/// expand to the empty string, matching how `%u` behaves on items missing +/// `link`. `%m` and `%u` are intentionally orthogonal — no automatic +/// fallback — so users can compose them explicitly in their macros. pub fn expand_argv_template(template: &[String], ctx: &ArticleContext<'_>) -> Vec { template .iter() @@ -2400,6 +2454,9 @@ fn expand_one_token(tok: &str, ctx: &ArticleContext<'_>) -> String { Some('d') => out.push_str(ctx.formatted_date.unwrap_or("")), Some('f') => out.push_str(ctx.feed_title), Some('F') => out.push_str(ctx.feed_url), + Some('m') => out.push_str(ctx.media_url.unwrap_or("")), + Some('M') => out.push_str(ctx.media_mime.unwrap_or("")), + Some('i') => out.push_str(ctx.thumbnail_url.unwrap_or("")), Some(other) => { out.push('%'); out.push(other); @@ -2496,6 +2553,8 @@ mod tests { formatted_date: None, parsed_date: Some(Utc::now() - chrono::Duration::days(30)), plain_text: Some("Old content".to_string()), + media: Vec::new(), + thumbnail: None, }, FeedItem { title: "New Article".to_string(), @@ -2507,6 +2566,8 @@ mod tests { formatted_date: None, parsed_date: Some(Utc::now() - chrono::Duration::hours(1)), plain_text: Some("New content".to_string()), + media: Vec::new(), + thumbnail: None, }, ], }, @@ -2524,6 +2585,8 @@ mod tests { formatted_date: None, parsed_date: Some(Utc::now() - chrono::Duration::hours(2)), plain_text: Some("Another new content".to_string()), + media: Vec::new(), + thumbnail: None, }], }, ]; @@ -2605,6 +2668,8 @@ mod tests { formatted_date: None, parsed_date: Some(Utc::now() - chrono::Duration::hours(3)), plain_text: Some("Alpha content".to_string()), + media: Vec::new(), + thumbnail: None, }], }); app.last_session_time = Some(Utc::now() - chrono::Duration::days(365)); @@ -2959,6 +3024,24 @@ mod tests { feed_title: "Example", feed_url: "https://ex.com/feed.xml", plain_text: Some("body text"), + media_url: Some("https://ex.com/v.mp4"), + media_mime: Some("video/mp4"), + thumbnail_url: Some("https://ex.com/thumb.jpg"), + } + } + + fn synthetic_ctx_no_media() -> ArticleContext<'static> { + ArticleContext { + title: "Plain", + url: Some("https://ex.com/p"), + author: None, + formatted_date: None, + feed_title: "Example", + feed_url: "https://ex.com/feed.xml", + plain_text: None, + media_url: None, + media_mime: None, + thumbnail_url: None, } } @@ -2987,6 +3070,36 @@ mod tests { assert_eq!(argv[1], "x%zy"); } + #[test] + fn test_expand_media_placeholders() { + let tmpl = vec![ + "%m".to_string(), + "--mime=%M".to_string(), + "--thumb=%i".to_string(), + ]; + let argv = expand_argv_template(&tmpl, &synthetic_ctx()); + assert_eq!(argv[0], "https://ex.com/v.mp4"); + assert_eq!(argv[1], "--mime=video/mp4"); + assert_eq!(argv[2], "--thumb=https://ex.com/thumb.jpg"); + } + + #[test] + fn test_expand_media_placeholders_empty_when_absent() { + // `%m` / `%M` / `%i` on an item without media expand to "", not to + // the page URL — placeholders are orthogonal to `%u`. + let tmpl = vec![ + "%m".to_string(), + "%M".to_string(), + "%i".to_string(), + "%u".to_string(), + ]; + let argv = expand_argv_template(&tmpl, &synthetic_ctx_no_media()); + assert_eq!(argv[0], ""); + assert_eq!(argv[1], ""); + assert_eq!(argv[2], ""); + assert_eq!(argv[3], "https://ex.com/p"); + } + #[test] fn test_make_pipe_payload_kinds() { use crate::keybindings::StdinKind; @@ -3017,6 +3130,8 @@ mod tests { parsed_date: None, plain_text: None, title_lower: t.to_lowercase(), + media: Vec::new(), + thumbnail: None, }) .collect(); Feed { @@ -3158,6 +3273,8 @@ mod tests { parsed_date: None, plain_text: None, title_lower: "t".to_string(), + media: Vec::new(), + thumbnail: None, }; feed.items.push(item.clone()); assert_eq!(make_item_id(&feed, &item), "https://ex.com/a"); diff --git a/src/events.rs b/src/events.rs index 97cbca4..e0158b9 100644 --- a/src/events.rs +++ b/src/events.rs @@ -1524,6 +1524,8 @@ mod tests { formatted_date: None, parsed_date: Some(Utc::now() - chrono::Duration::days(30)), plain_text: Some("Old content".to_string()), + media: Vec::new(), + thumbnail: None, }, FeedItem { title: "New Article".to_string(), @@ -1535,6 +1537,8 @@ mod tests { formatted_date: None, parsed_date: Some(Utc::now() - chrono::Duration::hours(1)), plain_text: Some("New content".to_string()), + media: Vec::new(), + thumbnail: None, }, ], }, @@ -1552,6 +1556,8 @@ mod tests { formatted_date: None, parsed_date: Some(Utc::now() - chrono::Duration::hours(2)), plain_text: Some("Another new content".to_string()), + media: Vec::new(), + thumbnail: None, }], }, ]; diff --git a/src/feed.rs b/src/feed.rs index f2cab5a..dc2c50d 100644 --- a/src/feed.rs +++ b/src/feed.rs @@ -38,6 +38,48 @@ pub struct Feed { pub title_lower: String, } +/// Coarse classification of a media attachment, derived from its MIME type's +/// top-level type. Used by `FeedItem::primary_media` to prefer playable media +/// (audio/video) over decorative (image) when picking the item's primary. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub enum MediaKind { + Audio, + Video, + Image, + Unknown, +} + +impl MediaKind { + fn from_mime(mime: Option<&str>) -> Self { + match mime { + Some(m) if m.starts_with("audio/") => Self::Audio, + Some(m) if m.starts_with("video/") => Self::Video, + Some(m) if m.starts_with("image/") => Self::Image, + _ => Self::Unknown, + } + } +} + +/// A single playable / viewable media URL attached to a feed item. Sourced +/// from RSS Media (``), RSS 2 enclosures (``), or +/// Atom out-of-line content (``). Surfaced to macro +/// placeholders via `%m` / `%M`. +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct MediaAttachment { + pub url: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub mime: Option, + pub kind: MediaKind, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub width: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub height: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub duration_secs: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub size_bytes: Option, +} + #[derive(Clone, Debug, Serialize, Deserialize)] pub struct FeedItem { pub title: String, @@ -52,6 +94,28 @@ pub struct FeedItem { pub plain_text: Option, #[serde(skip)] pub title_lower: String, + /// Media attachments parsed from ``, ``, and + /// Atom out-of-line ``. Empty for ordinary text feeds. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub media: Vec, + /// First `` URI on the entry, if any. Independent of + /// `media` — a YouTube entry typically has both a video attachment and + /// a thumbnail. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub thumbnail: Option, +} + +impl FeedItem { + /// Pick the most "playable" media attachment for placeholder expansion. + /// Audio/video win over images; images win over Unknown. Within a tier + /// the first (feed-order) attachment wins. + pub fn primary_media(&self) -> Option<&MediaAttachment> { + self.media + .iter() + .find(|m| matches!(m.kind, MediaKind::Audio | MediaKind::Video)) + .or_else(|| self.media.iter().find(|m| m.kind == MediaKind::Image)) + .or_else(|| self.media.first()) + } } #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] @@ -750,14 +814,25 @@ impl FeedItem { .map(|summary| summary.content.clone()) }; - // Cache plain text from description (avoids repeated HTML parsing) - let plain_text = description - .as_ref() - .map(|desc| html2text::from_read(desc.as_bytes(), 80)); + // Cache plain text from description (avoids repeated HTML parsing). + // Table structure is flattened first — see `flatten_description_html` + // for why (Reddit RSS in particular wraps metadata in a 2-column + // table that html2text would otherwise render with `|` separators). + let plain_text = description.as_ref().map(|desc| { + let prepped = flatten_description_html(desc); + html2text::from_read(prepped.as_bytes(), 80) + }); // Extract the primary link let link = entry.links.first().map(|link| link.href.clone()); + // Extract media attachments and thumbnails. Three sources, in priority + // order (first occurrence of a given URL wins after dedup): + // 1. from RSS Media (`entry.media[*].content[*]`) + // 2. from RSS 2 (links with `rel == Some("enclosure")`) + // 3. Atom out-of-line `` + let (media, thumbnail) = extract_media(entry); + let title = entry .title .as_ref() @@ -775,10 +850,208 @@ impl FeedItem { parsed_date, plain_text, title_lower, + media, + thumbnail, } } } +/// Walk a `feed_rs::model::Entry` and collect media attachments + a single +/// representative thumbnail. See `FeedItem::from_feed_entry` for source +/// priority. URL deduplication preserves first occurrence. +fn extract_media(entry: &feed_rs::model::Entry) -> (Vec, Option) { + let mut media: Vec = Vec::new(); + let mut seen_urls: HashSet = HashSet::new(); + let push = |att: MediaAttachment, seen: &mut HashSet, media: &mut Vec<_>| { + if seen.insert(att.url.clone()) { + media.push(att); + } + }; + + // 1. via media:group or default item-level grouping. + for obj in &entry.media { + for content in &obj.content { + let Some(url) = content.url.as_ref() else { + continue; + }; + let mime = content + .content_type + .as_ref() + .map(|t| t.essence().to_string()); + let att = MediaAttachment { + url: url.to_string(), + kind: MediaKind::from_mime(mime.as_deref()), + mime, + width: content.width, + height: content.height, + duration_secs: content.duration.map(|d| d.as_secs()), + size_bytes: content.size, + }; + push(att, &mut seen_urls, &mut media); + } + } + + // 2. RSS 2 : feed-rs lands these in entry.links with rel="enclosure". + for link in &entry.links { + if link.rel.as_deref() != Some("enclosure") { + continue; + } + let mime = link.media_type.clone(); + let att = MediaAttachment { + url: link.href.clone(), + kind: MediaKind::from_mime(mime.as_deref()), + mime, + width: None, + height: None, + duration_secs: None, + size_bytes: link.length, + }; + push(att, &mut seen_urls, &mut media); + } + + // 3. Atom out-of-line . The content's media type is on the + // parent `Content`, not on the inner `Link`. + if let Some(content) = entry.content.as_ref() { + if let Some(src) = content.src.as_ref() { + let mime = Some(content.content_type.essence().to_string()); + let att = MediaAttachment { + url: src.href.clone(), + kind: MediaKind::from_mime(mime.as_deref()), + mime, + width: None, + height: None, + duration_secs: None, + size_bytes: content.length, + }; + push(att, &mut seen_urls, &mut media); + } + } + + // Thumbnail priority: + // 1. on the entry (Reddit, YouTube, etc.) + // 2. First in the entry's content / summary HTML + // (xkcd, SMBC, Penny Arcade — comic feeds that put their single + // image inside the summary instead of a media namespace). + let thumbnail = entry + .media + .iter() + .flat_map(|obj| obj.thumbnails.iter()) + .map(|t| t.image.uri.clone()) + .find(|uri| !uri.is_empty()) + .or_else(|| { + // Fall back to scraping the description for an . We try + // `content` first (which `from_feed_entry` already prefers over + // `summary`), then `summary`. Resolution uses the first entry + // link as base so `src="/comics/foo.png"` becomes absolute. + let base = entry.links.first().map(|l| l.href.as_str()); + let html_sources = [ + entry.content.as_ref().and_then(|c| c.body.as_deref()), + entry.summary.as_ref().map(|s| s.content.as_str()), + ]; + html_sources + .into_iter() + .flatten() + .find_map(|html| extract_first_image_url(html, base)) + }); + + (media, thumbnail) +} + +/// Find the first `` URL in an HTML fragment. Relative URLs +/// are resolved against `base_url` if provided. Returns None on parse +/// failure, missing `src`, empty `src`, or `data:` / `javascript:` URIs. +fn extract_first_image_url(html: &str, base_url: Option<&str>) -> Option { + let doc = Html::parse_fragment(html); + let selector = Selector::parse("img[src]").ok()?; + let base = base_url.and_then(|u| Url::parse(u).ok()); + for el in doc.select(&selector) { + let src = el.value().attr("src")?.trim(); + if src.is_empty() { + continue; + } + // Skip tracking pixels / spacers: FeedBurner-style feeds lead with + // a 1x1 beacon that would otherwise win as "the" thumbnail. Only + // declared-tiny images are skipped — missing attributes pass. + let is_tiny = |attr: &str| { + el.value() + .attr(attr) + .and_then(|v| v.trim().trim_end_matches("px").parse::().ok()) + .is_some_and(|n| n <= 2) + }; + if is_tiny("width") || is_tiny("height") { + continue; + } + // Reject inline-data and unsupported schemes outright — we'd just + // fail to fetch them and burn a slot. + let lower = src.to_ascii_lowercase(); + if lower.starts_with("data:") || lower.starts_with("javascript:") { + continue; + } + let resolved = if let Some(b) = &base { + b.join(src) + .map(|u| u.to_string()) + .unwrap_or_else(|_| src.to_string()) + } else { + src.to_string() + }; + // Only http(s); ignore protocol-less or file:// URIs. + if resolved.starts_with("http://") || resolved.starts_with("https://") { + return Some(resolved); + } + } + None +} + +/// Pre-process feed-description HTML to flatten `` structures into +/// inline text before handing it to `html2text`. Reddit RSS posts (and many +/// other feeds) wrap title/author/link metadata in a 2-column HTML table; +/// html2text faithfully renders that as box-drawn columns with `|` +/// separators, which then mangles badly when the downstream +/// `format_content_for_reading` paragraph-joins the rows. Cell boundaries +/// become spaces, row ends emit `

` (a paragraph break — needed so +/// the blank line survives the line-joining pass), and the table/tbody/ +/// thead/tfoot wrappers are dropped. Tag matching is case-insensitive and +/// tolerates attributes. Non-table markup is left intact. +pub(crate) fn flatten_description_html(html: &str) -> String { + let mut out = String::with_capacity(html.len()); + let mut rest = html; + while !rest.is_empty() { + let Some(lt_idx) = rest.find('<') else { + out.push_str(rest); + break; + }; + out.push_str(&rest[..lt_idx]); + let after_lt = &rest[lt_idx..]; + let Some(gt_rel) = after_lt.find('>') else { + // Unterminated tag — emit the rest verbatim and stop. + out.push_str(after_lt); + break; + }; + let tag = &after_lt[..=gt_rel]; + let body = &tag[1..tag.len() - 1]; + let body_after_slash = body.strip_prefix('/').unwrap_or(body); + let name_end = body_after_slash + .find(|c: char| c.is_ascii_whitespace() || c == '/' || c == '>') + .unwrap_or(body_after_slash.len()); + let name_lower = body_after_slash[..name_end].to_ascii_lowercase(); + match name_lower.as_str() { + "table" | "tbody" | "thead" | "tfoot" | "colgroup" | "col" | "caption" => { + // Drop the wrapper — keep inner content inline. + } + "tr" => { + // Close any preceding row content with a paragraph break. + // We emit on both `` and ``; doubled `
`s + // collapse into a single paragraph break in html2text. + out.push_str("

"); + } + "td" | "th" => out.push(' '), + _ => out.push_str(tag), + } + rest = &after_lt[gt_rel + 1..]; + } + out +} + fn format_date(dt: DateTime) -> String { // Calculate how long ago the item was published let now = Utc::now(); @@ -800,6 +1073,74 @@ fn format_date(dt: DateTime) -> String { mod tests { use super::*; + #[test] + fn flatten_strips_reddit_style_table() { + // Reddit RSS wraps each post in a 2-column table; html2text would + // otherwise render that with `|` column separators that mangle on + // re-wrap. After flattening: cells joined by spaces, row ends are + // paragraph breaks, table wrappers gone, inner anchors intact. + let html = r#"
submitted by /u/poorzack
[link] [comments]
"#; + let out = flatten_description_html(html); + assert!(!out.contains(""#)); + assert!(out.contains("/u/poorzack")); + // Row ends emit a paragraph break. + assert!(out.contains("

")); + } + + #[test] + fn flatten_is_case_insensitive_and_tolerates_attributes() { + let html = + r#"
onetwo
"#; + let out = flatten_description_html(html); + assert!(!out.to_ascii_lowercase().contains("Hello link world.

  • one
  • two
"#; + let out = flatten_description_html(html); + // Nothing in this string is table markup, so byte-equal. + assert_eq!(out, html); + } + + #[test] + fn flatten_handles_unterminated_tag() { + // Don't panic on malformed HTML; just emit it verbatim from the + // unterminated `<` onward. + let html = "before
imgTitle here [link]
"#; + let prepped = flatten_description_html(html); + let text = html2text::from_read(prepped.as_bytes(), 80); + assert!( + !text.contains('|'), + "expected no column separators in flattened output, got:\n{text}" + ); + assert!(text.contains("Title here")); + } + #[test] fn test_discover_single_rss_feed() { let html = br#" @@ -1417,4 +1758,297 @@ mod tests { request ); } + + // ── media extraction ───────────────────────────────────────────────────── + + fn parse_first_entry(xml: &str) -> feed_rs::model::Entry { + let feed = feed_rs::parser::parse(xml.as_bytes()).expect("parse feed"); + feed.entries.into_iter().next().expect("at least one entry") + } + + #[test] + fn media_extract_youtube_atom_shape() { + // YouTube's channel feeds wrap the video in with a + // and a . + let xml = r#" + + yt:channel:UCexample + Example Channel + + yt:video:abc123 + Example Video + + 2025-01-01T00:00:00+00:00 + + Example Video + + + Some description. + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert_eq!(item.media.len(), 1, "expected exactly one media attachment"); + assert_eq!(item.media[0].url, "https://www.youtube.com/v/abc123"); + assert_eq!( + item.media[0].mime.as_deref(), + Some("application/x-shockwave-flash") + ); + assert_eq!(item.media[0].kind, MediaKind::Unknown); + assert_eq!(item.media[0].width, Some(640)); + assert_eq!(item.media[0].height, Some(390)); + assert_eq!( + item.thumbnail.as_deref(), + Some("https://i.ytimg.com/vi/abc123/hqdefault.jpg") + ); + } + + #[test] + fn media_extract_rss_enclosure() { + // RSS 2 podcast: lands in entry.links with rel="enclosure". + let xml = r#" + + + Example Podcast + https://podcast.example.com/ + A podcast. + + Episode 1 + https://podcast.example.com/ep1 + https://podcast.example.com/ep1 + + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert_eq!(item.media.len(), 1); + assert_eq!(item.media[0].url, "https://podcast.example.com/ep1.mp3"); + assert_eq!(item.media[0].kind, MediaKind::Audio); + assert_eq!(item.media[0].mime.as_deref(), Some("audio/mpeg")); + assert_eq!(item.media[0].size_bytes, Some(12_345_678)); + assert!(item.thumbnail.is_none()); + } + + #[test] + fn media_extract_atom_content_src() { + // Atom out-of-line . + let xml = r#" + + tag:example.com,2025:feed + Example + + tag:example.com,2025:1 + A Video + + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert_eq!(item.media.len(), 1); + assert_eq!(item.media[0].url, "https://cdn.example.com/clip.mp4"); + assert_eq!(item.media[0].kind, MediaKind::Video); + assert_eq!(item.media[0].mime.as_deref(), Some("video/mp4")); + } + + #[test] + fn media_dedup_across_sources() { + // Same URL in both and should produce one + // attachment, keeping the Media RSS metadata (which lands first). + let xml = r#" + + + Test + https://example.com/ + x + + Dup + https://example.com/ep1 + https://example.com/ep1 + + + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert_eq!(item.media.len(), 1, "duplicate URL should collapse"); + // First-source wins: the media:content MIME, not the enclosure MIME. + assert_eq!(item.media[0].mime.as_deref(), Some("audio/mpeg")); + } + + #[test] + fn media_primary_picks_audio_video_over_image() { + let item = FeedItem { + title: "t".into(), + link: None, + description: None, + pub_date: None, + author: None, + formatted_date: None, + parsed_date: None, + plain_text: None, + title_lower: "t".into(), + media: vec![ + MediaAttachment { + url: "https://x/thumb.jpg".into(), + mime: Some("image/jpeg".into()), + kind: MediaKind::Image, + width: None, + height: None, + duration_secs: None, + size_bytes: None, + }, + MediaAttachment { + url: "https://x/clip.mp4".into(), + mime: Some("video/mp4".into()), + kind: MediaKind::Video, + width: None, + height: None, + duration_secs: None, + size_bytes: None, + }, + ], + thumbnail: None, + }; + let primary = item.primary_media().expect("has primary"); + assert_eq!(primary.kind, MediaKind::Video); + } + + #[test] + fn thumbnail_falls_back_to_img_in_summary() { + // xkcd-style: inside . + let xml = r#" + + https://xkcd.com/ + xkcd + + https://xkcd.com/3246/ + Speedrun + + <img src="https://imgs.xkcd.com/comics/speedrun.png" alt="..."/> + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert_eq!( + item.thumbnail.as_deref(), + Some("https://imgs.xkcd.com/comics/speedrun.png") + ); + // The media list itself stays empty — no or . + assert!(item.media.is_empty()); + } + + #[test] + fn thumbnail_prefers_media_thumbnail_over_summary_img() { + // When both are present, the explicit wins. + let xml = r#" + + x + x + + x1 + x + + <img src="https://example.com/in-summary.png"/> + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert_eq!( + item.thumbnail.as_deref(), + Some("https://example.com/explicit-thumb.jpg") + ); + } + + #[test] + fn thumbnail_resolves_relative_img_against_link() { + let xml = r#" + + + Comics + https://comics.example.com/ + x + + Strip 1 + https://comics.example.com/strip/1 + https://comics.example.com/strip/1 + <img src="/img/strip-1.png"/> today's strip + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert_eq!( + item.thumbnail.as_deref(), + Some("https://comics.example.com/img/strip-1.png") + ); + } + + #[test] + fn thumbnail_skips_data_uri_imgs() { + // A leading data: URI must be skipped; the second wins. + let xml = r#" + + + x + https://example.com/ + x + + x + https://example.com/p1 + https://example.com/p1 + <img src="data:image/png;base64,abc"/><img src="https://example.com/real.png"/> + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert_eq!( + item.thumbnail.as_deref(), + Some("https://example.com/real.png") + ); + } + + #[test] + fn thumbnail_skips_tracking_pixels() { + // A leading 1x1 beacon must be skipped; the real image wins. An + // image with no width/height attributes is NOT treated as tiny. + let html = r#""#; + assert_eq!( + extract_first_image_url(html, None).as_deref(), + Some("https://example.com/real.png") + ); + let html_px = r#""#; + assert_eq!( + extract_first_image_url(html_px, None).as_deref(), + Some("https://example.com/comic.png") + ); + } + + #[test] + fn media_empty_for_plain_text_feed() { + // A plain RSS item with no enclosure / media should produce no media. + let xml = r#" + + + Blog + https://blog.example.com/ + x + + Post + https://blog.example.com/p1 + https://blog.example.com/p1 + Plain text post. + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + assert!(item.media.is_empty()); + assert!(item.thumbnail.is_none()); + } } diff --git a/src/image.rs b/src/image.rs new file mode 100644 index 0000000..a22e550 --- /dev/null +++ b/src/image.rs @@ -0,0 +1,659 @@ +//! Inline image rendering via the Kitty graphics protocol. +//! +//! Scope (MVP): a single image per article, placed in a reserved strip of +//! the detail view. Background fetch via a worker thread pool. Decoded into +//! `image::DynamicImage`, downscaled, re-encoded as PNG, base64-encoded, +//! and transmitted to the terminal in 4096-byte chunks per the Kitty spec. +//! +//! Non-goals (deliberate): +//! * iTerm2 / Sixel / Terminology / half-block fallbacks. In terminals +//! without Kitty support the image area silently stays blank. +//! * Multiple inline images per article. The article's `thumbnail` field +//! is the single source surfaced. +//! * tmux DCS passthrough. Skip until a user actually needs it. +//! * Cell-pixel-size ioctl. A fixed aspect approximation gives perfectly +//! usable thumbnails; precision can come later. +//! +//! Security: HTTP fetches reuse the existing feedr blocking client and +//! gate URLs through `feed::is_safe_auto_url` (SSRF allow-list). The body +//! is capped at `MAX_IMAGE_BYTES`. A worker panic is caught so a hostile +//! image cannot strand the fetch slot in-flight. + +use base64::Engine; +use image::{DynamicImage, GenericImageView}; +use std::collections::{HashMap, HashSet, VecDeque}; +use std::io::{self, Write}; +use std::sync::{mpsc, Arc}; + +/// Concurrency cap for background fetches. Detail-view ever needs one +/// image at a time, but a quick navigate-spam from the user can queue +/// multiple — bound it so a hostile feed can't fan-out a connection burst. +const MAX_CONCURRENT_FETCHES: usize = 4; + +/// Hard cap on the body size of an image fetch. Anything larger is treated +/// as a fetch failure (`None`) rather than buffered into memory. 5 MB +/// matches the FULLTEXT_MAX_BYTES used by `feed::extract_article`. +const MAX_IMAGE_BYTES: u64 = 5 * 1024 * 1024; + +/// Largest source dimension (px) we keep after decoding. Larger inputs are +/// Lanczos-downscaled before re-encoding to PNG, so transmission stays +/// fast and the terminal isn't asked to scale a 4K image into 30 cells. +const MAX_SOURCE_DIM: u32 = 1024; + +/// Decoder guards against decompression bombs: a 5 MB PNG can legally +/// inflate to gigabytes of pixels. Anything that would decode past these +/// is treated as a fetch failure. +const MAX_DECODE_DIM: u32 = 8192; +const MAX_DECODE_ALLOC: u64 = 128 * 1024 * 1024; + +/// LRU cap on decoded images. Each slot can hold up to +/// `MAX_SOURCE_DIM`² RGBA (~4 MB), so the cache is bounded at ~128 MB +/// worst-case instead of growing for the life of the process. Same +/// insertion-order eviction pattern as `App::extracted`. +const MAX_CACHED_IMAGES: usize = 32; + +/// Heuristic ratio of (cell width / cell height) in pixels. Most modern +/// fixed-pitch fonts hit ~0.5 — cells are roughly twice as tall as wide. +/// Used to compute how many *columns* an image of a given pixel aspect +/// should occupy when constrained to a target row count. +const CELL_ASPECT: f32 = 0.5; + +/// Which inline-image protocol the current terminal supports. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ImageProtocol { + Kitty, + None, +} + +/// Detect the terminal's inline-image capability from env. We treat any +/// terminal that *speaks* Kitty's graphics protocol as Kitty, including +/// Ghostty (which implements the protocol natively). +pub fn detect_protocol() -> ImageProtocol { + // tmux inherits KITTY_WINDOW_ID / TERM_PROGRAM from the outer terminal + // but swallows raw APC sequences (no passthrough support here yet), so + // the image would never appear while the strip still gets reserved. + if std::env::var_os("TMUX").is_some() { + return ImageProtocol::None; + } + // Kitty itself. + if std::env::var_os("KITTY_WINDOW_ID").is_some() { + return ImageProtocol::Kitty; + } + if let Ok(term) = std::env::var("TERM") { + let term = term.to_lowercase(); + if term.contains("kitty") || term.contains("ghostty") { + return ImageProtocol::Kitty; + } + } + if let Ok(prog) = std::env::var("TERM_PROGRAM") { + let prog = prog.to_lowercase(); + if prog == "ghostty" || prog == "wezterm" { + return ImageProtocol::Kitty; + } + } + ImageProtocol::None +} + +/// A PNG blob ready for Kitty transmission, with the dimensions needed to +/// position the placement. +#[derive(Clone, Debug)] +struct KittyImage { + /// 24-bit Kitty image id. Reserved between transmit and place. + id: u32, + /// Pre-encoded PNG bytes, transmitted once on first render and dropped. + pending_png: Option>>, + /// Whether the PNG has been transmitted to the terminal. Once true, + /// subsequent renders just emit placement escapes (cheap). + transmitted: bool, + /// Source-image aspect (width / height) — needed to size the placement + /// in cells against the available rect. + src_aspect: f32, +} + +/// Per-process inline-image cache + Kitty protocol driver. +/// +/// State machine for a given URL: +/// absent → in_flight (worker queued) → images: Some(img) (decoded) +/// → kitty_images: Some(KittyImage) (PNG-ready) +/// → on first render: transmitted=true (PNG sent to terminal) +/// +/// A failed fetch lands as `images: None` (insert-known-failed); subsequent +/// `start_fetch` calls for that URL no-op. +/// +/// Debug is implemented manually because the internal `mpsc::Receiver` +/// has no `Debug` impl — we surface counts instead of contents. +pub struct ImageCache { + protocol: ImageProtocol, + /// Decoded images. `None` slot = fetch attempted and failed. + images: HashMap>>, + /// Insertion order for `images`, oldest first. Drives LRU eviction at + /// `MAX_CACHED_IMAGES` — see `insert_image`. + images_order: VecDeque, + /// PNG-ready Kitty payloads. Only populated when `protocol == Kitty`. + kitty_images: HashMap>, + /// Monotonic Kitty image id allocator. Skips 0 (reserved by spec). + next_kitty_id: u32, + /// Background fetch channel + bookkeeping. + sender: mpsc::Sender<(String, Option)>, + receiver: mpsc::Receiver<(String, Option)>, + in_flight: HashSet, + /// Lazily-built HTTP client. Kept here so the detail-view renderer can + /// trigger fetches via `&mut App` without the caller threading a client + /// through every UI layer. Construction is cheap relative to a fetch. + client: Option, +} + +impl Default for ImageCache { + fn default() -> Self { + Self::new() + } +} + +impl std::fmt::Debug for ImageCache { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ImageCache") + .field("protocol", &self.protocol) + .field("images", &self.images.len()) + .field("kitty_images", &self.kitty_images.len()) + .field("in_flight", &self.in_flight.len()) + .finish() + } +} + +impl ImageCache { + pub fn new() -> Self { + let (sender, receiver) = mpsc::channel(); + Self { + protocol: detect_protocol(), + images: HashMap::new(), + images_order: VecDeque::new(), + kitty_images: HashMap::new(), + next_kitty_id: 0, + sender, + receiver, + in_flight: HashSet::new(), + client: None, + } + } + + pub fn protocol(&self) -> ImageProtocol { + self.protocol + } + + /// Test-only override: protocol detection is env-driven, so tests that + /// exercise the render path force Kitty explicitly. + #[cfg(test)] + pub(crate) fn force_protocol(&mut self, protocol: ImageProtocol) { + self.protocol = protocol; + } + + /// True iff a decoded image is ready for this URL (independent of + /// whether the Kitty PNG has been transmitted yet — that's lazy). + pub fn has_image(&self, url: &str) -> bool { + self.images.get(url).is_some_and(|o| o.is_some()) + } + + /// Record a fetch result (decoded image or known failure), evicting the + /// oldest entry past `MAX_CACHED_IMAGES`. Eviction also drops the + /// matching Kitty payload so stale PNG state can't outlive its image; + /// an evicted-but-displayed URL simply re-fetches on the next render. + pub(crate) fn insert_image(&mut self, url: String, img: Option>) { + if self.images.insert(url.clone(), img).is_none() { + self.images_order.push_back(url); + } + while self.images_order.len() > MAX_CACHED_IMAGES { + if let Some(oldest) = self.images_order.pop_front() { + self.images.remove(&oldest); + self.kitty_images.remove(&oldest); + } + } + } + + /// Queue a background fetch+decode for `url`. Returns false if the + /// concurrency cap is full — caller can re-queue on the next tick. + /// Idempotent: re-queueing an in-flight or already-attempted URL is a + /// no-op (returns true, the "already handled" signal). + pub fn start_fetch(&mut self, url: &str) -> bool { + if self.protocol == ImageProtocol::None { + return true; // no point fetching if we can't render + } + if self.images.contains_key(url) || self.in_flight.contains(url) { + return true; + } + if self.in_flight.len() >= MAX_CONCURRENT_FETCHES { + return false; + } + if !crate::feed::is_safe_auto_url(url) { + // Treat blocked hosts as a known failure so we don't re-queue. + self.insert_image(url.to_string(), None); + return true; + } + // Lazily build the client. Failure here is treated as a known-failed + // fetch so we don't re-queue on every render tick. The safe-redirect + // client re-runs `is_safe_auto_url` on every hop — thumbnails come + // from hostile feed content and are fetched with no user action, so + // a public-looking URL must not be allowed to 302 into the user's + // internal network (same threat model as the auto-fulltext path). + if self.client.is_none() { + match crate::feed::Feed::build_safe_redirect_client(15) { + Ok(c) => self.client = Some(c), + Err(_) => { + self.insert_image(url.to_string(), None); + return true; + } + } + } + let client = self.client.as_ref().unwrap().clone(); + self.in_flight.insert(url.to_string()); + let sender = self.sender.clone(); + let url_owned = url.to_string(); + std::thread::spawn(move || { + // Panic-safe: a panic in the image decoder must still free the + // in-flight slot via `poll_completed`. Without `catch_unwind`, + // a single hostile image could permanently strand the slot. + let img = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + fetch_and_decode(&url_owned, &client) + })) + .unwrap_or(None); + let _ = sender.send((url_owned, img)); + }); + true + } + + /// Drain completed background fetches. Returns true if any new images + /// arrived (so the caller can request a redraw). + pub fn poll_completed(&mut self) -> bool { + let mut any = false; + while let Ok((url, img)) = self.receiver.try_recv() { + self.in_flight.remove(&url); + self.insert_image(url, img.map(Arc::new)); + any = true; + } + any + } + + /// Compute the (cols, rows) the image should occupy when constrained + /// to `(max_cols, max_rows)` cells, preserving the image's aspect. + /// Returns None if the image isn't loaded yet. + pub fn display_cells(&self, url: &str, max_cols: usize, max_rows: usize) -> Option<(u16, u16)> { + let img = self.images.get(url)?.as_ref()?; + let (w, h) = img.dimensions(); + Some(fit_cells(w, h, max_cols, max_rows)) + } + + /// Pre-encode the PNG for `url` if needed, ready to transmit. Called + /// from `render_at` on first use of an image. + fn ensure_kitty_ready(&mut self, url: &str) -> bool { + if self.protocol != ImageProtocol::Kitty { + return false; + } + if let Some(ki) = self.kitty_images.get(url) { + return ki.is_some(); + } + let img = match self.images.get(url).and_then(|o| o.as_ref()) { + Some(img) => img, + None => return false, + }; + let png = match encode_png(img) { + Some(p) => p, + None => { + self.kitty_images.insert(url.to_string(), None); + return false; + } + }; + self.next_kitty_id = self.next_kitty_id.wrapping_add(1); + // 24-bit range; ID 0 is reserved by the spec. + if self.next_kitty_id == 0 || self.next_kitty_id > 0x00FF_FFFF { + self.next_kitty_id = 1; + } + let (w, h) = img.dimensions(); + let src_aspect = if h == 0 { 1.0 } else { w as f32 / h as f32 }; + self.kitty_images.insert( + url.to_string(), + Some(KittyImage { + id: self.next_kitty_id, + pending_png: Some(Arc::new(png)), + transmitted: false, + src_aspect, + }), + ); + true + } + + /// Place the image for `url` at the top-left of `rect`, sized to fit + /// within `rect`. Returns true if anything was written to stdout. + /// No-op on non-Kitty terminals. + pub fn render_at( + &mut self, + stdout: &mut impl Write, + url: &str, + rect_col: u16, + rect_row: u16, + max_cols: u16, + max_rows: u16, + ) -> io::Result { + if self.protocol != ImageProtocol::Kitty { + return Ok(false); + } + if !self.ensure_kitty_ready(url) { + return Ok(false); + } + let ki = match self.kitty_images.get_mut(url).and_then(|o| o.as_mut()) { + Some(ki) => ki, + None => return Ok(false), + }; + + // Size the placement: respect both rect bounds AND image aspect. + let (cols, rows) = + fit_cells_with_aspect(ki.src_aspect, max_cols as usize, max_rows as usize); + if cols == 0 || rows == 0 { + return Ok(false); + } + + // Position cursor at the rect's top-left in raw 1-based coords. + // crossterm's MoveTo is 0-based, so emit the CSI directly to stay + // independent of which Writer the caller uses (stdout, BufWriter…). + write!(stdout, "\x1b[{};{}H", rect_row + 1, rect_col + 1)?; + + if !ki.transmitted { + if let Some(png) = ki.pending_png.take() { + transmit_kitty_image(stdout, &png, ki.id)?; + } + ki.transmitted = true; + } + place_kitty_image(stdout, ki.id, cols, rows)?; + // Leave the cursor in a sane spot below the image so the next + // ratatui frame's diff doesn't get confused. + write!(stdout, "\x1b[{};1H", rect_row + rows + 1)?; + Ok(true) + } + + /// Tell the terminal to drop every Kitty placement. Call when leaving + /// any view that was showing an image so it doesn't bleed into the + /// next view's cells. Does NOT evict our in-process cache — we keep + /// the decoded image and the registered transmit so re-entering the + /// view is cheap. + pub fn clear_terminal(&mut self, stdout: &mut impl Write) -> io::Result<()> { + if self.protocol != ImageProtocol::Kitty { + return Ok(()); + } + // a=d (delete), d=a (all). q=2 suppresses the terminal's response. + stdout.write_all(b"\x1b_Ga=d,d=a,q=2\x1b\\")?; + // Every image we previously sent is gone from the terminal's + // registry, and successful transmits already dropped their PNG + // bytes — so drop `kitty_images` entirely and re-encode fresh on + // the next render. The decoded `images` cache is preserved. + self.kitty_images.clear(); + Ok(()) + } +} + +/// Fetch `url`, validate the response, decode into a `DynamicImage`, and +/// downscale. Returns None on any failure (network, status, size, format). +fn fetch_and_decode(url: &str, client: &reqwest::blocking::Client) -> Option { + let response = client.get(url).send().ok()?; + if !response.status().is_success() { + return None; + } + // Cap the body read so a hostile or misconfigured server can't OOM us. + use std::io::Read; + let mut bytes = Vec::with_capacity(64 * 1024); + response + .take(MAX_IMAGE_BYTES + 1) + .read_to_end(&mut bytes) + .ok()?; + if bytes.len() as u64 > MAX_IMAGE_BYTES { + return None; + } + // Decode with explicit limits: the byte cap above doesn't bound the + // *decoded* size, and a small PNG can inflate to gigabytes of pixels. + let mut limits = image::Limits::default(); + limits.max_image_width = Some(MAX_DECODE_DIM); + limits.max_image_height = Some(MAX_DECODE_DIM); + limits.max_alloc = Some(MAX_DECODE_ALLOC); + let mut reader = image::ImageReader::new(std::io::Cursor::new(&bytes)) + .with_guessed_format() + .ok()?; + reader.limits(limits); + let img = reader.decode().ok()?; + Some(downscale(img, MAX_SOURCE_DIM)) +} + +fn downscale(img: DynamicImage, max_dim: u32) -> DynamicImage { + let (w, h) = img.dimensions(); + if w <= max_dim && h <= max_dim { + return img; + } + let scale = max_dim as f32 / w.max(h) as f32; + let new_w = ((w as f32 * scale).round() as u32).max(1); + let new_h = ((h as f32 * scale).round() as u32).max(1); + img.resize(new_w, new_h, image::imageops::FilterType::Lanczos3) +} + +fn encode_png(img: &DynamicImage) -> Option> { + let mut out = Vec::with_capacity(64 * 1024); + img.write_to(&mut std::io::Cursor::new(&mut out), image::ImageFormat::Png) + .ok()?; + Some(out) +} + +fn fit_cells(src_w: u32, src_h: u32, max_cols: usize, max_rows: usize) -> (u16, u16) { + let aspect = if src_h == 0 { + 1.0 + } else { + src_w as f32 / src_h as f32 + }; + fit_cells_with_aspect(aspect, max_cols, max_rows) +} + +/// Pick (cols, rows) that preserves `src_aspect` (w/h in pixels), respecting +/// terminal cell aspect via `CELL_ASPECT` and clamping to `(max_cols, max_rows)`. +fn fit_cells_with_aspect(src_aspect: f32, max_cols: usize, max_rows: usize) -> (u16, u16) { + // Convert pixel-aspect → cell-aspect. A "square" pixel image of side N + // needs N*CELL_ASPECT cols of width per row of height to look square. + // i.e. cell_aspect = pixel_aspect / CELL_ASPECT. + let cell_aspect = src_aspect / CELL_ASPECT; + // Try fitting to width first. + let cols_from_rows = (max_rows as f32 * cell_aspect).round() as usize; + let rows_from_cols = (max_cols as f32 / cell_aspect).round() as usize; + let (cols, rows) = if cols_from_rows <= max_cols { + (cols_from_rows, max_rows) + } else { + (max_cols, rows_from_cols) + }; + ( + cols.min(max_cols).max(1) as u16, + rows.min(max_rows).max(1) as u16, + ) +} + +const BASE64: base64::engine::general_purpose::GeneralPurpose = + base64::engine::general_purpose::STANDARD; + +/// Send a PNG to the terminal under image id `id`, chunked at 4096 base64 +/// bytes per Kitty's protocol. Subsequent placements reference the id. +fn transmit_kitty_image(stdout: &mut impl Write, png: &[u8], id: u32) -> io::Result<()> { + let b64 = BASE64.encode(png); + let chunk_size = 4096; + let total_chunks = b64.len().div_ceil(chunk_size); + for (i, chunk) in b64.as_bytes().chunks(chunk_size).enumerate() { + let more = if i + 1 < total_chunks { 1 } else { 0 }; + if i == 0 { + // f=100: PNG. t=d: direct (inline data, not file path). + // q=2: suppress response (terminal stays quiet on success). + write!(stdout, "\x1b_Ga=t,f=100,t=d,i={},q=2,m={};", id, more)?; + } else { + write!(stdout, "\x1b_Gm={};", more)?; + } + stdout.write_all(chunk)?; + stdout.write_all(b"\x1b\\")?; + } + Ok(()) +} + +/// Place a transmitted image at the current cursor with the given cell +/// dimensions. `c` = columns, `r` = rows; the terminal scales to fit. +/// The explicit placement id (`p=1`) makes re-placement idempotent: +/// placements are keyed by (image id, placement id), so emitting this +/// every frame replaces the previous placement instead of accumulating +/// new ones in the terminal. +fn place_kitty_image(stdout: &mut impl Write, id: u32, cols: u16, rows: u16) -> io::Result<()> { + write!( + stdout, + "\x1b_Ga=p,i={},p=1,q=2,c={},r={};\x1b\\", + id, cols, rows + )?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + // ── fit_cells ──────────────────────────────────────────────────────── + + #[test] + fn fit_cells_square_image() { + // A square pixel image in a 40x20 cell area. With CELL_ASPECT=0.5, + // cells are 2x as tall as wide, so a square fits as cols = 2*rows. + // At max_rows=20 → cols_from_rows = 40 → exactly fits. + let (c, r) = fit_cells(100, 100, 40, 20); + assert_eq!(r, 20); + assert_eq!(c, 40); + } + + #[test] + fn fit_cells_wide_image() { + // 2:1 pixel-aspect → cell-aspect = 2/0.5 = 4. In a 40x20 area, + // width-bound: rows_from_cols = 40/4 = 10. + let (c, r) = fit_cells(200, 100, 40, 20); + assert_eq!(c, 40); + assert_eq!(r, 10); + } + + #[test] + fn fit_cells_clamps_to_at_least_one() { + // Tiny target should still yield at least 1x1. + let (c, r) = fit_cells(100, 100, 1, 1); + assert!(c >= 1); + assert!(r >= 1); + } + + // ── ImageCache ─────────────────────────────────────────────────────── + + #[test] + fn detect_protocol_none_by_default() { + // Env can poison this test if KITTY_WINDOW_ID is set in the parent. + // We can't safely unset process-global env in a multi-threaded + // test runner, so just assert the function returns *something*. + let p = detect_protocol(); + assert!(matches!(p, ImageProtocol::Kitty | ImageProtocol::None)); + } + + #[test] + fn cache_tracks_known_failures() { + let mut cache = ImageCache::new(); + // Mark as known-failed. + cache.insert_image("http://example.com/x.png".into(), None); + assert!(cache.images.contains_key("http://example.com/x.png")); + assert!(!cache.has_image("http://example.com/x.png")); + } + + #[test] + fn start_fetch_noop_in_none_protocol() { + let mut cache = ImageCache::new(); + cache.protocol = ImageProtocol::None; + assert!(cache.start_fetch("https://example.com/img.png")); + // No fetch should have spawned. + assert!(!cache.in_flight.contains("https://example.com/img.png")); + assert!(!cache.images.contains_key("https://example.com/img.png")); + } + + #[test] + fn start_fetch_rejects_unsafe_urls() { + let mut cache = ImageCache::new(); + cache.protocol = ImageProtocol::Kitty; + // localhost / private IPs are blocked by is_safe_auto_url. + assert!(cache.start_fetch("http://127.0.0.1/x.png")); + // Recorded as a known failure so it isn't re-queued every tick. + assert!(cache.images.contains_key("http://127.0.0.1/x.png")); + assert!(!cache.has_image("http://127.0.0.1/x.png")); + assert!(!cache.in_flight.contains("http://127.0.0.1/x.png")); + } + + #[test] + fn insert_image_evicts_oldest_past_cap() { + let mut cache = ImageCache::new(); + cache.protocol = ImageProtocol::Kitty; + let img = DynamicImage::new_rgb8(2, 2); + for i in 0..MAX_CACHED_IMAGES + 3 { + cache.insert_image(format!("https://x/{i}.png"), Some(Arc::new(img.clone()))); + } + assert_eq!(cache.images.len(), MAX_CACHED_IMAGES); + assert_eq!(cache.images_order.len(), MAX_CACHED_IMAGES); + // The three oldest are gone, the newest survive. + assert!(!cache.images.contains_key("https://x/0.png")); + assert!(!cache.images.contains_key("https://x/2.png")); + assert!(cache.has_image(&format!("https://x/{}.png", MAX_CACHED_IMAGES + 2))); + // Eviction also drops the Kitty payload for the evicted URL. + cache.kitty_images.insert("https://x/3.png".into(), None); + cache.insert_image("https://x/new.png".into(), Some(Arc::new(img))); + assert!(!cache.kitty_images.contains_key("https://x/3.png")); + } + + #[test] + fn insert_image_overwrite_does_not_grow_order() { + let mut cache = ImageCache::new(); + let img = DynamicImage::new_rgb8(2, 2); + cache.insert_image("https://x/a.png".into(), None); + cache.insert_image("https://x/a.png".into(), Some(Arc::new(img))); + assert_eq!(cache.images_order.len(), 1); + assert!(cache.has_image("https://x/a.png")); + } + + #[test] + fn ensure_kitty_ready_assigns_unique_ids() { + let mut cache = ImageCache::new(); + cache.protocol = ImageProtocol::Kitty; + let img = DynamicImage::new_rgb8(8, 8); + cache.images.insert("a".into(), Some(Arc::new(img.clone()))); + cache.images.insert("b".into(), Some(Arc::new(img))); + assert!(cache.ensure_kitty_ready("a")); + assert!(cache.ensure_kitty_ready("b")); + let id_a = cache.kitty_images.get("a").unwrap().as_ref().unwrap().id; + let id_b = cache.kitty_images.get("b").unwrap().as_ref().unwrap().id; + assert_ne!(id_a, id_b); + assert!(id_a > 0 && id_b > 0); + } + + #[test] + fn encode_png_round_trip() { + let img = DynamicImage::new_rgb8(4, 4); + let png = encode_png(&img).expect("png encodes"); + // PNG signature: 89 50 4E 47 0D 0A 1A 0A + assert_eq!(&png[..8], &[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + } + + #[test] + fn transmit_kitty_image_chunks_correctly() { + // Build a PNG large enough to require chunking. + let img = DynamicImage::new_rgb8(128, 128); + let png = encode_png(&img).unwrap(); + let mut out: Vec = Vec::new(); + transmit_kitty_image(&mut out, &png, 42).unwrap(); + let s = String::from_utf8_lossy(&out); + // First chunk has the full control payload. + assert!(s.contains("a=t,f=100,t=d,i=42,q=2")); + // Last chunk must end with the terminator after m=0. + assert!(s.contains("m=0;")); + assert!(s.ends_with("\x1b\\")); + } + + #[test] + fn place_kitty_image_formats_csi() { + let mut out: Vec = Vec::new(); + place_kitty_image(&mut out, 7, 30, 12).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert_eq!(s, "\x1b_Ga=p,i=7,p=1,q=2,c=30,r=12;\x1b\\"); + } +} diff --git a/src/lib.rs b/src/lib.rs index c089a8c..bf1481f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod config_tui; pub mod config_ui; pub mod events; pub mod feed; +pub mod image; pub mod keybindings; pub mod tui; pub mod ui; diff --git a/src/tui.rs b/src/tui.rs index 3952e06..5cfab08 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -505,6 +505,29 @@ fn spawn_pending_extractions( } } +/// Drain `app.pending_image_render` and emit Kitty graphics escapes to +/// place the image at the reserved rect. On view-change (no pending image +/// this frame but had one last frame), emit a clear-all so previous-frame +/// placements don't bleed into the next view. Errors are swallowed — a +/// failed image emit must never crash the TUI. +fn emit_inline_image(app: &mut App) -> Result<()> { + use std::io::Write; + if let Some(pi) = app.pending_image_render.take() { + let mut stdout = io::stdout().lock(); + let _ = app + .image_cache + .render_at(&mut stdout, &pi.url, pi.col, pi.row, pi.cols, pi.rows); + let _ = stdout.flush(); + app.last_frame_had_image = true; + } else if app.last_frame_had_image { + let mut stdout = io::stdout().lock(); + let _ = app.image_cache.clear_terminal(&mut stdout); + let _ = stdout.flush(); + app.last_frame_had_image = false; + } + Ok(()) +} + fn run_app(terminal: &mut Terminal, app: &mut App) -> Result<()> { let mut last_tick = std::time::Instant::now(); let tick_rate = Duration::from_millis(app.config.ui.tick_rate); @@ -532,11 +555,23 @@ fn run_app(terminal: &mut Terminal, app: &mut App) -> Result<()> let (mut pending_count, mut feed_rx) = spawn_feed_refresh(app); loop { + // Drain any background-fetched inline images so the next render + // can use them. Cheap (try_recv loop, no I/O). + app.image_cache.poll_completed(); + terminal.draw(|f| { app.update_compact_mode(f.size().height); ui::render(f, app); })?; + // Inline-image post-pass. Runs AFTER `terminal.draw()` so all + // ratatui escapes have been flushed; emitting our Kitty escapes + // here paints them on top of the (intentionally blank) image + // strip the detail view reserved in `app.pending_image_render`. + // On view-change we emit `clear_terminal` so a previous frame's + // image doesn't bleed into a non-detail view. + emit_inline_image(app)?; + // Check if a refresh was requested (by 'r' key or auto-refresh) if app.refresh_requested { app.refresh_requested = false; @@ -894,11 +929,61 @@ mod tests { parsed_date: None, plain_text: None, title_lower: title.to_lowercase(), + media: Vec::new(), + thumbnail: None, }) .collect(), } } + /// Regression lock: the Kitty escapes are emitted after + /// `terminal.draw()` returns, so a frame with a visible modal must not + /// queue an image placement — it would paint on top of the modal. + #[test] + fn test_modal_suppresses_pending_image_render() { + use ratatui::{backend::TestBackend, Terminal}; + let mut app = App::new(); + let mut feed = build_feed_with_items( + "https://ex.com/feed.xml", + vec![("Post", Some("https://ex.com/a"))], + ); + feed.items[0].thumbnail = Some("https://ex.com/t.png".to_string()); + app.feeds.push(feed); + app.view = crate::app::View::FeedItemDetail; + app.selected_feed = Some(0); + app.selected_item = Some(0); + // Pretend the thumbnail is already decoded and the terminal speaks + // Kitty, so the detail renderer reserves the strip and queues a + // placement. + app.image_cache + .force_protocol(crate::image::ImageProtocol::Kitty); + app.image_cache.insert_image( + "https://ex.com/t.png".to_string(), + Some(std::sync::Arc::new(image::DynamicImage::new_rgb8(100, 100))), + ); + + let backend = TestBackend::new(80, 40); + let mut terminal = Terminal::new(backend).unwrap(); + + terminal.draw(|f| crate::ui::render(f, &mut app)).unwrap(); + assert!( + app.pending_image_render.is_some(), + "modal-free frame should queue an image placement" + ); + + app.show_help_overlay = true; + terminal.draw(|f| crate::ui::render(f, &mut app)).unwrap(); + assert!( + app.pending_image_render.is_none(), + "frame with a visible modal must not queue an image placement" + ); + + // Closing the modal restores the placement on the next frame. + app.show_help_overlay = false; + terminal.draw(|f| crate::ui::render(f, &mut app)).unwrap(); + assert!(app.pending_image_render.is_some()); + } + #[test] fn test_enqueue_fulltext_for_new_skips_when_feed_not_opted_in() { let mut app = App::new(); diff --git a/src/ui/dashboard.rs b/src/ui/dashboard.rs index 30ffee3..3a57e04 100644 --- a/src/ui/dashboard.rs +++ b/src/ui/dashboard.rs @@ -1,7 +1,6 @@ use crate::app::App; use crate::ui::utils::{count_wrapped_lines, format_content_for_reading}; use crate::ui::ColorScheme; -use html2text::from_read; use ratatui::{ backend::Backend, layout::{Alignment, Constraint, Direction, Layout, Rect}, @@ -492,7 +491,8 @@ fn render_preview_pane( // Content if let Some(desc) = &item.description { - let raw_text = from_read(desc.as_bytes(), area.width.saturating_sub(10) as usize); + let raw_text = + crate::ui::utils::render_clean_html(desc, area.width.saturating_sub(10) as usize); let formatted = format_content_for_reading(&raw_text); for line in formatted.lines() { lines.push(Line::from(vec![Span::styled( diff --git a/src/ui/detail.rs b/src/ui/detail.rs index 8c46aa8..4481534 100644 --- a/src/ui/detail.rs +++ b/src/ui/detail.rs @@ -2,7 +2,6 @@ use crate::app::{find_article_matches, App, ArticleMatch, ExtractionState, Input use crate::keybindings::{key_display, KeyAction}; use crate::ui::utils::{count_wrapped_lines, format_content_for_reading, truncate_url}; use crate::ui::ColorScheme; -use html2text::from_read; use ratatui::{ backend::Backend, layout::{Alignment, Constraint, Direction, Layout, Rect}, @@ -41,29 +40,82 @@ pub(super) fn render_item_detail( // suffix all agree on visibility. let search_footer_visible = matches!(app.input_mode, InputMode::ArticleSearch) || !app.article_search_query.is_empty(); - if let Some(item) = app.current_item() { - // Split the area into header, content, and (optionally) a 1-row - // search footer. The footer is omitted when in-article search is - // inactive so quiet reading is unchanged. - let chunks = if search_footer_visible { - Layout::default() - .direction(Direction::Vertical) - .constraints([ - Constraint::Length(9), // Header - Constraint::Min(0), // Content - Constraint::Length(1), // Search footer - ]) - .split(area) - } else { - Layout::default() - .direction(Direction::Vertical) - .constraints([ - Constraint::Length(9), // Header - Constraint::Min(0), // Content - ]) - .split(area) - }; + // ── Image strip setup (hoisted above the `current_item` borrow) ───── + // We extract the thumbnail URL with a short-lived immutable borrow, + // then release it so we can mutate `app.image_cache` and write to + // `app.pending_image_render` below. The actual layout decision needs + // the URL string, which we own here. + let image_url: Option = app.current_item().and_then(|i| i.thumbnail.clone()); + let image_strip_rows: u16 = match &image_url { + Some(url) + if app.image_cache.has_image(url) + && app.image_cache.protocol() == crate::image::ImageProtocol::Kitty => + { + // 10 rows is a reasonable thumbnail size on a typical + // terminal: tall enough to show the image content, short + // enough to leave room for the body below. + 10 + } + _ => 0, + }; + // Queue a background fetch on first display of an item with a + // thumbnail. `start_fetch` is idempotent for already-attempted URLs, + // so re-rendering the same detail view is cheap. + if let Some(url) = &image_url { + app.image_cache.start_fetch(url); + } + // Split the area into header, optional image strip, content, and + // (optionally) a 1-row search footer. The footer is omitted when + // in-article search is inactive so quiet reading is unchanged. The + // split happens before the `item` borrow below so the *actual* strip + // rect can be stored in `pending_image_render`. + let mut constraints: Vec = vec![Constraint::Length(9)]; // Header + if image_strip_rows > 0 { + constraints.push(Constraint::Length(image_strip_rows)); + } + constraints.push(Constraint::Min(0)); // Content + if search_footer_visible { + constraints.push(Constraint::Length(1)); + } + let chunks = Layout::default() + .direction(Direction::Vertical) + .constraints(constraints) + .split(area); + // Resolve chunk indices: header is always [0]; image strip (if + // present) is [1] and shifts content/footer down by one. + let content_chunk_idx: usize = if image_strip_rows > 0 { 2 } else { 1 }; + let footer_chunk_idx: Option = if search_footer_visible { + Some(if image_strip_rows > 0 { 3 } else { 2 }) + } else { + None + }; + + // Compute the pending image rect from the strip chunk the layout + // solver actually produced — on short terminals it can be squeezed + // below the requested rows, and the image must not spill past it + // onto the content or help bar. The image is horizontally centered + // within the strip. None when no strip is shown or it collapsed. + app.pending_image_render = match (&image_url, image_strip_rows > 0) { + (Some(url), true) => { + let strip = chunks[1]; + if strip.width == 0 || strip.height == 0 { + None + } else { + app.image_cache + .display_cells(url, strip.width as usize, strip.height as usize) + .map(|(cols, rows)| crate::app::PendingImage { + url: url.clone(), + col: strip.x + strip.width.saturating_sub(cols) / 2, + row: strip.y, + cols, + rows, + }) + } + } + _ => None, + }; + if let Some(item) = app.current_item() { // Create header with enhanced typography let mut header_lines = vec![ // Title with better emphasis @@ -153,6 +205,12 @@ pub(super) fn render_item_detail( .alignment(Alignment::Left); f.render_widget(header, chunks[0]); + // The image strip (chunks[1], if present) is intentionally NOT + // rendered into — the cells stay blank so the Kitty escapes + // emitted after `terminal.draw()` returns paint the image on + // top, and ratatui's diff sees no changes there on subsequent + // frames (image persists). The target rect was already stored in + // `app.pending_image_render` before the item-borrow began. // Decide which body to render: the original summary, the extracted // full-text, or a placeholder for in-flight / failed extractions. @@ -181,7 +239,7 @@ pub(super) fn render_item_detail( // duplicated when extraction fails and we fall back to the summary. let summary_text = || -> String { if let Some(desc) = &item.description { - let raw_text = from_read(desc.as_bytes(), 100); + let raw_text = crate::ui::utils::render_clean_html(desc, 100); format_content_for_reading(&raw_text) } else { "No description available".to_string() @@ -221,13 +279,13 @@ pub(super) fn render_item_detail( }; // Calculate the viewport height (accounting for borders and padding) - let viewport_height = chunks[1] + let viewport_height = chunks[content_chunk_idx] .height .saturating_sub(2) // borders (top and bottom) .saturating_sub(4); // increased padding (top and bottom) // Calculate the content width (accounting for borders and padding) - let content_width = chunks[1] + let content_width = chunks[content_chunk_idx] .width .saturating_sub(2) // borders (left and right) .saturating_sub(8) // increased padding for better reading width @@ -359,10 +417,10 @@ pub(super) fn render_item_detail( .wrap(Wrap { trim: true }) .alignment(Alignment::Left); - f.render_widget(content, chunks[1]); + f.render_widget(content, chunks[content_chunk_idx]); - if search_footer_visible { - render_search_footer(f, app, chunks[2], colors); + if let Some(idx) = footer_chunk_idx { + render_search_footer(f, app, chunks[idx], colors); } } } diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 0bbaa20..8d3de31 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -355,6 +355,28 @@ pub fn render(f: &mut Frame, app: &mut App) { if app.show_help_overlay { render_help_overlay(f, app, &colors); } + + // Inline-image vs modal z-order: the Kitty escapes are emitted *after* + // `terminal.draw()` returns, so they would paint on top of any modal + // rendered above. Suppress the image on frames where a modal is + // visible (mirroring the conditions above); the run loop's clear-all / + // re-place cycle restores it when the modal closes. The success + // notification is deliberately excluded — it renders inside the + // title-bar region and cannot overlap the image strip. + let modal_visible = app.error.is_some() + || matches!( + app.input_mode, + InputMode::InsertUrl + | InputMode::SearchMode + | InputMode::SelectDiscoveredFeed + | InputMode::CategoryNameInput + ) + || app.filter_mode + || app.show_link_overlay + || app.show_help_overlay; + if modal_visible { + app.pending_image_render = None; + } } fn render_title_bar(f: &mut Frame, app: &App, area: Rect, colors: &ColorScheme) { diff --git a/src/ui/utils.rs b/src/ui/utils.rs index 910e3bf..5e2108d 100644 --- a/src/ui/utils.rs +++ b/src/ui/utils.rs @@ -1,6 +1,100 @@ +use html2text::render::text_renderer::{TaggedLine, TextDecorator}; use ratatui::layout::Rect; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; +/// html2text decorator tuned for RSS-summary rendering. Differs from the +/// crate's default `PlainDecorator` in two ways: +/// +/// * **No link annotations.** `decorate_link_start`/`_end` and `finalise` +/// return empty strings, so anchors render as just their inner text — no +/// `[text][N]` markers and no `[N]: url` footnote dump at the bottom. +/// RSS summaries (Reddit especially) are otherwise drowned in +/// `submitted by [ /u/... ][2] [[link]][3] [[comments]][4]` boilerplate +/// plus a multi-line footnote block, none of which the user needs — the +/// article URL is already shown in the detail-view header. +/// * **Image alt text falls back to title.** Same as `PlainDecorator` +/// (`[title]`) so the user still sees *something* for inline images. +/// +/// Emphasis (`*…*`), strong (`**…**`), code (`` `…` ``), list/header/quote +/// prefixes are preserved. +#[derive(Clone, Debug, Default)] +pub(crate) struct CleanDecorator; + +impl CleanDecorator { + pub(crate) fn new() -> Self { + Self + } +} + +impl TextDecorator for CleanDecorator { + type Annotation = (); + + fn decorate_link_start(&mut self, _url: &str) -> (String, Self::Annotation) { + (String::new(), ()) + } + fn decorate_link_end(&mut self) -> String { + String::new() + } + fn decorate_em_start(&mut self) -> (String, Self::Annotation) { + ("*".to_string(), ()) + } + fn decorate_em_end(&mut self) -> String { + "*".to_string() + } + fn decorate_strong_start(&mut self) -> (String, Self::Annotation) { + ("**".to_string(), ()) + } + fn decorate_strong_end(&mut self) -> String { + "**".to_string() + } + fn decorate_strikeout_start(&mut self) -> (String, Self::Annotation) { + (String::new(), ()) + } + fn decorate_strikeout_end(&mut self) -> String { + String::new() + } + fn decorate_code_start(&mut self) -> (String, Self::Annotation) { + ("`".to_string(), ()) + } + fn decorate_code_end(&mut self) -> String { + "`".to_string() + } + fn decorate_preformat_first(&mut self) -> Self::Annotation {} + fn decorate_preformat_cont(&mut self) -> Self::Annotation {} + fn decorate_image(&mut self, _src: &str, title: &str) -> (String, Self::Annotation) { + (format!("[{}]", title), ()) + } + fn header_prefix(&mut self, level: usize) -> String { + "#".repeat(level) + " " + } + fn quote_prefix(&mut self) -> String { + "> ".to_string() + } + fn unordered_item_prefix(&mut self) -> String { + "* ".to_string() + } + fn ordered_item_prefix(&mut self, i: i64) -> String { + format!("{}. ", i) + } + fn finalise(&mut self, _links: Vec) -> Vec> { + // Crucially: no footnote-style `[N]: url` lines. The default + // `PlainDecorator` returns one per link here — that's the entire + // reason summaries grew a noisy reference-list trailer. + Vec::new() + } + fn make_subblock_decorator(&self) -> Self { + Self + } +} + +/// Convenience: flatten any `` markup, then render with +/// [`CleanDecorator`] at the given width. Used by detail/dashboard views +/// to convert a feed item's HTML description into prose plain text. +pub(crate) fn render_clean_html(html: &str, width: usize) -> String { + let prepped = crate::feed::flatten_description_html(html); + html2text::from_read_with_decorator(prepped.as_bytes(), width, CleanDecorator::new()) +} + // Helper function to create a centered rect with minimum dimensions pub(crate) fn centered_rect_with_min( percent_x: u16, @@ -157,3 +251,52 @@ pub(crate) fn count_wrapped_lines(text: &str, width: usize) -> u16 { // If text is empty, return at least 1 line line_count.max(1) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn render_clean_html_drops_reddit_link_footnotes() { + // Reddit summary: title link, body, "submitted by" line with three + // anchors. Default html2text would emit `[N]` markers inline plus a + // `[N]: url` footnote dump. Our decorator must produce neither. + let html = r#"
Excuse me?! What??
This is weird, right? Like what is going on with suggestive content lately?!
submitted by /u/A [link] [comments]
"#; + let out = render_clean_html(html, 80); + // No reference markers anywhere. + assert!( + !out.contains("][1]") && !out.contains("][2]") && !out.contains("][3]"), + "expected no `][N]` link markers in:\n{out}" + ); + // No footnote dump. + assert!( + !out.contains("[1]:") && !out.contains("[2]:"), + "expected no `[N]: url` footnotes in:\n{out}" + ); + // No `|` column artifacts from the table. + assert!( + !out.contains('|'), + "expected no column separators in:\n{out}" + ); + // Anchor inner text is preserved. + assert!(out.contains("Excuse me?! What??")); + assert!(out.contains("/u/A")); + // Emphasis is preserved (this is what distinguishes us from + // html2text's TrivialDecorator, which would strip the `*`s). + assert!( + out.contains("*suggestive*"), + "expected `` to render as `*...*` in:\n{out}" + ); + } + + #[test] + fn render_clean_html_strips_image_with_empty_alt() { + // An image with no alt/title should render as `[]` (PlainDecorator + // behavior we inherit) rather than disappearing — the user still + // gets a visible hint that something was there. + let html = r#"

Before after.

"#; + let out = render_clean_html(html, 80); + assert!(out.contains("Before")); + assert!(out.contains("after.")); + } +} From 5a4beabcd37f763e9b3483438ae40ec9da5aac05 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Thu, 11 Jun 2026 22:13:20 +0530 Subject: [PATCH 3/5] build: hold image at 0.25.6 to keep the toolchain floor at 1.81 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. --- Cargo.lock | 39 +++++++++++---------------------------- Cargo.toml | 5 +++++ 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b544a90..8a6a0b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -810,9 +810,9 @@ dependencies = [ [[package]] name = "gif" -version = "0.14.2" +version = "0.13.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee8cfcc411d9adbbaba82fb72661cc1bcca13e8bba98b364e62b2dba8f960159" +checksum = "4ae047235e33e2829703574b54fdec96bfbad892062d97fed2f76022287de61b" dependencies = [ "color_quant", "weezl", @@ -1174,16 +1174,15 @@ dependencies = [ [[package]] name = "image" -version = "0.25.10" +version = "0.25.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85ab80394333c02fe689eaf900ab500fbd0c2213da414687ebf995a65d5a6104" +checksum = "db35664ce6b9810857a38a906215e75a9c879f0696556a39f59c62829710251a" dependencies = [ "bytemuck", "byteorder-lite", "color_quant", "gif", "image-webp", - "moxcms", "num-traits", "png", "zune-core", @@ -1394,16 +1393,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "moxcms" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb85c154ba489f01b25c0d36ae69a87e4a1c73a72631fc6c0eb6dde34a73e44b" -dependencies = [ - "num-traits", - "pxfm", -] - [[package]] name = "native-tls" version = "0.2.14" @@ -1724,11 +1713,11 @@ checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" [[package]] name = "png" -version = "0.18.1" +version = "0.17.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60769b8b31b2a9f263dae2776c37b1b28ae246943cf719eb6946a1db05128a61" +checksum = "82151a2fc869e011c153adc57cf2789ccb8d9906ce52c0b39a6b5697749d7526" dependencies = [ - "bitflags 2.9.0", + "bitflags 1.3.2", "crc32fast", "fdeflate", "flate2", @@ -1765,12 +1754,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "pxfm" -version = "0.1.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0c5ccf5294c6ccd63a74f1565028353830a9c2f5eb0c682c355c471726a6e3f" - [[package]] name = "quick-error" version = "2.0.1" @@ -3303,15 +3286,15 @@ dependencies = [ [[package]] name = "zune-core" -version = "0.5.1" +version = "0.4.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb8a0807f7c01457d0379ba880ba6322660448ddebc890ce29bb64da71fb40f9" +checksum = "3f423a2c17029964870cfaabb1f13dfab7d092a62a29a89264f4d36990ca414a" [[package]] name = "zune-jpeg" -version = "0.5.15" +version = "0.4.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "27bc9d5b815bc103f142aa054f561d9187d191692ec7c2d1e2b4737f8dbd7296" +checksum = "29ce2c8a9384ad323cf564b67da86e21d3cfdff87908bc1223ed5c99bc792713" dependencies = [ "zune-core", ] diff --git a/Cargo.toml b/Cargo.toml index 843d1c1..78a722f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,11 @@ encoding_rs = "0.8" # features disabled to avoid pulling in formats we don't surface — we only # need to *decode* whatever the network sends and *encode* PNG to hand to # the Kitty protocol. +# Held at 0.25.6 via the committed lockfile (same pattern as dom_smoothie +# above): image 0.25.7+ declares rust-version up to 1.88, while the rest of +# this project's dependency graph tops out at 1.81. Bumping image past +# 0.25.6 is a deliberate toolchain-floor decision, not a routine +# `cargo update`. image = { version = "0.25", default-features = false, features = ["png", "jpeg", "gif", "webp"] } base64 = "0.22" From 3bfc4e91f53846f833d955a41add5e78a8ce92a0 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Thu, 11 Jun 2026 22:14:09 +0530 Subject: [PATCH 4/5] fix(feed): route cached plain_text through the clean HTML renderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CLAUDE.md | 2 +- src/feed.rs | 192 ++++++++++++++++++++++++++++++++++++++++++-- src/ui/dashboard.rs | 3 +- src/ui/detail.rs | 2 +- src/ui/utils.rs | 143 --------------------------------- 5 files changed, 188 insertions(+), 154 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a3809e7..d5be0a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,7 @@ MSRV: 1.75.0. CI runs tests on stable, beta, and 1.75.0. - **`tui.rs`** — Terminal setup/teardown, main event loop (`run_app`), feed refresh logic, and external-command runners (`suspend_for_command`, `spawn_detached`, `drain_macro_steps`, `collect_exec_on_new`/`flush_exec_on_new`). `TerminalRestoreGuard` (RAII) re-enters alt-screen + raw mode + mouse capture on drop so a panic in a child invocation can't leave the terminal broken. - **`events.rs`** — All keyboard and mouse event handling (`handle_events`). Input dispatches based on `View` × `InputMode` enums. Hosts the macro engine (`dispatch_action`, `run_macro`, `build_pipe_invocation`, `build_exec_invocation`). Separated from `tui.rs` for maintainability. - **`keybindings.rs`** — `KeyAction` enum, default keybinding map, key string parsing, config-driven keybinding overrides via `[keybindings]` TOML section, and newsboat-style macro parsing (`MacroStep`, `MacroBinding`, `MacroOptions`, `parse_macro_string`). -- **`feed.rs`** — Data models (`Feed`, `FeedItem`, `FeedCategory`), RSS/Atom parsing via `feed-rs`, and HTML feed auto-discovery via `scraper`. +- **`feed.rs`** — Data models (`Feed`, `FeedItem`, `FeedCategory`), RSS/Atom parsing via `feed-rs`, and HTML feed auto-discovery via `scraper`. Also owns the shared HTML→clean-text path (`render_clean_html` + `CleanDecorator`: tables flattened, no `[N]` link markers/footnotes) — it lives here, not in `ui/`, because the cached `FeedItem::plain_text` is built with it at parse time, so pipe payloads, content filters, and the detail/dashboard views all see the same text. - **`config.rs`** — XDG-compliant config loading/saving (`~/.config/feedr/config.toml`). Includes `keybindings: HashMap` for custom key overrides, `[hooks]` (`exec_on_new`), `[macros]`, and `[macro_options]`. Auto-generates defaults on first run. - **`config_cli.rs`** — CLI subcommand handler for `feedr config list/get/set`. - **`config_tui.rs`** — Interactive TUI config editor (`feedr config --tui`). diff --git a/src/feed.rs b/src/feed.rs index dc2c50d..fb2c04a 100644 --- a/src/feed.rs +++ b/src/feed.rs @@ -2,6 +2,7 @@ use anyhow::{Context, Result}; use chrono::{DateTime, Utc}; use dom_smoothie::{Config as ReadabilityConfig, Readability}; use feed_rs::parser; +use html2text::render::text_renderer::{TaggedLine, TextDecorator}; use scraper::{Html, Selector}; use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; @@ -815,13 +816,11 @@ impl FeedItem { }; // Cache plain text from description (avoids repeated HTML parsing). - // Table structure is flattened first — see `flatten_description_html` - // for why (Reddit RSS in particular wraps metadata in a 2-column - // table that html2text would otherwise render with `|` separators). - let plain_text = description.as_ref().map(|desc| { - let prepped = flatten_description_html(desc); - html2text::from_read(prepped.as_bytes(), 80) - }); + // Rendered through the same `render_clean_html` path as the + // detail/dashboard views so pipe payloads and content filters see + // the text the user sees — tables flattened, no `[N]` link markers, + // no footnote dump. + let plain_text = description.as_ref().map(|desc| render_clean_html(desc, 80)); // Extract the primary link let link = entry.links.first().map(|link| link.href.clone()); @@ -1052,6 +1051,105 @@ pub(crate) fn flatten_description_html(html: &str) -> String { out } +/// html2text decorator tuned for RSS-summary rendering. Differs from the +/// crate's default `PlainDecorator` in two ways: +/// +/// * **No link annotations.** `decorate_link_start`/`_end` and `finalise` +/// return empty strings, so anchors render as just their inner text — no +/// `[text][N]` markers and no `[N]: url` footnote dump at the bottom. +/// RSS summaries (Reddit especially) are otherwise drowned in +/// `submitted by [ /u/... ][2] [[link]][3] [[comments]][4]` boilerplate +/// plus a multi-line footnote block, none of which the user needs — the +/// article URL is already shown in the detail-view header. +/// * **Image alt text falls back to title.** Same as `PlainDecorator` +/// (`[title]`) so the user still sees *something* for inline images. +/// +/// Emphasis (`*…*`), strong (`**…**`), code (`` `…` ``), list/header/quote +/// prefixes are preserved. +/// +/// Lives here (not in `ui/`) because the cached `FeedItem::plain_text` is +/// built with it at parse time — pipe payloads, content filters, and the +/// rendered views must all see the same text. +#[derive(Clone, Debug, Default)] +pub(crate) struct CleanDecorator; + +impl CleanDecorator { + pub(crate) fn new() -> Self { + Self + } +} + +impl TextDecorator for CleanDecorator { + type Annotation = (); + + fn decorate_link_start(&mut self, _url: &str) -> (String, Self::Annotation) { + (String::new(), ()) + } + fn decorate_link_end(&mut self) -> String { + String::new() + } + fn decorate_em_start(&mut self) -> (String, Self::Annotation) { + ("*".to_string(), ()) + } + fn decorate_em_end(&mut self) -> String { + "*".to_string() + } + fn decorate_strong_start(&mut self) -> (String, Self::Annotation) { + ("**".to_string(), ()) + } + fn decorate_strong_end(&mut self) -> String { + "**".to_string() + } + fn decorate_strikeout_start(&mut self) -> (String, Self::Annotation) { + (String::new(), ()) + } + fn decorate_strikeout_end(&mut self) -> String { + String::new() + } + fn decorate_code_start(&mut self) -> (String, Self::Annotation) { + ("`".to_string(), ()) + } + fn decorate_code_end(&mut self) -> String { + "`".to_string() + } + fn decorate_preformat_first(&mut self) -> Self::Annotation {} + fn decorate_preformat_cont(&mut self) -> Self::Annotation {} + fn decorate_image(&mut self, _src: &str, title: &str) -> (String, Self::Annotation) { + (format!("[{}]", title), ()) + } + fn header_prefix(&mut self, level: usize) -> String { + "#".repeat(level) + " " + } + fn quote_prefix(&mut self) -> String { + "> ".to_string() + } + fn unordered_item_prefix(&mut self) -> String { + "* ".to_string() + } + fn ordered_item_prefix(&mut self, i: i64) -> String { + format!("{}. ", i) + } + fn finalise(&mut self, _links: Vec) -> Vec> { + // Crucially: no footnote-style `[N]: url` lines. The default + // `PlainDecorator` returns one per link here — that's the entire + // reason summaries grew a noisy reference-list trailer. + Vec::new() + } + fn make_subblock_decorator(&self) -> Self { + Self + } +} + +/// Convenience: flatten any `` markup, then render with +/// [`CleanDecorator`] at the given width. The single HTML→text path for +/// feed descriptions: `FeedItem::plain_text` (pipe payloads, filters) and +/// the detail/dashboard views all go through here, so what the user sees, +/// searches, and pipes is the same text. +pub(crate) fn render_clean_html(html: &str, width: usize) -> String { + let prepped = flatten_description_html(html); + html2text::from_read_with_decorator(prepped.as_bytes(), width, CleanDecorator::new()) +} + fn format_date(dt: DateTime) -> String { // Calculate how long ago the item was published let now = Utc::now(); @@ -1141,6 +1239,86 @@ mod tests { assert!(text.contains("Title here")); } + #[test] + fn render_clean_html_drops_reddit_link_footnotes() { + // Reddit summary: title link, body, "submitted by" line with three + // anchors. Default html2text would emit `[N]` markers inline plus a + // `[N]: url` footnote dump. Our decorator must produce neither. + let html = r#"
Excuse me?! What??
This is weird, right? Like what is going on with suggestive content lately?!
submitted by /u/A [link] [comments]
"#; + let out = render_clean_html(html, 80); + // No reference markers anywhere. + assert!( + !out.contains("][1]") && !out.contains("][2]") && !out.contains("][3]"), + "expected no `][N]` link markers in:\n{out}" + ); + // No footnote dump. + assert!( + !out.contains("[1]:") && !out.contains("[2]:"), + "expected no `[N]: url` footnotes in:\n{out}" + ); + // No `|` column artifacts from the table. + assert!( + !out.contains('|'), + "expected no column separators in:\n{out}" + ); + // Anchor inner text is preserved. + assert!(out.contains("Excuse me?! What??")); + assert!(out.contains("/u/A")); + // Emphasis is preserved (this is what distinguishes us from + // html2text's TrivialDecorator, which would strip the `*`s). + assert!( + out.contains("*suggestive*"), + "expected `` to render as `*...*` in:\n{out}" + ); + } + + #[test] + fn render_clean_html_strips_image_with_empty_alt() { + // An image with no alt/title should render as `[]` (PlainDecorator + // behavior we inherit) rather than disappearing — the user still + // gets a visible hint that something was there. + let html = r#"

Before after.

"#; + let out = render_clean_html(html, 80); + assert!(out.contains("Before")); + assert!(out.contains("after.")); + } + + #[test] + fn plain_text_cache_matches_clean_rendering() { + // The cached plain_text (pipe payloads, content filters) must go + // through the same clean-render path as the views: no `[N]` link + // markers, no footnote dump. + let xml = r#" + + + x + https://example.com/ + x + + Post + https://example.com/p1 + https://example.com/p1 + <p>Read <a href="https://example.com/more">the rest</a> now.</p> + + +"#; + let entry = parse_first_entry(xml); + let item = FeedItem::from_feed_entry(&entry); + let plain = item.plain_text.expect("plain_text cached"); + assert!(plain.contains("the rest")); + assert!( + !plain.contains("[1]") && !plain.contains("[1]:"), + "expected no link annotations in cached plain_text:\n{plain}" + ); + assert_eq!( + plain, + render_clean_html( + "

Read the rest now.

", + 80 + ) + ); + } + #[test] fn test_discover_single_rss_feed() { let html = br#" diff --git a/src/ui/dashboard.rs b/src/ui/dashboard.rs index 3a57e04..bc0ed11 100644 --- a/src/ui/dashboard.rs +++ b/src/ui/dashboard.rs @@ -491,8 +491,7 @@ fn render_preview_pane( // Content if let Some(desc) = &item.description { - let raw_text = - crate::ui::utils::render_clean_html(desc, area.width.saturating_sub(10) as usize); + let raw_text = crate::feed::render_clean_html(desc, area.width.saturating_sub(10) as usize); let formatted = format_content_for_reading(&raw_text); for line in formatted.lines() { lines.push(Line::from(vec![Span::styled( diff --git a/src/ui/detail.rs b/src/ui/detail.rs index 4481534..f44edc9 100644 --- a/src/ui/detail.rs +++ b/src/ui/detail.rs @@ -239,7 +239,7 @@ pub(super) fn render_item_detail( // duplicated when extraction fails and we fall back to the summary. let summary_text = || -> String { if let Some(desc) = &item.description { - let raw_text = crate::ui::utils::render_clean_html(desc, 100); + let raw_text = crate::feed::render_clean_html(desc, 100); format_content_for_reading(&raw_text) } else { "No description available".to_string() diff --git a/src/ui/utils.rs b/src/ui/utils.rs index 5e2108d..910e3bf 100644 --- a/src/ui/utils.rs +++ b/src/ui/utils.rs @@ -1,100 +1,6 @@ -use html2text::render::text_renderer::{TaggedLine, TextDecorator}; use ratatui::layout::Rect; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; -/// html2text decorator tuned for RSS-summary rendering. Differs from the -/// crate's default `PlainDecorator` in two ways: -/// -/// * **No link annotations.** `decorate_link_start`/`_end` and `finalise` -/// return empty strings, so anchors render as just their inner text — no -/// `[text][N]` markers and no `[N]: url` footnote dump at the bottom. -/// RSS summaries (Reddit especially) are otherwise drowned in -/// `submitted by [ /u/... ][2] [[link]][3] [[comments]][4]` boilerplate -/// plus a multi-line footnote block, none of which the user needs — the -/// article URL is already shown in the detail-view header. -/// * **Image alt text falls back to title.** Same as `PlainDecorator` -/// (`[title]`) so the user still sees *something* for inline images. -/// -/// Emphasis (`*…*`), strong (`**…**`), code (`` `…` ``), list/header/quote -/// prefixes are preserved. -#[derive(Clone, Debug, Default)] -pub(crate) struct CleanDecorator; - -impl CleanDecorator { - pub(crate) fn new() -> Self { - Self - } -} - -impl TextDecorator for CleanDecorator { - type Annotation = (); - - fn decorate_link_start(&mut self, _url: &str) -> (String, Self::Annotation) { - (String::new(), ()) - } - fn decorate_link_end(&mut self) -> String { - String::new() - } - fn decorate_em_start(&mut self) -> (String, Self::Annotation) { - ("*".to_string(), ()) - } - fn decorate_em_end(&mut self) -> String { - "*".to_string() - } - fn decorate_strong_start(&mut self) -> (String, Self::Annotation) { - ("**".to_string(), ()) - } - fn decorate_strong_end(&mut self) -> String { - "**".to_string() - } - fn decorate_strikeout_start(&mut self) -> (String, Self::Annotation) { - (String::new(), ()) - } - fn decorate_strikeout_end(&mut self) -> String { - String::new() - } - fn decorate_code_start(&mut self) -> (String, Self::Annotation) { - ("`".to_string(), ()) - } - fn decorate_code_end(&mut self) -> String { - "`".to_string() - } - fn decorate_preformat_first(&mut self) -> Self::Annotation {} - fn decorate_preformat_cont(&mut self) -> Self::Annotation {} - fn decorate_image(&mut self, _src: &str, title: &str) -> (String, Self::Annotation) { - (format!("[{}]", title), ()) - } - fn header_prefix(&mut self, level: usize) -> String { - "#".repeat(level) + " " - } - fn quote_prefix(&mut self) -> String { - "> ".to_string() - } - fn unordered_item_prefix(&mut self) -> String { - "* ".to_string() - } - fn ordered_item_prefix(&mut self, i: i64) -> String { - format!("{}. ", i) - } - fn finalise(&mut self, _links: Vec) -> Vec> { - // Crucially: no footnote-style `[N]: url` lines. The default - // `PlainDecorator` returns one per link here — that's the entire - // reason summaries grew a noisy reference-list trailer. - Vec::new() - } - fn make_subblock_decorator(&self) -> Self { - Self - } -} - -/// Convenience: flatten any `` markup, then render with -/// [`CleanDecorator`] at the given width. Used by detail/dashboard views -/// to convert a feed item's HTML description into prose plain text. -pub(crate) fn render_clean_html(html: &str, width: usize) -> String { - let prepped = crate::feed::flatten_description_html(html); - html2text::from_read_with_decorator(prepped.as_bytes(), width, CleanDecorator::new()) -} - // Helper function to create a centered rect with minimum dimensions pub(crate) fn centered_rect_with_min( percent_x: u16, @@ -251,52 +157,3 @@ pub(crate) fn count_wrapped_lines(text: &str, width: usize) -> u16 { // If text is empty, return at least 1 line line_count.max(1) } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn render_clean_html_drops_reddit_link_footnotes() { - // Reddit summary: title link, body, "submitted by" line with three - // anchors. Default html2text would emit `[N]` markers inline plus a - // `[N]: url` footnote dump. Our decorator must produce neither. - let html = r#"
Excuse me?! What??
This is weird, right? Like what is going on with suggestive content lately?!
submitted by /u/A [link] [comments]
"#; - let out = render_clean_html(html, 80); - // No reference markers anywhere. - assert!( - !out.contains("][1]") && !out.contains("][2]") && !out.contains("][3]"), - "expected no `][N]` link markers in:\n{out}" - ); - // No footnote dump. - assert!( - !out.contains("[1]:") && !out.contains("[2]:"), - "expected no `[N]: url` footnotes in:\n{out}" - ); - // No `|` column artifacts from the table. - assert!( - !out.contains('|'), - "expected no column separators in:\n{out}" - ); - // Anchor inner text is preserved. - assert!(out.contains("Excuse me?! What??")); - assert!(out.contains("/u/A")); - // Emphasis is preserved (this is what distinguishes us from - // html2text's TrivialDecorator, which would strip the `*`s). - assert!( - out.contains("*suggestive*"), - "expected `` to render as `*...*` in:\n{out}" - ); - } - - #[test] - fn render_clean_html_strips_image_with_empty_alt() { - // An image with no alt/title should render as `[]` (PlainDecorator - // behavior we inherit) rather than disappearing — the user still - // gets a visible hint that something was there. - let html = r#"

Before after.

"#; - let out = render_clean_html(html, 80); - assert!(out.contains("Before")); - assert!(out.contains("after.")); - } -} From 8d2f52abce0c2a35d9a665b9404d1284d24dfb8e Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Thu, 11 Jun 2026 22:14:23 +0530 Subject: [PATCH 5/5] fix(image): stop leaking terminal pixel data and stacking placements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CLAUDE.md | 2 +- src/image.rs | 258 ++++++++++++++++++++++++++++++++++++++++++----- src/tui.rs | 47 +++++++++ src/ui/detail.rs | 19 +++- 4 files changed, 294 insertions(+), 32 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d5be0a4..e6519de 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,7 +66,7 @@ MSRV: 1.75.0. CI runs tests on stable, beta, and 1.75.0. - **Compact mode**: `app.compact` bool is updated each frame by `update_compact_mode(terminal_height)`. Rendering in `ui.rs` branches on `app.compact` for layout, title bar, help bar, and dashboard item format. Controlled by `config.ui.compact_mode` (`Auto`/`Always`/`Never`). Dialog modals use `centered_rect_with_min()` to enforce minimum dimensions regardless of compact mode. - **External-command hooks (macros + `exec_on_new`)**: Commands are run **without a shell**. Templates are tokenized once at config load via `shlex`, then `expand_argv_template` substitutes `%X` placeholders (`%t %u %a %d %f %F %m %M %i %%`) into individual argv tokens (no re-expansion), so feed content cannot break out of an argument. `%m` (primary media URL), `%M` (its MIME), and `%i` (thumbnail URL) come from `` / `` / Atom `` parsed in `feed::extract_media` and routed through `FeedItem::primary_media`; they expand to `""` when absent (no fallback to `%u`). The macro engine queues steps into `app.pending_macro_steps` from `events.rs` and the TUI loop drains them in `tui.rs::drain_macro_steps` — drain lives at the loop level because `pipe-to` needs the terminal handle to suspend the TUI. Chains halt on the first step error (tracked via a `pre_error` guard so a stale `app.error` doesn't spuriously abort). The macro prefix (default `,`) is checked at the top of `handle_key_event` and only when `input_mode == Normal`, so text-input modes are not disturbed; an idle prefix times out via the existing success-message timeout. - **`exec_on_new` crash semantics**: AT-MOST-ONCE. `flush_exec_on_new` persists the `seen_items` / `feeds_seeded` sets *before* spawning any child, so a kill mid-fire loses a notification rather than re-firing on the next launch. `mark_feed_seen` only flips `feeds_seeded` on a fetch that returned items (transiently-empty first fetches don't seed), and the first observation of a feed seeds the seen-set silently to avoid a firehose. Children are spawned detached with stdio nulled; a reaper thread waits on each so they don't linger as zombies. The seen-set is pruned in `remove_current_feed` to prevent monotonic growth across feed churn. **Single-shared mark per feed**: `mark_feed_seen` is hoisted to the feed-drain call site in `tui.rs` (gated on `exec_on_new_template.is_some() || fulltext_feeds.contains(&feed.url)`) so multiple consumers (currently exec_on_new and fulltext) share one mark per feed arrival — calling it twice would double-mark and the second consumer would see an empty `newly_seen` list. -- **Inline images (Kitty protocol)**: `src/image.rs` owns an `ImageCache` (hung on `App.image_cache`) that fetches `` URLs in background threads (`MAX_CONCURRENT_FETCHES = 4`), decodes via the `image` crate, downscales to `MAX_SOURCE_DIM`, re-encodes as PNG, and transmits to the terminal via the Kitty graphics protocol in 4096-byte base64 chunks. Protocol detection (`detect_protocol`) checks `KITTY_WINDOW_ID`, `TERM=*kitty/ghostty*`, `TERM_PROGRAM=ghostty|wezterm` — but returns `None` first when `$TMUX` is set (tmux swallows APC sequences, no passthrough support); everything else is a silent no-op. **Integration shape**: the detail-view renderer (`ui::detail`) reserves a 10-row strip below the header by adding a `Constraint::Length(10)` to the vertical Layout — but **renders nothing into those cells**. Instead it stores `(url, col, row, cols, rows)` in `App.pending_image_render`, derived from the strip rect the layout solver *actually* produced (which can be squeezed below 10 rows on short terminals — the image must not spill past it). After `terminal.draw()` returns in `tui::run_app`, `emit_inline_image` drains that slot and writes the Kitty escapes directly to stdout via `io::stdout().lock()`. ratatui's diff sees the strip cells as "unchanged blank" on subsequent frames, so it emits no escapes there and the image persists; placements use a fixed placement id (`p=1`) so per-frame re-placement replaces rather than accumulates. **Modal z-order**: because the escapes land after the draw, `ui::render` nulls `pending_image_render` on any frame where a modal is visible (error, input modes, filter, link overlay, help overlay) so the image can't paint over it — regression-locked by `test_modal_suppresses_pending_image_render`. On view-change (no pending image this frame but `last_frame_had_image` is true), `emit_inline_image` writes `\x1b_Ga=d,d=a\x1b\\` (Kitty delete-all) so the image doesn't bleed into the next view. SSRF: the upfront URL is gated through `feed::is_safe_auto_url` AND the fetch uses `Feed::build_safe_redirect_client` so every redirect hop is re-validated (thumbnails are hostile feed content fetched with no user action). Body size is hard-capped at `MAX_IMAGE_BYTES = 5 MB`, and decoding runs under explicit `image::Limits` (`MAX_DECODE_DIM`, `MAX_DECODE_ALLOC`) so a small PNG can't inflate to gigabytes. The decoded cache is LRU-capped at `MAX_CACHED_IMAGES = 32` via `insert_image` (eviction also drops the matching Kitty payload). Worker panics are caught so a hostile image can't strand an `in_flight` slot. The summary-`` thumbnail fallback (`feed::extract_first_image_url`) skips declared-tiny images (width/height ≤ 2) so FeedBurner-style 1×1 beacons don't win as "the" thumbnail. +- **Inline images (Kitty protocol)**: `src/image.rs` owns an `ImageCache` (hung on `App.image_cache`) that fetches `` URLs in background threads (`MAX_CONCURRENT_FETCHES = 4`), decodes via the `image` crate, downscales to `MAX_SOURCE_DIM`, re-encodes as PNG, and transmits to the terminal via the Kitty graphics protocol in 4096-byte base64 chunks. Protocol detection (`detect_protocol`) checks `KITTY_WINDOW_ID`, `TERM=*kitty/ghostty*`, `TERM_PROGRAM=ghostty|wezterm` — but returns `None` first when `$TMUX` is set (tmux swallows APC sequences, no passthrough support); everything else is a silent no-op. **Integration shape**: the detail-view renderer (`ui::detail`) reserves a 10-row strip below the header by adding a `Constraint::Length(10)` to the vertical Layout — but **renders nothing into those cells**. The strip is only reserved when the image is loaded AND `!app.compact` AND the detail area is ≥30 rows (`MIN_AREA_HEIGHT_FOR_IMAGE`) — header (9) + strip (10) both outrank the `Min(0)` body, so on shorter areas the strip would squeeze the article to near-zero rows (regression-locked by `test_image_strip_suppressed_on_short_or_compact_terminals`). It stores `(url, col, row, cols, rows)` in `App.pending_image_render`, derived from the strip rect the layout solver *actually* produced (which can still be squeezed below 10 rows — the image must not spill past it). After `terminal.draw()` returns in `tui::run_app`, `emit_inline_image` drains that slot and writes the Kitty escapes directly to stdout via `io::stdout().lock()`. ratatui's diff sees the strip cells as "unchanged blank" on subsequent frames, so it emits no escapes there and the image persists; placements use a fixed placement id (`p=1`) so per-frame re-placement of the *same* image replaces rather than accumulates; placements are keyed by (image id, placement id), so when the displayed image *changes*, `render_at` deletes the previous image's placement (tracked via `last_placed_id`) before placing the new one — without that, both images would stack in the strip. **Modal z-order**: because the escapes land after the draw, `ui::render` nulls `pending_image_render` on any frame where a modal is visible (error, input modes, filter, link overlay, help overlay) so the image can't paint over it — regression-locked by `test_modal_suppresses_pending_image_render`. On view-change (no pending image this frame but `last_frame_had_image` is true), `emit_inline_image` calls `clear_terminal`, which deletes all *placements* (`a=d,d=a` — lowercase `d=a` keeps the transmitted pixel data in the terminal's registry) so the image doesn't bleed into the next view; `kitty_images` ids/`transmitted` flags stay valid against that retained data, so re-entering the view is a single placement escape with no PNG re-encode or re-transmit. Terminal-side pixel data is freed only when a URL ages out of the LRU: eviction queues the id in `pending_deletes` and the next write flushes a `d=I` (uppercase: placements + data) — keeping the terminal's image memory bounded by `MAX_CACHED_IMAGES`. SSRF: the upfront URL is gated through `feed::is_safe_auto_url` AND the fetch uses `Feed::build_safe_redirect_client` so every redirect hop is re-validated (thumbnails are hostile feed content fetched with no user action). Body size is hard-capped at `MAX_IMAGE_BYTES = 5 MB`, and decoding runs under explicit `image::Limits` (`MAX_DECODE_DIM`, `MAX_DECODE_ALLOC`) so a small PNG can't inflate to gigabytes. The decoded cache is LRU-capped at `MAX_CACHED_IMAGES = 32` via `insert_image` (eviction also drops the matching Kitty payload). Worker panics are caught so a hostile image can't strand an `in_flight` slot, and workers spawn via `thread::Builder` so OS thread-creation failure releases the slot (returns false = retry next tick) instead of panicking the TUI. Failed fetches are cached as permanent known-failures (no retry/TTL) until LRU-evicted — deliberate MVP semantics. The summary-`` thumbnail fallback (`feed::extract_first_image_url`) skips declared-tiny images (width/height ≤ 2) so FeedBurner-style 1×1 beacons don't win as "the" thumbnail. - **Full-text extraction**: `feed::extract_article` fetches an article URL with the existing `reqwest::blocking::Client` and runs Mozilla Readability via the `dom_smoothie` crate. **Sync, no tokio.** Per-feed `Authorization`/auth headers are intentionally NOT forwarded to article URLs (they're third-party hosts — propagating would be a credential leak). The response body is read via `Response::take(FULLTEXT_MAX_BYTES+1).read_to_end(...)` so peak allocation is hard-capped at ~5 MB regardless of what the server sends, and the response charset is honored (`Content-Type charset=` → `` sniff → UTF-8) via `encoding_rs` so non-UTF8 pages don't mojibake. Each extraction runs on a `std::thread::spawn` worker wrapped in `catch_unwind` (so a `dom_smoothie` panic on hostile HTML surfaces as `Failed("…panicked…")` instead of stranding the slot on `Pending` forever). The TUI loop maintains a `Arc` `extract_inflight` budget capped at `EXTRACTION_MAX_INFLIGHT = 4`; queued requests beyond that budget — or whose domain was last fetched less than `refresh_rate_limit_delay` ago — are pushed back onto `pending_extraction_requests` to retry on the next loop tick. State lives only in `App::extracted: HashMap` (`Pending` / `Ready(ExtractedArticle)` / `Failed(String)`) — **in-memory only**, never persisted; LRU-capped at `EXTRACTED_CACHE_CAP = 500` with insertion-order tracking via `extracted_order: VecDeque`. The cap is **hard**: `insert_extraction` always evicts the deque head when full, and `record_extraction_result` rejects results for slots that aren't currently `Pending` (so a late worker for an evicted / removed id is dropped rather than resurrecting dead state). `Shift+F` (`KeyAction::FetchFullText`) toggles between summary and extracted text when `Ready`, queues a new request when absent, and re-queues on `Failed` (so the user can retry). Per-feed `fulltext = true` in config auto-extracts newly-seen items on refresh (same `mark_feed_seen` "newly seen" semantics as `exec_on_new` — first fetch seeds silently, no firehose); the auto path additionally filters via `feed::is_safe_auto_url` (http/https only, rejects RFC1918 / loopback / link-local / CGNAT / multicast / 6to4 / NAT64 / `localhost`-style names) to prevent a hostile feed from probing internal hosts. The auto path's worker also uses `Feed::build_safe_redirect_client`, whose `redirect::Policy::custom` re-runs `is_safe_auto_url` on every hop, so a public-looking `` that 302s into an internal target is rejected mid-chain instead of slipping past the upfront URL check. Each `ExtractionRequest` carries a `safe_redirects` flag (true for auto, false for manual `Shift+F`); the spawn loop picks the matching client per-request. Manual `Shift+F` bypasses both the URL allowlist and the safe-redirect client (it's the user's explicit action, same trust model as opening the article in a browser). The spawn loop also gates each pop on the slot still being `Pending`, so requests whose `extracted` entry was evicted by LRU or pruned by `remove_current_feed` get dropped without spawning a worker, and uses `std::thread::Builder::new().spawn()` so an OS thread-creation failure releases the inflight slot and re-queues the request instead of crashing the TUI. The detail-view lookup uses `current_article_indices()` (same resolver as the action handler) so they stay in lockstep. Extracted entries are pruned alongside `seen_items` in `remove_current_feed` via the `remove_extraction(&id)` helper. ## Commit Conventions diff --git a/src/image.rs b/src/image.rs index a22e550..b5cfd98 100644 --- a/src/image.rs +++ b/src/image.rs @@ -118,7 +118,11 @@ struct KittyImage { /// → on first render: transmitted=true (PNG sent to terminal) /// /// A failed fetch lands as `images: None` (insert-known-failed); subsequent -/// `start_fetch` calls for that URL no-op. +/// `start_fetch` calls for that URL no-op. Deliberately permanent (no +/// retry, no TTL) until the slot ages out of the LRU: a transient network +/// blip costs that article its thumbnail for the session, in exchange for +/// never re-hitting a dead URL at render frequency. Revisit if users +/// report chronically missing thumbnails. /// /// Debug is implemented manually because the internal `mpsc::Receiver` /// has no `Debug` impl — we surface counts instead of contents. @@ -141,6 +145,17 @@ pub struct ImageCache { /// trigger fetches via `&mut App` without the caller threading a client /// through every UI layer. Construction is cheap relative to a fetch. client: Option, + /// Kitty image id of the placement currently on screen, if any. + /// `render_at` deletes the previous placement when the id changes — + /// placements are keyed by (image id, placement id), so re-using + /// `p=1` across *different* image ids would otherwise stack both + /// images in the strip. + last_placed_id: Option, + /// Kitty ids whose terminal-side pixel data should be freed (their + /// URL was evicted from our cache, so we'd never place them again). + /// Deletion needs a writer, which eviction sites don't have — queued + /// here and flushed by `render_at` / `clear_terminal`. + pending_deletes: Vec, } impl Default for ImageCache { @@ -173,6 +188,8 @@ impl ImageCache { receiver, in_flight: HashSet::new(), client: None, + last_placed_id: None, + pending_deletes: Vec::new(), } } @@ -197,20 +214,38 @@ impl ImageCache { /// oldest entry past `MAX_CACHED_IMAGES`. Eviction also drops the /// matching Kitty payload so stale PNG state can't outlive its image; /// an evicted-but-displayed URL simply re-fetches on the next render. + /// Transmitted ids of dropped payloads are queued so the *terminal's* + /// copy of the pixel data is freed too (see `pending_deletes`). pub(crate) fn insert_image(&mut self, url: String, img: Option>) { - if self.images.insert(url.clone(), img).is_none() { + if self.images.insert(url.clone(), img).is_some() { + // Overwrite (e.g. a retried known-failure): the old Kitty + // payload no longer matches the new image. + self.drop_kitty_payload(&url); + } else { self.images_order.push_back(url); } while self.images_order.len() > MAX_CACHED_IMAGES { if let Some(oldest) = self.images_order.pop_front() { self.images.remove(&oldest); - self.kitty_images.remove(&oldest); + self.drop_kitty_payload(&oldest); + } + } + } + + /// Remove the Kitty payload for `url`; if its PNG was already + /// transmitted, queue a terminal-side data delete for the next flush. + fn drop_kitty_payload(&mut self, url: &str) { + if let Some(Some(ki)) = self.kitty_images.remove(url) { + if ki.transmitted { + self.pending_deletes.push(ki.id); } } } /// Queue a background fetch+decode for `url`. Returns false if the - /// concurrency cap is full — caller can re-queue on the next tick. + /// fetch could not be started right now (concurrency cap full, or OS + /// thread creation failed) — both are transient, and the caller can + /// re-queue on the next tick. /// Idempotent: re-queueing an in-flight or already-attempted URL is a /// no-op (returns true, the "already handled" signal). pub fn start_fetch(&mut self, url: &str) -> bool { @@ -247,16 +282,27 @@ impl ImageCache { self.in_flight.insert(url.to_string()); let sender = self.sender.clone(); let url_owned = url.to_string(); - std::thread::spawn(move || { - // Panic-safe: a panic in the image decoder must still free the - // in-flight slot via `poll_completed`. Without `catch_unwind`, - // a single hostile image could permanently strand the slot. - let img = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - fetch_and_decode(&url_owned, &client) - })) - .unwrap_or(None); - let _ = sender.send((url_owned, img)); - }); + // Builder::spawn (not thread::spawn) so an OS thread-creation + // failure is an Err instead of a panic that takes down the TUI — + // same hardening as the fulltext extraction workers. + let spawned = std::thread::Builder::new() + .name("feedr-img-fetch".to_string()) + .spawn(move || { + // Panic-safe: a panic in the image decoder must still free the + // in-flight slot via `poll_completed`. Without `catch_unwind`, + // a single hostile image could permanently strand the slot. + let img = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + fetch_and_decode(&url_owned, &client) + })) + .unwrap_or(None); + let _ = sender.send((url_owned, img)); + }); + if spawned.is_err() { + // Transient resource exhaustion: release the slot so a later + // render can retry rather than caching a permanent failure. + self.in_flight.remove(url); + return false; + } true } @@ -338,6 +384,9 @@ impl ImageCache { if !self.ensure_kitty_ready(url) { return Ok(false); } + // Free terminal-side data for any ids evicted since the last write. + self.flush_pending_deletes(stdout)?; + let last_placed = self.last_placed_id; let ki = match self.kitty_images.get_mut(url).and_then(|o| o.as_mut()) { Some(ki) => ki, None => return Ok(false), @@ -361,29 +410,55 @@ impl ImageCache { } ki.transmitted = true; } - place_kitty_image(stdout, ki.id, cols, rows)?; + let id = ki.id; + // Placements are keyed by (image id, placement id). Re-placing the + // SAME image id with p=1 replaces the previous placement, but a + // DIFFERENT image id would stack on top of the old one — so when + // switching images within a frame-pair, delete the old image's + // placement first (d=i, lowercase: placement only, its pixel data + // stays cached terminal-side for a cheap re-place later). + if let Some(old) = last_placed { + if old != id { + write!(stdout, "\x1b_Ga=d,d=i,i={},q=2\x1b\\", old)?; + } + } + place_kitty_image(stdout, id, cols, rows)?; + self.last_placed_id = Some(id); // Leave the cursor in a sane spot below the image so the next // ratatui frame's diff doesn't get confused. write!(stdout, "\x1b[{};1H", rect_row + rows + 1)?; Ok(true) } - /// Tell the terminal to drop every Kitty placement. Call when leaving - /// any view that was showing an image so it doesn't bleed into the - /// next view's cells. Does NOT evict our in-process cache — we keep - /// the decoded image and the registered transmit so re-entering the - /// view is cheap. + /// Tell the terminal to drop every Kitty *placement* (`d=a`, lowercase: + /// the transmitted pixel data stays in the terminal's image registry). + /// Call when leaving any view that was showing an image so it doesn't + /// bleed into the next view's cells. Keeps `kitty_images` intact — + /// ids and `transmitted` flags stay valid against the retained + /// terminal-side data, so re-entering the view is a single cheap + /// placement escape, with no PNG re-encode or re-transmit. Terminal + /// data is freed only when a URL ages out of our LRU (`d=I` via + /// `pending_deletes`), keeping the terminal's copy bounded by + /// `MAX_CACHED_IMAGES`. pub fn clear_terminal(&mut self, stdout: &mut impl Write) -> io::Result<()> { if self.protocol != ImageProtocol::Kitty { return Ok(()); } - // a=d (delete), d=a (all). q=2 suppresses the terminal's response. + self.flush_pending_deletes(stdout)?; + // a=d (delete), d=a (all visible placements). q=2 suppresses the + // terminal's response. stdout.write_all(b"\x1b_Ga=d,d=a,q=2\x1b\\")?; - // Every image we previously sent is gone from the terminal's - // registry, and successful transmits already dropped their PNG - // bytes — so drop `kitty_images` entirely and re-encode fresh on - // the next render. The decoded `images` cache is preserved. - self.kitty_images.clear(); + self.last_placed_id = None; + Ok(()) + } + + /// Emit `d=I` (uppercase: delete placements AND free pixel data) for + /// every id whose URL was evicted from the cache. Queued at eviction + /// time because eviction sites have no writer. + fn flush_pending_deletes(&mut self, stdout: &mut impl Write) -> io::Result<()> { + for id in self.pending_deletes.drain(..) { + write!(stdout, "\x1b_Ga=d,d=I,i={},q=2\x1b\\", id)?; + } Ok(()) } } @@ -656,4 +731,135 @@ mod tests { let s = String::from_utf8(out).unwrap(); assert_eq!(s, "\x1b_Ga=p,i=7,p=1,q=2,c=30,r=12;\x1b\\"); } + + // ── placement / terminal-data lifecycle ───────────────────────────── + + /// Build a Kitty-mode cache with a decoded image already in place. + fn cache_with_images(urls: &[&str]) -> ImageCache { + let mut cache = ImageCache::new(); + cache.protocol = ImageProtocol::Kitty; + let img = DynamicImage::new_rgb8(8, 8); + for url in urls { + cache.insert_image(url.to_string(), Some(Arc::new(img.clone()))); + } + cache + } + + #[test] + fn clear_terminal_deletes_placements_only_and_keeps_transmit_state() { + let mut cache = cache_with_images(&["https://x/a.png"]); + let mut out: Vec = Vec::new(); + assert!(cache + .render_at(&mut out, "https://x/a.png", 0, 0, 40, 10) + .unwrap()); + let first = String::from_utf8_lossy(&out).to_string(); + assert!(first.contains("a=t"), "first render transmits the PNG"); + + // View-change clear: placements-only (lowercase d=a), never d=A — + // the terminal keeps the pixel data so re-entry is cheap. + let mut cleared: Vec = Vec::new(); + cache.clear_terminal(&mut cleared).unwrap(); + let cleared = String::from_utf8_lossy(&cleared).to_string(); + assert!( + cleared.contains("a=d,d=a"), + "expected placement-only clear in: {cleared}" + ); + assert!( + !cleared.contains("d=A"), + "must not free terminal data on view change" + ); + assert!(cache.last_placed_id.is_none()); + + // Re-entering the view re-places without re-encoding/re-transmitting. + let mut out2: Vec = Vec::new(); + assert!(cache + .render_at(&mut out2, "https://x/a.png", 0, 0, 40, 10) + .unwrap()); + let second = String::from_utf8_lossy(&out2).to_string(); + assert!( + !second.contains("a=t"), + "re-render must not retransmit: {second}" + ); + assert!( + second.contains("a=p"), + "re-render places the retained image" + ); + } + + #[test] + fn switching_images_deletes_previous_placement() { + // Regression lock for placement stacking: placements are keyed by + // (image id, placement id), so placing image B with p=1 does NOT + // replace image A's (id_a, p=1) placement — render_at must delete + // it explicitly when the displayed image changes. + let mut cache = cache_with_images(&["https://x/a.png", "https://x/b.png"]); + let mut out: Vec = Vec::new(); + cache + .render_at(&mut out, "https://x/a.png", 0, 0, 40, 10) + .unwrap(); + let id_a = cache + .kitty_images + .get("https://x/a.png") + .unwrap() + .as_ref() + .unwrap() + .id; + + let mut out_b: Vec = Vec::new(); + cache + .render_at(&mut out_b, "https://x/b.png", 0, 0, 40, 10) + .unwrap(); + let s = String::from_utf8_lossy(&out_b).to_string(); + assert!( + s.contains(&format!("a=d,d=i,i={},q=2", id_a)), + "expected placement delete for image {id_a} in: {s}" + ); + // Same-image re-render must NOT delete its own placement. + let mut out_b2: Vec = Vec::new(); + cache + .render_at(&mut out_b2, "https://x/b.png", 0, 0, 40, 10) + .unwrap(); + let s2 = String::from_utf8_lossy(&out_b2).to_string(); + assert!( + !s2.contains("a=d,d=i"), + "same image re-place needs no delete: {s2}" + ); + } + + #[test] + fn lru_eviction_frees_terminal_data_on_next_flush() { + let mut cache = cache_with_images(&["https://x/old.png"]); + let mut out: Vec = Vec::new(); + cache + .render_at(&mut out, "https://x/old.png", 0, 0, 40, 10) + .unwrap(); + let old_id = cache + .kitty_images + .get("https://x/old.png") + .unwrap() + .as_ref() + .unwrap() + .id; + + // Push the transmitted image out of the LRU. + let img = DynamicImage::new_rgb8(2, 2); + for i in 0..MAX_CACHED_IMAGES { + cache.insert_image( + format!("https://x/fill-{i}.png"), + Some(Arc::new(img.clone())), + ); + } + assert!(!cache.kitty_images.contains_key("https://x/old.png")); + assert_eq!(cache.pending_deletes, vec![old_id]); + + // The next write flushes a d=I (placements + pixel data) for it. + let mut cleared: Vec = Vec::new(); + cache.clear_terminal(&mut cleared).unwrap(); + let s = String::from_utf8_lossy(&cleared).to_string(); + assert!( + s.contains(&format!("a=d,d=I,i={},q=2", old_id)), + "expected terminal-data delete for {old_id} in: {s}" + ); + assert!(cache.pending_deletes.is_empty()); + } } diff --git a/src/tui.rs b/src/tui.rs index 5cfab08..be0c471 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -984,6 +984,53 @@ mod tests { assert!(app.pending_image_render.is_some()); } + /// Regression lock: the 10-row image strip must give way to the + /// article body — no strip (and no placement) on short terminals or + /// in compact mode, where header + strip would squeeze the body to + /// near-zero rows. + #[test] + fn test_image_strip_suppressed_on_short_or_compact_terminals() { + use ratatui::{backend::TestBackend, Terminal}; + let mut app = App::new(); + let mut feed = build_feed_with_items( + "https://ex.com/feed.xml", + vec![("Post", Some("https://ex.com/a"))], + ); + feed.items[0].thumbnail = Some("https://ex.com/t.png".to_string()); + app.feeds.push(feed); + app.view = crate::app::View::FeedItemDetail; + app.selected_feed = Some(0); + app.selected_item = Some(0); + app.image_cache + .force_protocol(crate::image::ImageProtocol::Kitty); + app.image_cache.insert_image( + "https://ex.com/t.png".to_string(), + Some(std::sync::Arc::new(image::DynamicImage::new_rgb8(100, 100))), + ); + + // Short terminal: detail area falls below the strip threshold. + let mut terminal = Terminal::new(TestBackend::new(80, 24)).unwrap(); + terminal.draw(|f| crate::ui::render(f, &mut app)).unwrap(); + assert!( + app.pending_image_render.is_none(), + "short terminal must not reserve the image strip" + ); + + // Tall terminal but compact mode forced on: also suppressed. + let mut terminal = Terminal::new(TestBackend::new(80, 40)).unwrap(); + app.compact = true; + terminal.draw(|f| crate::ui::render(f, &mut app)).unwrap(); + assert!( + app.pending_image_render.is_none(), + "compact mode must not reserve the image strip" + ); + + // Sanity: same tall terminal, compact off → strip + placement. + app.compact = false; + terminal.draw(|f| crate::ui::render(f, &mut app)).unwrap(); + assert!(app.pending_image_render.is_some()); + } + #[test] fn test_enqueue_fulltext_for_new_skips_when_feed_not_opted_in() { let mut app = App::new(); diff --git a/src/ui/detail.rs b/src/ui/detail.rs index f44edc9..35f65ca 100644 --- a/src/ui/detail.rs +++ b/src/ui/detail.rs @@ -46,15 +46,24 @@ pub(super) fn render_item_detail( // `app.pending_image_render` below. The actual layout decision needs // the URL string, which we own here. let image_url: Option = app.current_item().and_then(|i| i.thumbnail.clone()); + // 10 rows is a reasonable thumbnail size on a typical terminal: tall + // enough to show the image content, short enough to leave room for the + // body below. + const IMAGE_STRIP_ROWS: u16 = 10; + // Minimum detail-area height to reserve the strip. The header is a + // fixed 9 rows and the strip 10, both higher layout priority than the + // `Min(0)` body — on anything shorter the body (which loses another 6 + // rows to borders/padding) collapses to near-zero and the article + // becomes unreadable. 30 leaves at least ~5 visible text lines. + const MIN_AREA_HEIGHT_FOR_IMAGE: u16 = 30; let image_strip_rows: u16 = match &image_url { Some(url) - if app.image_cache.has_image(url) + if !app.compact + && area.height >= MIN_AREA_HEIGHT_FOR_IMAGE + && app.image_cache.has_image(url) && app.image_cache.protocol() == crate::image::ImageProtocol::Kitty => { - // 10 rows is a reasonable thumbnail size on a typical - // terminal: tall enough to show the image content, short - // enough to leave room for the body below. - 10 + IMAGE_STRIP_ROWS } _ => 0, };