diff --git a/CLAUDE.md b/CLAUDE.md index 3c0c1c0..ab71c5f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Project Overview -Feedr is a terminal-based RSS/Atom feed reader built with Rust, using ratatui/crossterm for the TUI. It supports feed management, categorization, filtering, dual themes, OPML import, auto-refresh with per-domain rate limiting, feed auto-discovery from HTML pages, configurable keybindings, mouse support, and a help overlay. +Feedr is a terminal-based RSS/Atom feed reader built with Rust, using ratatui/crossterm for the TUI. It supports feed management, categorization, filtering, dual themes, OPML import, auto-refresh with per-domain rate limiting, feed auto-discovery from HTML pages, configurable keybindings, mouse support, a help overlay, and newsboat-style external-command hooks (macros and `exec_on_new`). ## Build & Development Commands @@ -28,11 +28,11 @@ MSRV: 1.75.0. CI runs tests on stable, beta, and 1.75.0. ### Core modules - **`app.rs`** — `App` struct holding all application state. All state mutations (feed ops, filtering, categorization, persistence) happen through its methods. This is the largest and most central file. -- **`tui.rs`** — Terminal setup/teardown, main event loop (`run_app`), and feed refresh logic. -- **`events.rs`** — All keyboard and mouse event handling (`handle_events`). Input dispatches based on `View` × `InputMode` enums. Separated from `tui.rs` for maintainability. -- **`keybindings.rs`** — `KeyAction` enum, default keybinding map, key string parsing, and config-driven keybinding overrides via `[keybindings]` TOML section. +- **`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`. -- **`config.rs`** — XDG-compliant config loading/saving (`~/.config/feedr/config.toml`). Includes `keybindings: HashMap` for custom key overrides. Auto-generates defaults on first run. +- **`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`). - **`config_ui.rs`** — Rendering for the TUI config editor. @@ -64,6 +64,8 @@ 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. +- **`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. ## Commit Conventions @@ -71,4 +73,4 @@ Uses **conventional commits** — `feat:`, `fix:`, `refactor:`, `docs:`, `perf:` ## Testing -Integration tests live in `/tests/integration_test.rs` and test feed parsing against real URLs. Unit tests are inline in `config.rs`, `app.rs`, and `keybindings.rs`. +Integration tests live in `/tests/integration_test.rs` and test feed parsing against real URLs. Unit tests are inline in `config.rs`, `app.rs`, `keybindings.rs`, `events.rs`, and `tui.rs` (the macro-drain tests use a `TestBackend` to avoid needing a real TTY). diff --git a/Cargo.lock b/Cargo.lock index 3a7f086..cc431c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -530,6 +530,7 @@ dependencies = [ "scraper", "serde", "serde_json", + "shlex", "toml", "unicode-width 0.1.14", "url", diff --git a/Cargo.toml b/Cargo.toml index 1cf6f38..8ee8fab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ opml = "1.1.6" toml = "0.8" scraper = "0.18" url = "2" +shlex = "1.3" [profile.release] codegen-units = 1 diff --git a/README.md b/README.md index 9a72e30..c29c125 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,7 @@ Feedr is a feature-rich terminal-based RSS feed reader written in Rust. It provi - **Compact Mode**: Automatic compact layout for small terminals (≤30 rows), with manual `always`/`never` override in config - **CLI Config Management**: Get, set, and list configuration from the command line (`feedr config`), or use the interactive TUI config editor (`feedr config --tui`) - **Configurable Keybindings**: Remap any key action via the `[keybindings]` section in `config.toml` +- **External-Command Hooks**: Newsboat-style macros (`pipe-to`, `exec`) bound to keys, plus `exec_on_new` notifications fired per new item — all with shell-free argument templating - **Configurable**: Customize timeouts, themes, UI behavior, and default feeds via TOML config - **XDG Compliant**: Follows standard directory specifications for configuration and data storage @@ -313,6 +314,66 @@ Cookie = "session=abc123" ``` Headers are sent with every request for that feed, including refreshes. +### External-Command Hooks + +Feedr supports newsboat-style external commands for two workflows: **macros** (key-triggered chains that act on the focused article) and **`exec_on_new`** (a notification hook fired per newly-seen item after each refresh). + +**Commands are not run through a shell.** Templates are tokenized once at config load, and `%X` placeholders are substituted into individual `argv` tokens — feed content can never break out of an argument. For pipes, redirection, or globbing, write a small shell script and invoke that. + +#### Template Variables + +Expanded in every `argv` token of macro and hook commands: + +| Variable | Expands to | +|----------|------------| +| `%t` | Article title | +| `%u` | Article URL | +| `%a` | Author | +| `%d` | Formatted publish date | +| `%f` | Feed title | +| `%F` | Feed URL | +| `%%` | Literal `%` | + +#### 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. + +```toml +[macros] +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' + +[macro_options] +prefix = "," # the macro-prefix key +pipe_default_stdin = "body" # body | title | url | metadata | none +``` + +**Step kinds:** +- `` — invoke a built-in action. Supported in macros: `open-in-browser`, `toggle-star`, `toggle-read`, `mark-all-read`, `refresh`, `toggle-theme`, `extract-links`, `help`. +- `pipe-to "cmd %u" [stdin=…]` — suspend the TUI, run the command, and pipe article content to its stdin. `stdin` is one of `body` (default), `title`, `url`, `metadata`, or `none`. +- `exec "cmd %u"` — spawn the command detached (no stdin, no terminal takeover). + +Chains halt on the first step error. Press `Esc` after the prefix to cancel; an unbound follow-up surfaces a "No macro bound" error. Macros are also rendered in the help overlay (`?`). + +#### `exec_on_new` Notifications + +Fire a command once per newly-seen item after each refresh. The first successful fetch of each feed seeds the seen-set silently — you do **not** get a firehose on initial load or first run. + +```toml +[hooks] +exec_on_new = 'notify-send "New: %t" "%f"' +``` + +Children are spawned **detached** so the TUI never blocks on them. Crash semantics are **at-most-once**: feedr persists the seen-set before spawning, so a kill mid-fire loses a notification rather than re-firing on the next launch. Prefer idempotent commands (e.g. `wallabag-cli add` is safe; `mail-me` is not). + +#### Security Notes + +- The shell is never invoked, so feed content in `%t` / `%a` / etc. cannot escape an argument. +- **Do not wrap your command in `sh -c "... %t ..."`** — that reintroduces shell injection through item titles. Write a script file and invoke it instead. +- `~` / `$HOME` / `$VAR` are **not** expanded — use absolute paths. +- If a macro's command template has unbalanced quotes or names an unknown action, feedr surfaces a startup warning rather than failing silently at trigger time. + ### Configurable Keybindings Remap any action by adding a `[keybindings]` section to your config file. Each action can be bound to a single key string or an array of keys: diff --git a/src/app.rs b/src/app.rs index ab8b8d4..f862b29 100644 --- a/src/app.rs +++ b/src/app.rs @@ -4,7 +4,7 @@ use crate::ui::ColorScheme; use anyhow::Result; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; -use std::collections::{HashMap, HashSet}; +use std::collections::{HashMap, HashSet, VecDeque}; use std::fs; use std::path::{Path, PathBuf}; use std::time::Instant; @@ -120,6 +120,41 @@ pub struct App { pub feed_tree: Vec, pub selected_tree_item: Option, // index into feed_tree pub keybindings: crate::keybindings::KeyBindingMap, + pub macros: Vec, + pub macro_options: crate::keybindings::MacroOptions, + /// True after the macro-prefix key has been pressed and we are awaiting the next key. + pub awaiting_macro_key: bool, + /// When `awaiting_macro_key` was last set, so the TUI tick loop can time + /// it out (reusing the 1.5 s success-message timeout) rather than + /// stranding the prefix state across an idle period. + pub awaiting_macro_key_since: Option, + /// Item IDs that exec_on_new has already fired for (persisted). + pub seen_items: HashSet, + /// Feed URLs whose first successful fetch has been observed (persisted). + pub feeds_seeded: HashSet, + /// FIFO queue of macro steps awaiting execution. Filled by the macro + /// engine in `events.rs`; drained in order by the TUI run loop after + /// each event-handling pass. Steps include Actions (which mutate app + /// state) and PipeTo/Exec (which spawn children). Draining happens at + /// the loop level because PipeTo needs the terminal handle to suspend + /// the TUI, which `handle_events` does not have access to. + pub pending_macro_steps: VecDeque, + /// Pre-tokenized `[hooks].exec_on_new` template. `None` if the hook is + /// disabled or the template failed to tokenize at startup (in which case + /// a startup warning is surfaced). + pub exec_on_new_template: Option>, +} + +/// Snapshot of the focused article passed to template-expansion / pipe-payload helpers. +#[derive(Clone, Debug)] +pub struct ArticleContext<'a> { + pub title: &'a str, + pub url: Option<&'a str>, + pub author: Option<&'a str>, + pub formatted_date: Option<&'a str>, + pub feed_title: &'a str, + pub feed_url: &'a str, + pub plain_text: Option<&'a str>, } #[derive(Clone, Debug)] @@ -165,6 +200,13 @@ struct SavedData { starred_items: HashSet, #[serde(default)] last_session_time: Option, + /// Item IDs (same scheme as `read_items`) that exec_on_new has already fired for. + #[serde(default)] + seen_items: HashSet, + /// Feed URLs whose first successful fetch has been observed. Items from a + /// feed not in this set are seeded into `seen_items` silently. + #[serde(default)] + feeds_seeded: HashSet, } impl Default for App { @@ -187,6 +229,8 @@ impl App { read_items: HashSet::new(), starred_items: HashSet::new(), last_session_time: None, + seen_items: HashSet::new(), + feeds_seeded: HashSet::new(), }); // Seed bookmarks from default_feeds if no saved bookmarks exist @@ -222,6 +266,30 @@ impl App { let show_summary = last_session_time.is_some() && has_bookmarks; let (keybindings, kb_warnings) = crate::keybindings::build_keybindings(&config.keybindings); + let (macro_options, mo_warnings) = crate::keybindings::resolve_macro_options( + &config.macro_options.prefix, + &config.macro_options.pipe_default_stdin, + ); + let (macros, mac_warnings) = + crate::keybindings::build_macros(&config.macros, ¯o_options); + + // Pre-tokenize the exec_on_new template once at startup so we don't + // re-shlex on every feed arrival, and surface tokenization errors as a + // startup warning instead of an unexpected runtime failure later. + let mut hook_warnings: Vec = Vec::new(); + let exec_on_new_template = config + .hooks + .exec_on_new + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + .and_then(|t| match shlex::split(t) { + Some(toks) if !toks.is_empty() => Some(toks), + _ => { + hook_warnings.push(format!("exec_on_new: could not tokenize: {}", t)); + None + } + }); let mut app = Self { config, @@ -275,13 +343,25 @@ impl App { feed_tree: Vec::new(), selected_tree_item: None, keybindings, + macros, + macro_options, + awaiting_macro_key: false, + awaiting_macro_key_since: None, + seen_items: saved_data.seen_items, + feeds_seeded: saved_data.feeds_seeded, + pending_macro_steps: VecDeque::new(), + exec_on_new_template, }; app.update_dashboard(); app.rebuild_feed_tree(); - if !kb_warnings.is_empty() { - app.error = Some(format!("Keybinding config: {}", kb_warnings.join("; "))); + let mut all_warnings = kb_warnings; + all_warnings.extend(mo_warnings); + all_warnings.extend(mac_warnings); + all_warnings.extend(hook_warnings); + if !all_warnings.is_empty() { + app.error = Some(format!("Config: {}", all_warnings.join("; "))); } app @@ -354,6 +434,8 @@ impl App { read_items: HashSet::new(), starred_items: HashSet::new(), last_session_time: None, + seen_items: HashSet::new(), + feeds_seeded: HashSet::new(), }); } @@ -374,6 +456,8 @@ impl App { read_items: self.read_items.clone(), starred_items: self.starred_items.clone(), last_session_time: Some(Utc::now().to_rfc3339()), + seen_items: self.seen_items.clone(), + feeds_seeded: self.feeds_seeded.clone(), }; let json = serde_json::to_string(&saved_data)?; @@ -598,10 +682,7 @@ impl App { pub(crate) fn get_item_id(&self, feed_idx: usize, item_idx: usize) -> String { if let Some(feed) = self.feeds.get(feed_idx) { if let Some(item) = feed.items.get(item_idx) { - if let Some(link) = &item.link { - return link.clone(); - } - return format!("{}_{}", feed.url, item.title); + return make_item_id(feed, item); } } String::new() @@ -822,6 +903,16 @@ impl App { if idx < self.feeds.len() { let url = self.feeds[idx].url.clone(); + // Drop this feed's exec_on_new tracking state before removing + // the feed itself — otherwise the URL would stay in + // feeds_seeded and its item IDs would stay in seen_items + // forever, growing the persisted JSON file monotonically. + self.feeds_seeded.remove(&url); + for item in &self.feeds[idx].items { + let id = make_item_id(&self.feeds[idx], item); + self.seen_items.remove(&id); + } + // Remove from feeds self.feeds.remove(idx); @@ -1549,6 +1640,193 @@ impl App { self.error = Some("No links or images found in this article".to_string()); } } + + /// Resolve `(feed_idx, item_idx)` for the focused article in the current + /// view. Dashboard and Starred views derive the indices from their derived + /// item list rather than from `selected_feed` (which is only populated + /// once the user drills into FeedItems / FeedItemDetail). This is the + /// single source of truth for "which item is focused right now". + pub fn current_article_indices(&self) -> Option<(usize, usize)> { + match self.view { + View::Dashboard => { + let sel = self.selected_item?; + let active = self.active_dashboard_items(); + if sel >= active.len() { + return None; + } + Some(active[sel]) + } + View::FeedItems | View::FeedItemDetail => { + Some((self.selected_feed?, self.selected_item?)) + } + View::Starred => { + let sel = self.selected_item?; + let starred = self.get_starred_dashboard_items(); + if sel >= starred.len() { + return None; + } + Some(starred[sel]) + } + _ => None, + } + } + + /// Resolve the focused article based on the current view + selection. + /// Returns None if nothing is selected or the indices don't resolve. + pub fn current_article_context(&self) -> Option> { + let (feed_idx, item_idx) = self.current_article_indices()?; + let feed = self.feeds.get(feed_idx)?; + let item = feed.items.get(item_idx)?; + Some(ArticleContext { + title: &item.title, + url: item.link.as_deref(), + author: item.author.as_deref(), + formatted_date: item.formatted_date.as_deref(), + feed_title: &feed.title, + feed_url: &feed.url, + plain_text: item.plain_text.as_deref(), + }) + } + + /// Look up a macro whose trigger matches the given key event. + pub fn macro_for_key( + &self, + key: &crossterm::event::KeyEvent, + ) -> Option<&crate::keybindings::MacroBinding> { + self.macros.iter().find(|m| m.trigger.matches(key)) + } +} + +/// Build an `ArticleContext` directly from a `Feed` + `FeedItem` pair — +/// 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> { + ArticleContext { + title: &item.title, + url: item.link.as_deref(), + author: item.author.as_deref(), + formatted_date: item.formatted_date.as_deref(), + feed_title: &feed.title, + feed_url: &feed.url, + plain_text: item.plain_text.as_deref(), + } +} + +/// Build the same identifier scheme used by `read_items`/`starred_items` from +/// a feed + item that may not yet be in `App::feeds`. +pub fn make_item_id(feed: &Feed, item: &FeedItem) -> String { + if let Some(link) = &item.link { + return link.clone(); + } + format!("{}_{}", feed.url, item.title) +} + +/// Update `seen_items` and `feeds_seeded` to reflect this feed having been +/// successfully fetched. Returns the indices into `feed.items` of items +/// that are newly seen *and should fire exec_on_new*. On the first +/// successful fetch of a feed (URL not yet in `feeds_seeded`), the seen +/// set is populated silently and an empty Vec is returned. +pub fn mark_feed_seen(app: &mut App, feed: &Feed) -> Vec { + let is_first_fetch = !app.feeds_seeded.contains(&feed.url); + let mut newly_seen = Vec::new(); + for (idx, item) in feed.items.iter().enumerate() { + let id = make_item_id(feed, item); + let inserted = app.seen_items.insert(id); + if inserted && !is_first_fetch { + newly_seen.push(idx); + } + } + // Only flip the seeded bit on a fetch that actually returned items. A + // transiently-empty first fetch (server hiccup, parser edge case) would + // otherwise mark the feed as seeded with no items recorded; the next + // non-empty fetch would then treat every item as new and firehose + // exec_on_new for the entire backlog. + if is_first_fetch && !feed.items.is_empty() { + app.feeds_seeded.insert(feed.url.clone()); + } + newly_seen +} + +/// Substitute newsboat-style %X placeholders into each argv token. Untrusted +/// content is never interpreted by a shell — placeholders are inserted +/// verbatim into individual argv elements. +/// +/// Variables: %t title, %u url, %a author, %d formatted-date, +/// %f feed-title, %F feed-url, %% literal %. +pub fn expand_argv_template(template: &[String], ctx: &ArticleContext<'_>) -> Vec { + template + .iter() + .map(|tok| expand_one_token(tok, ctx)) + .collect() +} + +fn expand_one_token(tok: &str, ctx: &ArticleContext<'_>) -> String { + let mut out = String::with_capacity(tok.len()); + let mut chars = tok.chars().peekable(); + while let Some(c) = chars.next() { + if c != '%' { + out.push(c); + continue; + } + match chars.next() { + Some('%') => out.push('%'), + Some('t') => out.push_str(ctx.title), + Some('u') => out.push_str(ctx.url.unwrap_or("")), + Some('a') => out.push_str(ctx.author.unwrap_or("")), + 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(other) => { + out.push('%'); + out.push(other); + } + None => out.push('%'), + } + } + out +} + +/// Build the byte payload that gets piped to a `pipe-to` child's stdin. +pub fn make_pipe_payload(ctx: &ArticleContext<'_>, kind: crate::keybindings::StdinKind) -> Vec { + use crate::keybindings::StdinKind; + match kind { + StdinKind::None => Vec::new(), + StdinKind::Title => ctx.title.as_bytes().to_vec(), + StdinKind::Url => ctx.url.unwrap_or("").as_bytes().to_vec(), + StdinKind::Body => ctx + .plain_text + .map(|s| s.as_bytes().to_vec()) + .unwrap_or_default(), + StdinKind::Metadata => { + let mut out = String::new(); + out.push_str("Title: "); + out.push_str(ctx.title); + out.push('\n'); + if let Some(u) = ctx.url { + out.push_str("URL: "); + out.push_str(u); + out.push('\n'); + } + if let Some(a) = ctx.author { + out.push_str("Author: "); + out.push_str(a); + out.push('\n'); + } + if let Some(d) = ctx.formatted_date { + out.push_str("Date: "); + out.push_str(d); + out.push('\n'); + } + out.push_str("Feed: "); + out.push_str(ctx.feed_title); + out.push('\n'); + out.push('\n'); + if let Some(body) = ctx.plain_text { + out.push_str(body); + } + out.into_bytes() + } + } } #[cfg(test)] @@ -2045,4 +2323,268 @@ mod tests { assert_eq!(app.extracted_links.len(), 1); assert_eq!(app.extracted_links[0].url, "https://example.com/about"); } + + fn synthetic_ctx() -> ArticleContext<'static> { + ArticleContext { + title: "Hello %u world", + url: Some("https://ex.com/a"), + author: Some("Alice"), + formatted_date: Some("2 hours ago"), + feed_title: "Example", + feed_url: "https://ex.com/feed.xml", + plain_text: Some("body text"), + } + } + + #[test] + fn test_expand_argv_template_per_token() { + let tmpl = vec![ + "yt-dlp".to_string(), + "%u".to_string(), + "--title=%t".to_string(), + ]; + let argv = expand_argv_template(&tmpl, &synthetic_ctx()); + assert_eq!(argv[0], "yt-dlp"); + // The url replaces %u literally — but the title contains a %u which + // must NOT be re-expanded (we only expand once, in the template, not + // in the substituted content). + assert_eq!(argv[1], "https://ex.com/a"); + assert_eq!(argv[2], "--title=Hello %u world"); + } + + #[test] + fn test_expand_argv_template_percent_literal_and_unknown() { + let tmpl = vec!["a%%b".to_string(), "x%zy".to_string()]; + let argv = expand_argv_template(&tmpl, &synthetic_ctx()); + assert_eq!(argv[0], "a%b"); + // Unknown escapes are passed through verbatim. + assert_eq!(argv[1], "x%zy"); + } + + #[test] + fn test_make_pipe_payload_kinds() { + use crate::keybindings::StdinKind; + let ctx = synthetic_ctx(); + assert_eq!(make_pipe_payload(&ctx, StdinKind::None), Vec::::new()); + assert_eq!(make_pipe_payload(&ctx, StdinKind::Title), b"Hello %u world"); + assert_eq!(make_pipe_payload(&ctx, StdinKind::Url), b"https://ex.com/a"); + assert_eq!(make_pipe_payload(&ctx, StdinKind::Body), b"body text"); + let metadata = make_pipe_payload(&ctx, StdinKind::Metadata); + let s = String::from_utf8(metadata).unwrap(); + assert!(s.contains("Title: Hello %u world")); + assert!(s.contains("URL: https://ex.com/a")); + assert!(s.contains("Author: Alice")); + assert!(s.contains("Feed: Example")); + assert!(s.ends_with("body text")); + } + + fn synthetic_feed(url: &str, item_titles: &[&str]) -> Feed { + let items = item_titles + .iter() + .map(|t| FeedItem { + title: t.to_string(), + link: Some(format!("https://ex.com/a/{}", t)), + description: None, + pub_date: None, + author: None, + formatted_date: None, + parsed_date: None, + plain_text: None, + title_lower: t.to_lowercase(), + }) + .collect(); + Feed { + url: url.to_string(), + title: "Example".to_string(), + items, + title_lower: "example".to_string(), + } + } + + #[test] + fn test_mark_feed_seen_first_fetch_seeds_silently() { + let mut app = App::new(); + let feed = synthetic_feed("https://ex.com/feed.xml", &["A", "B", "C"]); + + let newly = mark_feed_seen(&mut app, &feed); + assert!( + newly.is_empty(), + "first fetch must return no newly-seen items" + ); + assert_eq!(app.seen_items.len(), 3); + assert!(app.feeds_seeded.contains("https://ex.com/feed.xml")); + } + + #[test] + fn test_mark_feed_seen_second_fetch_returns_only_new() { + let mut app = App::new(); + let feed_v1 = synthetic_feed("https://ex.com/feed.xml", &["A", "B"]); + mark_feed_seen(&mut app, &feed_v1); + + let feed_v2 = synthetic_feed("https://ex.com/feed.xml", &["A", "B", "C", "D"]); + let newly = mark_feed_seen(&mut app, &feed_v2); + // Indices 2 and 3 (C, D) are new; A and B are already seen. + assert_eq!(newly, vec![2, 3]); + assert_eq!(app.seen_items.len(), 4); + } + + #[test] + fn test_mark_feed_seen_no_changes_returns_empty() { + let mut app = App::new(); + let feed = synthetic_feed("https://ex.com/feed.xml", &["A", "B"]); + mark_feed_seen(&mut app, &feed); + let newly = mark_feed_seen(&mut app, &feed); + assert!(newly.is_empty()); + } + + #[test] + fn test_mark_feed_seen_distinct_feeds_independent() { + let mut app = App::new(); + let feed_a = synthetic_feed("https://ex.com/a.xml", &["A1"]); + let feed_b = synthetic_feed("https://ex.com/b.xml", &["B1"]); + // First fetches both seed silently. + assert!(mark_feed_seen(&mut app, &feed_a).is_empty()); + assert!(mark_feed_seen(&mut app, &feed_b).is_empty()); + assert!(app.feeds_seeded.contains("https://ex.com/a.xml")); + assert!(app.feeds_seeded.contains("https://ex.com/b.xml")); + } + + #[test] + fn test_mark_feed_seen_empty_first_fetch_does_not_seed() { + // Regression: a transiently-empty first fetch must not flip + // feeds_seeded. If it did, the next non-empty fetch would be treated + // as "subsequent" and fire exec_on_new for every backlog item — the + // exact firehose the first-fetch suppression exists to prevent. + let mut app = App::new(); + let empty = synthetic_feed("https://ex.com/feed.xml", &[]); + + let newly = mark_feed_seen(&mut app, &empty); + assert!(newly.is_empty()); + assert!( + !app.feeds_seeded.contains("https://ex.com/feed.xml"), + "empty fetch must NOT mark feed as seeded" + ); + + // The next fetch — now with items — is still the first real fetch + // and must seed silently. + let with_items = synthetic_feed("https://ex.com/feed.xml", &["A", "B", "C"]); + let newly = mark_feed_seen(&mut app, &with_items); + assert!( + newly.is_empty(), + "first non-empty fetch must seed silently, not fire" + ); + assert!(app.feeds_seeded.contains("https://ex.com/feed.xml")); + assert_eq!(app.seen_items.len(), 3); + } + + #[test] + fn test_remove_current_feed_cleans_up_tracking_state() { + // Regression: feed removal used to leave the URL in feeds_seeded and + // every item ID in seen_items forever, growing the persisted JSON + // file monotonically across feed churn. + let mut app = App::new(); + let feed = synthetic_feed("https://ex.com/feed.xml", &["A", "B"]); + let item_ids: Vec = feed.items.iter().map(|i| make_item_id(&feed, i)).collect(); + + // Seed the tracking state as if exec_on_new had run for this feed. + mark_feed_seen(&mut app, &feed); + let second = synthetic_feed("https://ex.com/feed.xml", &["A", "B", "C"]); + mark_feed_seen(&mut app, &second); + assert!(app.feeds_seeded.contains("https://ex.com/feed.xml")); + for id in &item_ids { + assert!(app.seen_items.contains(id), "precondition: {} seen", id); + } + + // Install the feed and trigger removal via the public path. + app.bookmarks.push("https://ex.com/feed.xml".to_string()); + app.feeds.push(second); + app.selected_feed = Some(0); + app.remove_current_feed().unwrap(); + + assert!( + !app.feeds_seeded.contains("https://ex.com/feed.xml"), + "URL must be dropped from feeds_seeded" + ); + for id in &item_ids { + assert!( + !app.seen_items.contains(id), + "item id {} must be dropped from seen_items", + id + ); + } + } + + #[test] + fn test_make_item_id_prefers_link() { + let mut feed = Feed { + url: "https://ex.com/feed.xml".to_string(), + title: "Example".to_string(), + items: vec![], + title_lower: "example".to_string(), + }; + let item = FeedItem { + title: "T".to_string(), + link: Some("https://ex.com/a".to_string()), + description: None, + pub_date: None, + author: None, + formatted_date: None, + parsed_date: None, + plain_text: None, + title_lower: "t".to_string(), + }; + feed.items.push(item.clone()); + assert_eq!(make_item_id(&feed, &item), "https://ex.com/a"); + let mut item2 = item.clone(); + item2.link = None; + item2.title = "T2".to_string(); + assert_eq!(make_item_id(&feed, &item2), "https://ex.com/feed.xml_T2"); + } + + #[test] + fn test_saved_data_seen_items_feeds_seeded_roundtrip() { + // Verify that the new persistence fields survive a JSON round-trip + // with the same shape the App expects. + let mut seen_items = HashSet::new(); + seen_items.insert("https://ex.com/a".to_string()); + seen_items.insert("https://ex.com/feed.xml_T2".to_string()); + let mut feeds_seeded = HashSet::new(); + feeds_seeded.insert("https://ex.com/feed.xml".to_string()); + + let original = SavedData { + bookmarks: vec!["https://ex.com/feed.xml".to_string()], + categories: vec![], + read_items: HashSet::new(), + starred_items: HashSet::new(), + last_session_time: None, + seen_items: seen_items.clone(), + feeds_seeded: feeds_seeded.clone(), + }; + + let json = serde_json::to_string(&original).unwrap(); + let parsed: SavedData = serde_json::from_str(&json).unwrap(); + + assert_eq!(parsed.seen_items, seen_items); + assert_eq!(parsed.feeds_seeded, feeds_seeded); + } + + #[test] + fn test_saved_data_back_compat_without_new_fields() { + // An older feedr_data.json with no seen_items / feeds_seeded must + // still deserialize (defaults to empty sets), so users who upgrade + // mid-flight don't lose their bookmarks. + let json = r#"{ + "bookmarks": ["https://ex.com/feed.xml"], + "categories": [], + "read_items": [], + "starred_items": [] + }"#; + let parsed: SavedData = serde_json::from_str(json).unwrap(); + assert_eq!( + parsed.bookmarks, + vec!["https://ex.com/feed.xml".to_string()] + ); + assert!(parsed.seen_items.is_empty()); + assert!(parsed.feeds_seeded.is_empty()); + } } diff --git a/src/config.rs b/src/config.rs index 1e8259d..4dd1bf5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -18,6 +18,46 @@ pub struct Config { pub default_feeds: Vec, #[serde(default, skip_serializing_if = "HashMap::is_empty")] pub keybindings: HashMap, + #[serde(default)] + pub hooks: HooksConfig, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub macros: HashMap, + #[serde(default)] + pub macro_options: MacroOptionsConfig, +} + +#[derive(Clone, Debug, Default, Serialize, Deserialize)] +pub struct HooksConfig { + /// Shell-tokenized command template fired once per newly-seen item after a refresh. + /// Variables: %t title, %u url, %a author, %d formatted-date, %f feed-title, %F feed-url. + /// First-ever fetch of a feed is seeded silently (no firehose). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub exec_on_new: Option, +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct MacroOptionsConfig { + #[serde(default = "default_macro_prefix")] + pub prefix: String, + #[serde(default = "default_pipe_stdin")] + pub pipe_default_stdin: String, +} + +fn default_macro_prefix() -> String { + ",".to_string() +} + +fn default_pipe_stdin() -> String { + "body".to_string() +} + +impl Default for MacroOptionsConfig { + fn default() -> Self { + Self { + prefix: default_macro_prefix(), + pipe_default_stdin: default_pipe_stdin(), + } + } } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -364,7 +404,58 @@ impl Config { # [[default_feeds]]\n\ # url = \"https://private.example.com/feed.xml\"\n\ # [default_feeds.headers]\n\ - # Authorization = \"Bearer your_token_here\"\n", + # Authorization = \"Bearer your_token_here\"\n\ + #\n\ + # ── External-command hooks ──────────────────────────────\n\ + #\n\ + # Macros bind a key (invoked as , default prefix is ',') to\n\ + # an ordered chain of steps. Steps are separated by ';'. A trailing\n\ + # ` -- \"description\"` overrides the help-overlay label.\n\ + #\n\ + # Step kinds:\n\ + # run a built-in action (see list below)\n\ + # pipe-to \"cmd %u\" [stdin=…] run `cmd %u` with article content on stdin\n\ + # exec \"cmd %u\" run `cmd %u` with no stdin\n\ + #\n\ + # Supported actions inside macros:\n\ + # open-in-browser, toggle-star, toggle-read, mark-all-read,\n\ + # refresh, toggle-theme, extract-links, help\n\ + # Other keybinding actions are intentionally not callable from macros.\n\ + #\n\ + # Variables expanded per argv token:\n\ + # %t title %u url %a author %d date %f feed-title %F feed-url %% literal %\n\ + #\n\ + # IMPORTANT — commands are NOT run through a shell:\n\ + # * Templates are tokenized once and %X is substituted into argv tokens.\n\ + # Article content cannot break out of an argument.\n\ + # * For pipes, redirection, or globbing, write a small shell script\n\ + # and invoke that.\n\ + # * `~` and `$HOME` / `$VAR` are NOT expanded — use absolute paths.\n\ + # * Wrapping your command in `sh -c \"... %t ...\"` REINTRODUCES shell\n\ + # injection through item titles. Prefer a script file.\n\ + #\n\ + # Quoting: the outer parser is shell-style, so to put a quoted token in\n\ + # the inner argv, escape twice:\n\ + # pipe-to \"echo \\\"hello world\\\"\"\n\ + #\n\ + # [macros]\n\ + # y = 'open-in-browser ; pipe-to \"yt-dlp %u\"'\n\ + # w = 'pipe-to \"wallabag-cli add %u\" -- \"Save to Wallabag\"'\n\ + # n = 'pipe-to \"tee /tmp/out.txt\" stdin=metadata'\n\ + #\n\ + # [macro_options]\n\ + # prefix = \",\" # the macro-prefix key\n\ + # pipe_default_stdin = \"body\" # body | title | url | metadata | none\n\ + #\n\ + # The exec_on_new hook fires once per newly-seen item on each refresh.\n\ + # The first successful fetch of a feed seeds the seen set silently to\n\ + # avoid a firehose on initial load. Children are spawned detached.\n\ + # Semantics on crash are AT MOST ONCE — feedr persists the seen-set\n\ + # before spawning, so a kill mid-fire loses a notification rather than\n\ + # firing again on the next launch. Prefer idempotent commands.\n\ + #\n\ + # [hooks]\n\ + # exec_on_new = 'notify-send \"New: %t\" \"%f\"'\n", toml ) } @@ -403,6 +494,47 @@ mod tests { assert_eq!(headers.get("X-Custom").unwrap(), "value"); } + #[test] + fn test_hooks_and_macros_round_trip() { + let toml_str = r#" + [hooks] + exec_on_new = 'notify-send "New: %t" "%f"' + + [macros] + y = 'open-in-browser ; pipe-to "yt-dlp %u"' + + [macro_options] + prefix = "," + pipe_default_stdin = "metadata" + "#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert_eq!( + config.hooks.exec_on_new.as_deref(), + Some(r#"notify-send "New: %t" "%f""#) + ); + assert_eq!( + config.macros.get("y").map(String::as_str), + Some(r#"open-in-browser ; pipe-to "yt-dlp %u""#) + ); + assert_eq!(config.macro_options.prefix, ","); + assert_eq!(config.macro_options.pipe_default_stdin, "metadata"); + } + + #[test] + fn test_config_back_compat_without_new_sections() { + // An old config file with no [hooks]/[macros]/[macro_options] must still load. + let toml_str = r#" + [general] + max_dashboard_items = 50 + "#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert_eq!(config.general.max_dashboard_items, 50); + assert!(config.hooks.exec_on_new.is_none()); + assert!(config.macros.is_empty()); + assert_eq!(config.macro_options.prefix, ","); + assert_eq!(config.macro_options.pipe_default_stdin, "body"); + } + #[test] fn test_config_serialization() { let config = Config::default(); diff --git a/src/events.rs b/src/events.rs index ededb91..c883cbc 100644 --- a/src/events.rs +++ b/src/events.rs @@ -14,8 +14,11 @@ // - All text input modes (InsertUrl, SearchMode, CategoryNameInput) // - Detail view: g/G and Ctrl+u/Ctrl+d for scrolling -use crate::app::{AddFeedResult, App, CategoryAction, InputMode, TimeFilter, TreeItem, View}; -use crate::keybindings::KeyAction; +use crate::app::{ + expand_argv_template, make_pipe_payload, AddFeedResult, App, CategoryAction, InputMode, + TimeFilter, TreeItem, View, +}; +use crate::keybindings::{KeyAction, MacroBinding, StdinKind}; use anyhow::Result; use crossterm::event::{ self, Event, KeyCode, KeyEventKind, KeyModifiers, MouseButton, MouseEvent, MouseEventKind, @@ -50,41 +53,120 @@ fn handle_show_help(app: &mut App) { } fn handle_toggle_star_current(app: &mut App) { - if let Some(feed_idx) = app.selected_feed { - if let Some(item_idx) = app.selected_item { - match app.toggle_item_starred(feed_idx, item_idx) { - Ok(is_now_starred) => { - app.success_message = Some(if is_now_starred { - "\u{2605} Starred".to_string() - } else { - "\u{2606} Unstarred".to_string() - }); - app.success_message_time = Some(std::time::Instant::now()); - } - Err(e) => { - app.error = Some(format!("Failed to toggle star: {}", e)); - } - } + let Some((feed_idx, item_idx)) = app.current_article_indices() else { + return; + }; + match app.toggle_item_starred(feed_idx, item_idx) { + Ok(is_now_starred) => { + app.success_message = Some(if is_now_starred { + "\u{2605} Starred".to_string() + } else { + "\u{2606} Unstarred".to_string() + }); + app.success_message_time = Some(std::time::Instant::now()); + } + Err(e) => { + app.error = Some(format!("Failed to toggle star: {}", e)); } } } fn handle_toggle_read_current(app: &mut App) { - if let Some(feed_idx) = app.selected_feed { - if let Some(item_idx) = app.selected_item { - match app.toggle_item_read(feed_idx, item_idx) { - Ok(is_now_read) => { - app.success_message = Some(if is_now_read { - "\u{2713} Marked as read".to_string() - } else { - "\u{25CB} Marked as unread".to_string() - }); - app.success_message_time = Some(std::time::Instant::now()); - } - Err(e) => { - app.error = Some(format!("Failed to toggle read status: {}", e)); + let Some((feed_idx, item_idx)) = app.current_article_indices() else { + return; + }; + match app.toggle_item_read(feed_idx, item_idx) { + Ok(is_now_read) => { + app.success_message = Some(if is_now_read { + "\u{2713} Marked as read".to_string() + } else { + "\u{25CB} Marked as unread".to_string() + }); + app.success_message_time = Some(std::time::Instant::now()); + } + Err(e) => { + app.error = Some(format!("Failed to toggle read status: {}", e)); + } + } + // Re-filter so a `read_status = Some(false)` ("unread only") filter on the + // Dashboard drops the just-read item from the visible list. The Dashboard + // keypress used to do this inline; centralizing it here means macros and + // every other caller stay in sync. + app.apply_filters(); +} + +// ── Macro engine ─────────────────────────────────────────────────── +// +// Macros are checked at the top of `handle_key_event` and bypass the +// per-view dispatch entirely. ALL steps (Action / PipeTo / Exec) are +// queued onto `app.pending_macro_steps` so they run in newsboat-style +// sequential order. The TUI loop drains the queue after each +// event-handling pass — actions mutate state inline, PipeTo suspends +// the terminal, Exec spawns detached. + +/// Queue every step of a macro for the TUI drain in order. +fn run_macro(app: &mut App, mac: &MacroBinding) { + for step in &mac.steps { + app.pending_macro_steps.push_back(step.clone()); + } +} + +/// Build the (argv, stdin payload) tuple for a queued PipeTo step using +/// the current article context. Returns None if no article is focused. +pub(crate) fn build_pipe_invocation( + app: &App, + argv_template: &[String], + stdin: StdinKind, +) -> Option<(Vec, Vec)> { + let ctx = app.current_article_context()?; + let argv = expand_argv_template(argv_template, &ctx); + let payload = make_pipe_payload(&ctx, stdin); + Some((argv, payload)) +} + +/// Build the argv for a queued Exec step using the current article context. +/// Returns None if no article is focused. +pub(crate) fn build_exec_invocation(app: &App, argv_template: &[String]) -> Option> { + let ctx = app.current_article_context()?; + Some(expand_argv_template(argv_template, &ctx)) +} + +/// Run a `KeyAction` outside of normal key dispatch (called from the +/// macro queue drain). Only a curated subset is supported — actions whose +/// meaning is well-defined without view-specific input mode context. +pub(crate) fn dispatch_action(app: &mut App, action: KeyAction) { + match action { + KeyAction::Refresh => handle_refresh(app), + KeyAction::ToggleTheme => handle_toggle_theme(app), + KeyAction::Help => handle_show_help(app), + KeyAction::ToggleStar => handle_toggle_star_current(app), + KeyAction::ToggleRead => handle_toggle_read_current(app), + KeyAction::OpenInBrowser => match app.current_article_context() { + Some(ctx) => { + if let Some(url) = ctx.url { + if let Err(e) = open::that(url) { + app.error = Some(format!("Failed to open link: {}", e)); + } + } else { + app.error = Some("No URL on this article".to_string()); } } + None => app.error = Some("open-in-browser: no article in focus".to_string()), + }, + KeyAction::MarkAllRead => match app.mark_all_dashboard_read() { + Ok(count) => { + app.success_message = Some(format!("\u{2713} Marked {} items as read", count)); + app.success_message_time = Some(std::time::Instant::now()); + app.apply_filters(); + } + Err(e) => app.error = Some(format!("Failed to mark all read: {}", e)), + }, + KeyAction::ExtractLinks => app.extract_links_from_current_item(), + other => { + app.error = Some(format!( + "macro: action '{}' not supported in macros", + other.as_str() + )); } } } @@ -164,6 +246,42 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - if app.key_matches(KeyAction::ForceQuit, &key) { return Ok(true); } + + // Macro engine: handle the macro-prefix follow-up key, then check whether + // this key IS the macro prefix. Only active in Normal input mode so it + // never interferes with text-input or filter-cycling modes. + if app.input_mode == InputMode::Normal { + if app.awaiting_macro_key { + app.awaiting_macro_key = false; + app.awaiting_macro_key_since = None; + // Clear the "Macro prefix..." hint so it doesn't outlive the wait. + app.success_message = None; + app.success_message_time = None; + // ESC explicitly cancels; the key is consumed and nothing else fires. + if key.code == KeyCode::Esc { + return Ok(false); + } + match app.macro_for_key(&key) { + Some(mac) => { + let mac = mac.clone(); + run_macro(app, &mac); + } + None => { + app.error = Some("No macro bound to that key".to_string()); + } + } + return Ok(false); + } + if app.macro_options.prefix.matches(&key) && !app.macros.is_empty() { + let now = std::time::Instant::now(); + app.awaiting_macro_key = true; + app.awaiting_macro_key_since = Some(now); + app.success_message = Some("Macro prefix... press macro key".to_string()); + app.success_message_time = Some(now); + return Ok(false); + } + } + match app.input_mode { InputMode::Normal => match app.view { View::Dashboard => match key.code { @@ -1815,4 +1933,180 @@ mod tests { assert!(!result); assert_eq!(app.view, View::FeedList); } + + // ── Macro engine tests ──────────────────────────────────────── + + #[test] + fn test_dispatch_action_rejects_unsupported_action() { + let mut app = make_test_app(); + // MoveUp is intentionally not callable from macros — verify the error + // surfaces the friendly dashed name, not a debug-formatted variant. + dispatch_action(&mut app, KeyAction::MoveUp); + let err = app.error.as_deref().unwrap_or(""); + assert!(err.contains("move-up"), "got error: {}", err); + assert!(err.contains("not supported"), "got error: {}", err); + } + + #[test] + fn test_macro_prefix_esc_cancels_cleanly() { + let mut app = make_test_app(); + // Bind a macro so the prefix is active. + app.macros = vec![crate::keybindings::MacroBinding { + trigger: crate::keybindings::KeyBinding::new(KeyCode::Char('y')), + steps: vec![crate::keybindings::MacroStep::Action(KeyAction::Refresh)], + description: None, + }]; + + // Press the prefix. + let prefix = make_key(KeyCode::Char(','), KeyModifiers::NONE); + handle_key_event(&mut app, prefix).unwrap(); + assert!(app.awaiting_macro_key); + + // ESC must cancel without firing anything and without surfacing an error. + let esc = make_key(KeyCode::Esc, KeyModifiers::NONE); + handle_key_event(&mut app, esc).unwrap(); + assert!(!app.awaiting_macro_key); + assert!(app.awaiting_macro_key_since.is_none()); + assert!( + app.error.is_none(), + "ESC must not produce an error, got: {:?}", + app.error + ); + assert!(app.pending_macro_steps.is_empty()); + } + + #[test] + fn test_macro_prefix_unknown_followup_errors_but_clears_state() { + let mut app = make_test_app(); + app.macros = vec![crate::keybindings::MacroBinding { + trigger: crate::keybindings::KeyBinding::new(KeyCode::Char('y')), + steps: vec![crate::keybindings::MacroStep::Action(KeyAction::Refresh)], + description: None, + }]; + + let prefix = make_key(KeyCode::Char(','), KeyModifiers::NONE); + handle_key_event(&mut app, prefix).unwrap(); + // Press an unbound follow-up key. We surface an error rather than + // silently falling through to normal dispatch, but the prefix state + // must be cleared so subsequent keystrokes work normally. + let z = make_key(KeyCode::Char('z'), KeyModifiers::NONE); + handle_key_event(&mut app, z).unwrap(); + assert!(!app.awaiting_macro_key); + assert!(app.awaiting_macro_key_since.is_none()); + assert!(app.error.is_some()); + } + + #[test] + fn test_macro_prefix_then_bound_key_queues_steps() { + let mut app = make_test_app(); + app.macros = vec![crate::keybindings::MacroBinding { + trigger: crate::keybindings::KeyBinding::new(KeyCode::Char('y')), + steps: vec![ + crate::keybindings::MacroStep::Action(KeyAction::ToggleStar), + crate::keybindings::MacroStep::Exec { + argv_template: vec!["true".to_string()], + }, + ], + description: None, + }]; + + let prefix = make_key(KeyCode::Char(','), KeyModifiers::NONE); + handle_key_event(&mut app, prefix).unwrap(); + let y = make_key(KeyCode::Char('y'), KeyModifiers::NONE); + handle_key_event(&mut app, y).unwrap(); + assert!(!app.awaiting_macro_key); + assert_eq!(app.pending_macro_steps.len(), 2); + } + + #[test] + fn test_dispatch_toggle_star_works_in_dashboard_view() { + // Regression: dispatch_action used to read app.selected_feed, which is + // None in Dashboard view until the user drills into an item. The macro + // path silently no-op'd. After the fix, the focused dashboard item is + // resolved via current_article_indices() and toggling works. + let mut app = make_test_app(); + app.read_items.clear(); + app.starred_items.clear(); + app.view = View::Dashboard; + app.selected_item = Some(0); + assert!(app.selected_feed.is_none(), "precondition for regression"); + assert!( + !app.active_dashboard_items().is_empty(), + "make_test_app populates the dashboard" + ); + + dispatch_action(&mut app, KeyAction::ToggleStar); + + assert_eq!( + app.starred_items.len(), + 1, + "expected item to be starred via dispatch_action" + ); + assert!(app.error.is_none(), "should not error: {:?}", app.error); + } + + #[test] + fn test_dispatch_toggle_read_works_in_starred_view() { + // Regression: same root cause as the Dashboard case — Starred view + // also leaves selected_feed unset. + let mut app = make_test_app(); + app.read_items.clear(); + app.starred_items.clear(); + // Star one item so the Starred view has something to focus on. + app.toggle_item_starred(0, 0).unwrap(); + app.view = View::Starred; + app.selected_item = Some(0); + app.selected_feed = None; + assert_eq!( + app.get_starred_dashboard_items().len(), + 1, + "Starred view has one item to focus" + ); + + dispatch_action(&mut app, KeyAction::ToggleRead); + + assert_eq!( + app.read_items.len(), + 1, + "expected item to be marked read via dispatch_action" + ); + assert!(app.error.is_none(), "should not error: {:?}", app.error); + } + + #[test] + fn test_dispatch_toggle_read_reapplies_filters_on_dashboard() { + // Regression: a `,r = toggle-read` macro on the Dashboard with the + // "unread only" filter active must remove the just-read item from the + // visible list. Previously dispatch_action -> handle_toggle_read_current + // skipped apply_filters(), so the dashboard kept showing stale state + // until the next filter change. + let mut app = make_test_app(); + app.read_items.clear(); + app.starred_items.clear(); + app.view = View::Dashboard; + + // Activate "unread only" filter, then materialize the filtered list. + app.filter_options.read_status = Some(false); + app.apply_filters(); + let before = app.active_dashboard_items().len(); + assert!( + before > 0, + "precondition: dashboard has unread items to filter on" + ); + app.selected_item = Some(0); + + dispatch_action(&mut app, KeyAction::ToggleRead); + + assert_eq!( + app.read_items.len(), + 1, + "item should be marked read via dispatch_action" + ); + assert_eq!( + app.active_dashboard_items().len(), + before - 1, + "the just-read item must drop out of the unread-only filter" + ); + assert!(app.error.is_none(), "should not error: {:?}", app.error); + } } diff --git a/src/keybindings.rs b/src/keybindings.rs index d360b67..30aeafa 100644 --- a/src/keybindings.rs +++ b/src/keybindings.rs @@ -2,6 +2,69 @@ use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use std::collections::HashMap; use std::str::FromStr; +/// What gets piped to stdin when a `pipe-to` macro step runs. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum StdinKind { + None, + #[default] + Body, + Title, + Url, + Metadata, +} + +impl FromStr for StdinKind { + type Err = (); + fn from_str(s: &str) -> Result { + match s.to_ascii_lowercase().as_str() { + "none" => Ok(Self::None), + "body" => Ok(Self::Body), + "title" => Ok(Self::Title), + "url" => Ok(Self::Url), + "metadata" => Ok(Self::Metadata), + _ => Err(()), + } + } +} + +/// A single step inside a macro chain. +#[derive(Clone, Debug)] +pub enum MacroStep { + /// Invoke an existing keybinding action (e.g. open-in-browser, toggle-star). + Action(KeyAction), + /// Run argv with article content piped to stdin. + PipeTo { + argv_template: Vec, + stdin: StdinKind, + }, + /// Run argv with no stdin. + Exec { argv_template: Vec }, +} + +/// A macro: a key trigger + an ordered sequence of steps + optional description. +#[derive(Clone, Debug)] +pub struct MacroBinding { + pub trigger: KeyBinding, + pub steps: Vec, + pub description: Option, +} + +/// Top-level options for the macro engine. +#[derive(Clone, Debug)] +pub struct MacroOptions { + pub prefix: KeyBinding, + pub pipe_default_stdin: StdinKind, +} + +impl Default for MacroOptions { + fn default() -> Self { + Self { + prefix: KeyBinding::new(KeyCode::Char(',')), + pipe_default_stdin: StdinKind::Body, + } + } +} + #[derive(Clone, Debug, Hash, Eq, PartialEq)] pub enum KeyAction { // Global @@ -45,6 +108,47 @@ pub enum KeyAction { PrevTab, } +impl KeyAction { + /// Return the dashed lowercase name a user would write in their config + /// (the inverse of `FromStr`, modulo `-` vs `_`). + pub fn as_str(&self) -> &'static str { + match self { + Self::Quit => "quit", + Self::ForceQuit => "force-quit", + Self::Back => "back", + Self::Home => "home", + Self::ToggleTheme => "toggle-theme", + Self::Refresh => "refresh", + Self::Help => "help", + Self::OpenSearch => "open-search", + Self::MoveUp => "move-up", + Self::MoveDown => "move-down", + Self::PageUp => "page-up", + Self::PageDown => "page-down", + Self::JumpTop => "jump-top", + Self::JumpBottom => "jump-bottom", + Self::Select => "select", + Self::AddFeed => "add-feed", + Self::DeleteFeed => "delete-feed", + Self::ToggleRead => "toggle-read", + Self::ToggleStar => "toggle-star", + Self::MarkAllRead => "mark-all-read", + Self::OpenInBrowser => "open-in-browser", + Self::TogglePreview => "toggle-preview", + Self::OpenFilter => "open-filter", + Self::CycleCategory => "cycle-category", + Self::OpenCategoryManagement => "open-category-management", + Self::AssignCategory => "assign-category", + Self::ExtractLinks => "extract-links", + Self::ScrollPreviewUp => "scroll-preview-up", + Self::ScrollPreviewDown => "scroll-preview-down", + Self::ToggleExpand => "toggle-expand", + Self::NextTab => "next-tab", + Self::PrevTab => "prev-tab", + } + } +} + impl FromStr for KeyAction { type Err = (); @@ -387,43 +491,279 @@ pub fn build_keybindings( (map, warnings) } -/// Get display string for the first binding of an action -pub fn key_display(action: &KeyAction, map: &KeyBindingMap) -> String { - if let Some(bindings) = map.get(action) { - if let Some(binding) = bindings.first() { - let mut parts = Vec::new(); - if binding.modifiers.contains(KeyModifiers::CONTROL) { - parts.push("Ctrl".to_string()); +// ── Macro parsing ───────────────────────────────────────────────── +// +// Newsboat-compatible string syntax: +// ; [args]; ... [-- ""] +// +// Examples: +// open-in-browser ; pipe-to "yt-dlp %u" +// pipe-to "wallabag-cli add %u" -- "Save to Wallabag" +// pipe-to "tee out.txt" stdin=metadata +// +// Recognized op names mirror existing KeyAction names but use dashes +// (newsboat convention), e.g. `open-in-browser` <-> `open_in_browser`. +// Two synthetic ops, `pipe-to` and `exec`, take a quoted command template +// followed by optional `key=value` modifiers (currently only `stdin=`). + +/// Split a string on a single ASCII separator, ignoring occurrences inside +/// double-quoted spans. Backslash escapes the next character. +fn split_top_level(s: &str, sep: u8) -> Vec { + let bytes = s.as_bytes(); + let mut out = Vec::new(); + let mut start = 0; + let mut in_quote = false; + let mut escape = false; + let mut i = 0; + while i < bytes.len() { + let b = bytes[i]; + if escape { + escape = false; + i += 1; + continue; + } + if b == b'\\' { + escape = true; + i += 1; + continue; + } + if b == b'"' { + in_quote = !in_quote; + i += 1; + continue; + } + if !in_quote && b == sep { + out.push(s[start..i].to_string()); + start = i + 1; + } + i += 1; + } + out.push(s[start..].to_string()); + out +} + +/// Detect a trailing ` -- "description"` outside double-quoted spans. +fn split_description(s: &str) -> (&str, Option) { + let bytes = s.as_bytes(); + let mut in_quote = false; + let mut escape = false; + let mut i = 0; + while i < bytes.len() { + let b = bytes[i]; + if escape { + escape = false; + i += 1; + continue; + } + if b == b'\\' { + escape = true; + i += 1; + continue; + } + if b == b'"' { + in_quote = !in_quote; + i += 1; + continue; + } + // Look for the literal byte sequence ` -- ` (space-dash-dash-space) + // outside quoted spans. `b == b' '` is the leading space; the rest of + // the separator is matched with a single slice compare. + if !in_quote && b == b' ' && bytes.get(i + 1..=i + 3) == Some(b"-- ") { + // i and i+4 are ASCII boundaries; safe to slice. + let body = &s[..i]; + let desc_part = s[i + 4..].trim(); + let unquoted = desc_part + .strip_prefix('"') + .and_then(|d| d.strip_suffix('"')) + .unwrap_or(desc_part); + let desc = unquoted.to_string(); + return (body, if desc.is_empty() { None } else { Some(desc) }); + } + i += 1; + } + (s, None) +} + +fn split_op_and_rest(step: &str) -> Option<(&str, &str)> { + let s = step.trim(); + if s.is_empty() { + return None; + } + match s.find(char::is_whitespace) { + Some(idx) => Some((&s[..idx], s[idx..].trim_start())), + None => Some((s, "")), + } +} + +fn parse_pipe_or_exec_args(rest: &str) -> Result<(Vec, Vec), String> { + // shlex::split returns None on unbalanced quotes + let tokens = shlex::split(rest).ok_or_else(|| format!("could not tokenize: {}", rest))?; + if tokens.is_empty() { + return Err("missing command argument".into()); + } + let argv = shlex::split(&tokens[0]) + .ok_or_else(|| format!("could not tokenize command: {}", tokens[0]))?; + if argv.is_empty() { + return Err("command is empty".into()); + } + let modifiers = tokens[1..].to_vec(); + Ok((argv, modifiers)) +} + +/// Parse one macro definition string into a sequence of steps and +/// an optional human description. +pub fn parse_macro_string( + raw: &str, + default_stdin: StdinKind, +) -> Result<(Vec, Option), String> { + let (body, description) = split_description(raw); + let parts = split_top_level(body, b';'); + let mut steps = Vec::new(); + for part in parts { + let part = part.trim(); + if part.is_empty() { + continue; + } + let (op, rest) = split_op_and_rest(part).ok_or_else(|| "empty step".to_string())?; + match op { + "pipe-to" | "pipe_to" => { + let (argv_template, modifiers) = + parse_pipe_or_exec_args(rest).map_err(|e| format!("pipe-to: {}", e))?; + let mut stdin = default_stdin; + for m in modifiers { + let (k, v) = m + .split_once('=') + .ok_or_else(|| format!("pipe-to: unknown modifier '{}'", m))?; + match k { + "stdin" => { + stdin = StdinKind::from_str(v) + .map_err(|_| format!("pipe-to: invalid stdin kind '{}'", v))?; + } + _ => return Err(format!("pipe-to: unknown modifier '{}'", k)), + } + } + steps.push(MacroStep::PipeTo { + argv_template, + stdin, + }); + } + "exec" => { + let (argv_template, modifiers) = + parse_pipe_or_exec_args(rest).map_err(|e| format!("exec: {}", e))?; + if !modifiers.is_empty() { + return Err(format!("exec takes no modifiers, got: {:?}", modifiers)); + } + steps.push(MacroStep::Exec { argv_template }); } - if binding.modifiers.contains(KeyModifiers::SHIFT) { - parts.push("Shift".to_string()); + _ => { + let normalized = op.replace('-', "_"); + let action = KeyAction::from_str(&normalized) + .map_err(|_| format!("unknown action '{}'", op))?; + if !rest.is_empty() { + return Err(format!("action '{}' takes no arguments, got: {}", op, rest)); + } + steps.push(MacroStep::Action(action)); } - if binding.modifiers.contains(KeyModifiers::ALT) { - parts.push("Alt".to_string()); + } + } + if steps.is_empty() { + return Err("macro has no steps".into()); + } + Ok((steps, description)) +} + +/// Build the parsed macro list from a config map of `key -> definition string`. +/// Returns the bindings and a list of warnings for invalid entries. +pub fn build_macros( + config_macros: &HashMap, + options: &MacroOptions, +) -> (Vec, Vec) { + let mut out = Vec::new(); + let mut warnings = Vec::new(); + let mut keys: Vec<&String> = config_macros.keys().collect(); + keys.sort(); + for key in keys { + let body = &config_macros[key]; + let trigger = match parse_key_string(key) { + Some(b) => b, + None => { + warnings.push(format!("could not parse macro key '{}'", key)); + continue; } - let key_name = match binding.code { - KeyCode::Char(' ') => "Space".to_string(), - KeyCode::Char(c) => c.to_string(), - KeyCode::Enter => "Enter".to_string(), - KeyCode::Esc => "Esc".to_string(), - KeyCode::Tab => "Tab".to_string(), - KeyCode::Backspace => "Backspace".to_string(), - KeyCode::Up => "\u{2191}".to_string(), - KeyCode::Down => "\u{2193}".to_string(), - KeyCode::Left => "\u{2190}".to_string(), - KeyCode::Right => "\u{2192}".to_string(), - KeyCode::Home => "Home".to_string(), - KeyCode::End => "End".to_string(), - KeyCode::PageUp => "PgUp".to_string(), - KeyCode::PageDown => "PgDn".to_string(), - KeyCode::F(n) => format!("F{}", n), - _ => "?".to_string(), - }; - parts.push(key_name); - return parts.join("+"); + }; + match parse_macro_string(body, options.pipe_default_stdin) { + Ok((steps, description)) => out.push(MacroBinding { + trigger, + steps, + description, + }), + Err(e) => warnings.push(format!("macro '{}': {}", key, e)), } } - "?".to_string() + (out, warnings) +} + +/// Resolve `MacroOptions` from raw string-typed config values. +pub fn resolve_macro_options( + prefix_str: &str, + pipe_default_stdin_str: &str, +) -> (MacroOptions, Vec) { + let mut options = MacroOptions::default(); + let mut warnings = Vec::new(); + match parse_key_string(prefix_str) { + Some(b) => options.prefix = b, + None => warnings.push(format!("invalid macro prefix '{}'", prefix_str)), + } + match StdinKind::from_str(pipe_default_stdin_str) { + Ok(k) => options.pipe_default_stdin = k, + Err(_) => warnings.push(format!( + "invalid pipe_default_stdin '{}'", + pipe_default_stdin_str + )), + } + (options, warnings) +} + +/// Format a single `KeyBinding` like `Ctrl+q`, `Shift+Tab`, `,`, `Space`. +pub fn binding_display(binding: &KeyBinding) -> String { + let mut parts = Vec::new(); + if binding.modifiers.contains(KeyModifiers::CONTROL) { + parts.push("Ctrl".to_string()); + } + if binding.modifiers.contains(KeyModifiers::SHIFT) { + parts.push("Shift".to_string()); + } + if binding.modifiers.contains(KeyModifiers::ALT) { + parts.push("Alt".to_string()); + } + let key_name = match binding.code { + KeyCode::Char(' ') => "Space".to_string(), + KeyCode::Char(c) => c.to_string(), + KeyCode::Enter => "Enter".to_string(), + KeyCode::Esc => "Esc".to_string(), + KeyCode::Tab => "Tab".to_string(), + KeyCode::Backspace => "Backspace".to_string(), + KeyCode::Up => "\u{2191}".to_string(), + KeyCode::Down => "\u{2193}".to_string(), + KeyCode::Left => "\u{2190}".to_string(), + KeyCode::Right => "\u{2192}".to_string(), + KeyCode::Home => "Home".to_string(), + KeyCode::End => "End".to_string(), + KeyCode::PageUp => "PgUp".to_string(), + KeyCode::PageDown => "PgDn".to_string(), + KeyCode::F(n) => format!("F{}", n), + _ => "?".to_string(), + }; + parts.push(key_name); + parts.join("+") +} + +/// Get display string for the first binding of an action +pub fn key_display(action: &KeyAction, map: &KeyBindingMap) -> String { + map.get(action) + .and_then(|bindings| bindings.first()) + .map(binding_display) + .unwrap_or_else(|| "?".to_string()) } #[cfg(test)] @@ -604,4 +944,190 @@ mod tests { // Multi-char key name that isn't a special key assert!(parse_key_string("abc").is_none()); } + + // ── Macro parser ────────────────────────────────────────────── + + #[test] + fn test_macro_single_action() { + let (steps, desc) = parse_macro_string("open-in-browser", StdinKind::Body).unwrap(); + assert_eq!(steps.len(), 1); + assert!(matches!( + steps[0], + MacroStep::Action(KeyAction::OpenInBrowser) + )); + assert!(desc.is_none()); + } + + #[test] + fn test_macro_chain_with_pipe() { + let (steps, desc) = + parse_macro_string(r#"open-in-browser ; pipe-to "yt-dlp %u""#, StdinKind::Body) + .unwrap(); + assert_eq!(steps.len(), 2); + assert!(matches!( + steps[0], + MacroStep::Action(KeyAction::OpenInBrowser) + )); + match &steps[1] { + MacroStep::PipeTo { + argv_template, + stdin, + } => { + assert_eq!(argv_template, &vec!["yt-dlp".to_string(), "%u".to_string()]); + assert_eq!(*stdin, StdinKind::Body); + } + other => panic!("expected PipeTo, got {:?}", other), + } + assert!(desc.is_none()); + } + + #[test] + fn test_macro_with_description() { + let (steps, desc) = + parse_macro_string(r#"toggle-star -- "Star current item""#, StdinKind::Body).unwrap(); + assert_eq!(steps.len(), 1); + assert_eq!(desc, Some("Star current item".to_string())); + } + + #[test] + fn test_macro_pipe_with_stdin_modifier() { + let (steps, _) = + parse_macro_string(r#"pipe-to "tee out.txt" stdin=url"#, StdinKind::Body).unwrap(); + match &steps[0] { + MacroStep::PipeTo { + argv_template, + stdin, + } => { + assert_eq!( + argv_template, + &vec!["tee".to_string(), "out.txt".to_string()] + ); + assert_eq!(*stdin, StdinKind::Url); + } + other => panic!("expected PipeTo, got {:?}", other), + } + } + + #[test] + fn test_macro_exec_no_stdin() { + let (steps, _) = + parse_macro_string(r#"exec "notify-send hello""#, StdinKind::Body).unwrap(); + match &steps[0] { + MacroStep::Exec { argv_template } => { + assert_eq!( + argv_template, + &vec!["notify-send".to_string(), "hello".to_string()] + ); + } + other => panic!("expected Exec, got {:?}", other), + } + } + + #[test] + fn test_macro_unknown_action_errors() { + assert!(parse_macro_string("frobnicate", StdinKind::Body).is_err()); + } + + #[test] + fn test_macro_unbalanced_quote_errors() { + assert!(parse_macro_string(r#"pipe-to "tee out.txt"#, StdinKind::Body).is_err()); + } + + #[test] + fn test_macro_empty_errors() { + assert!(parse_macro_string("", StdinKind::Body).is_err()); + assert!(parse_macro_string(" ; ; ", StdinKind::Body).is_err()); + } + + #[test] + fn test_macro_semicolon_inside_quotes_not_split() { + let (steps, _) = parse_macro_string(r#"pipe-to "echo a;b""#, StdinKind::Body).unwrap(); + assert_eq!(steps.len(), 1); + match &steps[0] { + MacroStep::PipeTo { argv_template, .. } => { + // shlex splits "echo a;b" into ["echo", "a;b"] + assert_eq!(argv_template, &vec!["echo".to_string(), "a;b".to_string()]); + } + _ => panic!(), + } + } + + #[test] + fn test_build_macros_warns_on_bad_key() { + let mut macros = HashMap::new(); + macros.insert("notakey++".to_string(), "open-in-browser".to_string()); + let (out, warnings) = build_macros(¯os, &MacroOptions::default()); + assert!(out.is_empty()); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("could not parse macro key")); + } + + #[test] + fn test_build_macros_warns_on_bad_body() { + let mut macros = HashMap::new(); + macros.insert("y".to_string(), "frobnicate".to_string()); + let (out, warnings) = build_macros(¯os, &MacroOptions::default()); + assert!(out.is_empty()); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("unknown action")); + } + + #[test] + fn test_resolve_macro_options() { + let (opts, warnings) = resolve_macro_options(",", "metadata"); + assert!(warnings.is_empty()); + assert_eq!(opts.pipe_default_stdin, StdinKind::Metadata); + + let (_, warnings) = resolve_macro_options("notakey++", "body"); + assert!(!warnings.is_empty()); + } + + #[test] + fn test_keyaction_as_str_roundtrip_with_fromstr() { + // Every KeyAction must round-trip through as_str() -> "-"→"_" -> from_str(). + // If you add a new KeyAction variant, add it to both maps. + let all = [ + KeyAction::Quit, + KeyAction::ForceQuit, + KeyAction::Back, + KeyAction::Home, + KeyAction::ToggleTheme, + KeyAction::Refresh, + KeyAction::Help, + KeyAction::OpenSearch, + KeyAction::MoveUp, + KeyAction::MoveDown, + KeyAction::PageUp, + KeyAction::PageDown, + KeyAction::JumpTop, + KeyAction::JumpBottom, + KeyAction::Select, + KeyAction::AddFeed, + KeyAction::DeleteFeed, + KeyAction::ToggleRead, + KeyAction::ToggleStar, + KeyAction::MarkAllRead, + KeyAction::OpenInBrowser, + KeyAction::TogglePreview, + KeyAction::OpenFilter, + KeyAction::CycleCategory, + KeyAction::OpenCategoryManagement, + KeyAction::AssignCategory, + KeyAction::ExtractLinks, + KeyAction::ScrollPreviewUp, + KeyAction::ScrollPreviewDown, + KeyAction::ToggleExpand, + KeyAction::NextTab, + KeyAction::PrevTab, + ]; + for a in all { + let s = a.as_str(); + // Display form uses dashes for readability. + assert!(!s.contains('_'), "as_str() should use dashes, got: {}", s); + let parsed = KeyAction::from_str(&s.replace('-', "_")).unwrap_or_else(|_| { + panic!("could not round-trip KeyAction::{:?} via as_str()={}", a, s) + }); + assert_eq!(parsed, a); + } + } } diff --git a/src/tui.rs b/src/tui.rs index 275e883..a3e43a8 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -2,7 +2,7 @@ use crate::app::{App, View}; use crate::events::handle_events; use crate::feed::Feed; use crate::ui; -use anyhow::Result; +use anyhow::{Context, Result}; use crossterm::{ event::{self, DisableMouseCapture, EnableMouseCapture}, execute, @@ -12,9 +12,108 @@ use ratatui::{ backend::{Backend, CrosstermBackend}, Terminal, }; +use std::io::Write; +use std::process::{Command, Stdio}; use std::sync::mpsc; use std::{io, time::Duration}; +/// RAII guard that re-enters the alt-screen + raw mode + mouse capture on drop, +/// so a panic during a pipe-to child invocation cannot leave the terminal in +/// a broken state. +struct TerminalRestoreGuard; + +impl TerminalRestoreGuard { + fn enter() -> io::Result { + // Leave the TUI before handing the tty to the child. + let mut stdout = io::stdout(); + execute!(stdout, LeaveAlternateScreen, DisableMouseCapture)?; + disable_raw_mode()?; + Ok(Self) + } +} + +impl Drop for TerminalRestoreGuard { + fn drop(&mut self) { + // Best-effort restore — ignore errors during unwind. + let _ = enable_raw_mode(); + let _ = execute!(io::stdout(), EnterAlternateScreen, EnableMouseCapture); + } +} + +/// Suspend the TUI, run argv with the given stdin payload (or inherited stdin +/// if `stdin_payload` is None), then re-enter the TUI and force a redraw. +/// Drains any pending terminal events so terminal-probe responses from the +/// child don't leak into our key handler. +pub fn suspend_for_command( + terminal: &mut Terminal, + argv: &[String], + stdin_payload: Option<&[u8]>, +) -> Result { + if argv.is_empty() { + anyhow::bail!("empty command"); + } + let status = { + let _guard = TerminalRestoreGuard::enter()?; + let mut cmd = Command::new(&argv[0]); + cmd.args(&argv[1..]); + if stdin_payload.is_some() { + cmd.stdin(Stdio::piped()); + } + // stdout/stderr inherited so user sees output of the child. + let mut child = cmd + .spawn() + .with_context(|| format!("failed to spawn '{}'", argv[0]))?; + // Write the stdin payload from a dedicated thread so a payload larger + // than the pipe buffer (typically 64 KiB on Linux) cannot deadlock the + // parent against a child that hasn't started reading yet. + let writer = match (stdin_payload, child.stdin.take()) { + (Some(payload), Some(mut stdin)) => { + let payload = payload.to_vec(); + Some(std::thread::spawn(move || { + let _ = stdin.write_all(&payload); + // stdin drops here, closing the pipe so the child sees EOF. + })) + } + _ => None, + }; + let status = child.wait().context("failed to wait for child")?; + if let Some(w) = writer { + let _ = w.join(); + } + status + }; + // Drain any pending input that might have arrived during the child's run + // (e.g. terminal-probe responses from less/vim). + while event::poll(Duration::from_millis(0))? { + let _ = event::read(); + } + terminal.clear()?; + Ok(status) +} + +/// Spawn a child detached: stdio nulled, parent does not block. A reaper +/// thread waits on the child so it doesn't linger as a zombie for the +/// lifetime of the TUI session — `Child` left undropped on Unix would +/// otherwise stay `` until parent exit. Used by exec_on_new where +/// we may fan out many children and must not block the main loop. +pub fn spawn_detached(argv: &[String]) -> Result<()> { + if argv.is_empty() { + anyhow::bail!("empty command"); + } + let mut cmd = Command::new(&argv[0]); + cmd.args(&argv[1..]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + let mut child = cmd + .spawn() + .with_context(|| format!("failed to spawn '{}'", argv[0]))?; + std::thread::spawn(move || { + let _ = child.wait(); + }); + Ok(()) +} + pub fn run(mut app: App) -> Result<()> { // Set up terminal enable_raw_mode()?; @@ -43,6 +142,118 @@ pub fn run(mut app: App) -> Result<()> { Ok(()) } +/// Drain all queued macro steps in FIFO order. Action steps mutate app +/// state inline, PipeTo suspends the terminal for a blocking child, Exec +/// spawns detached. On the first failure of any step the rest of the queue +/// is dropped — a "failure" is any condition that surfaces `app.error` +/// (spawn failures, missing article context, or an Action that sets the +/// error itself, e.g. `open-in-browser` with no URL). PipeTo non-zero exits +/// are not treated as failures: a script that returns 1 to mean "skip" is a +/// reasonable usage, and the user already saw the child's output. +fn drain_macro_steps(terminal: &mut Terminal, app: &mut App) { + use crate::keybindings::MacroStep; + while let Some(step) = app.pending_macro_steps.pop_front() { + // Track whether this step introduced a new error so a chain like + // `open-in-browser ; toggle-star` halts when `open-in-browser` fails, + // matching the contract documented above and the PipeTo / Exec paths. + let pre_error = app.error.is_some(); + match step { + MacroStep::Action(a) => crate::events::dispatch_action(app, a), + MacroStep::PipeTo { + argv_template, + stdin, + } => { + let (argv, payload) = + match crate::events::build_pipe_invocation(app, &argv_template, stdin) { + Some(v) => v, + None => { + app.error = Some("pipe-to: no article in focus".to_string()); + app.pending_macro_steps.clear(); + break; + } + }; + if let Err(e) = suspend_for_command(terminal, &argv, Some(&payload)) { + app.error = Some(format!("pipe-to: {}", e)); + app.pending_macro_steps.clear(); + break; + } + } + MacroStep::Exec { argv_template } => { + let argv = match crate::events::build_exec_invocation(app, &argv_template) { + Some(v) => v, + None => { + app.error = Some("exec: no article in focus".to_string()); + app.pending_macro_steps.clear(); + break; + } + }; + if let Err(e) = spawn_detached(&argv) { + app.error = Some(format!("exec: {}", e)); + app.pending_macro_steps.clear(); + break; + } + } + } + if !pre_error && app.error.is_some() { + app.pending_macro_steps.clear(); + break; + } + } +} + +/// Diff a freshly-arrived feed against `app.seen_items`, mutate the seen +/// state in memory, and return the argv list to spawn for each genuinely-new +/// item. On the first successful fetch of a feed (URL not yet in +/// `feeds_seeded`), seed the seen set silently and return an empty list. +/// +/// Returns Vec rather than spawning + saving inline so a refresh batch can +/// `save_data` ONCE before spawning ANY child, instead of once per feed +/// arrival (write amplification of N for N bookmarks). Crash semantics still +/// AT-MOST-ONCE: see [`flush_exec_on_new`] for the persistence ordering. +/// +/// No-op when the hook is not configured. seen_items / feeds_seeded only +/// exist to drive this hook, so users who never opt in pay neither the +/// memory cost nor any save_data round-trip. +fn collect_exec_on_new(app: &mut App, feed: &Feed) -> Vec> { + let argv_template = match app.exec_on_new_template.as_ref() { + Some(t) => t.clone(), + None => return Vec::new(), + }; + let newly_seen_idx = crate::app::mark_feed_seen(app, feed); + if newly_seen_idx.is_empty() { + return Vec::new(); + } + newly_seen_idx + .into_iter() + .filter_map(|idx| { + let item = feed.items.get(idx)?; + let ctx = crate::app::article_context_from(feed, item); + Some(crate::app::expand_argv_template(&argv_template, &ctx)) + }) + .collect() +} + +/// Persist the updated seen-set, then spawn all collected exec_on_new +/// children. The "save then spawn" order is the AT-MOST-ONCE guarantee: +/// a kill between save and spawn loses notifications, but a restart never +/// re-fires the same item. Matters for side-effecting hooks like +/// `wallabag-cli add` where a duplicate is actively wrong. +fn flush_exec_on_new(app: &mut App, pending: Vec>) { + if pending.is_empty() { + return; + } + let _ = app.save_data(); + let mut first_failure_recorded = false; + for argv in pending { + if let Err(e) = spawn_detached(&argv) { + if !first_failure_recorded { + app.error = Some(format!("exec_on_new: {}", e)); + first_failure_recorded = true; + } + } + } +} + /// Spawn background threads to fetch all bookmarked feeds, sending results through the channel. /// Returns the sender's pending count and the receiver. fn spawn_feed_refresh(app: &mut App) -> (usize, mpsc::Receiver<(usize, Result)>) { @@ -105,8 +316,14 @@ fn run_app(terminal: &mut Terminal, app: &mut App) -> Result<()> // Drain any feeds that arrived from background threads if pending_count > 0 { + // Accumulate exec_on_new spawns across every feed arriving in this + // tick, so we only `save_data` once before spawning. Saving per + // feed (the previous shape) was N whole-file JSON writes per + // refresh. + let mut pending_exec: Vec> = Vec::new(); while let Ok((idx, result)) = feed_rx.try_recv() { if let Ok(feed) = result { + pending_exec.extend(collect_exec_on_new(app, &feed)); // Insert at the correct position to maintain bookmark order, // or append if earlier feeds haven't arrived yet let insert_pos = app @@ -147,6 +364,10 @@ fn run_app(terminal: &mut Terminal, app: &mut App) -> Result<()> let _ = app.save_data(); } } + // Persist seen-set ONCE for the whole batch, then spawn. Done + // outside the per-feed loop so a refresh that brings back items + // for many feeds writes the JSON file once instead of N times. + flush_exec_on_new(app, pending_exec); } // If loading, use a shorter timeout for animation @@ -165,6 +386,7 @@ fn run_app(terminal: &mut Terminal, app: &mut App) -> Result<()> if handle_events(app)? { return Ok(()); } + drain_macro_steps(terminal, app); } else if last_tick.elapsed() >= tick_rate { // Update animation frame on tick if app.is_loading { @@ -185,6 +407,15 @@ fn run_app(terminal: &mut Terminal, app: &mut App) -> Result<()> } } + // Time out a stranded macro-prefix wait so the next unrelated + // keystroke isn't silently consumed as a macro follow-up. + if let Some(since) = app.awaiting_macro_key_since { + if since.elapsed() >= success_timeout { + app.awaiting_macro_key = false; + app.awaiting_macro_key_since = None; + } + } + // Check if auto-refresh should trigger if app.should_auto_refresh() { app.refresh_requested = true; @@ -194,3 +425,89 @@ fn run_app(terminal: &mut Terminal, app: &mut App) -> Result<()> } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::keybindings::{KeyAction, MacroStep}; + use ratatui::backend::TestBackend; + + fn test_terminal() -> Terminal { + Terminal::new(TestBackend::new(80, 24)).unwrap() + } + + #[test] + fn test_drain_halts_when_action_sets_error() { + // A chain where step 1 errors must not run step 2. MoveUp is not + // supported in macros (sets app.error to a "not supported" message); + // OpenInBrowser with no focused article sets a different error. With + // halt-on-error the first message survives, because step 2 never runs. + // Without the halt, step 2 would overwrite app.error and a follow-up + // side-effecting step would run silently. + let mut app = App::new(); + app.error = None; + app.pending_macro_steps + .push_back(MacroStep::Action(KeyAction::MoveUp)); + app.pending_macro_steps + .push_back(MacroStep::Action(KeyAction::OpenInBrowser)); + + let mut terminal = test_terminal(); + drain_macro_steps(&mut terminal, &mut app); + + let err = app.error.as_deref().unwrap_or(""); + assert!( + err.contains("move-up") && err.contains("not supported"), + "first error must survive; got: {}", + err + ); + assert!( + !err.contains("open-in-browser"), + "step 2 must not have run (would have overwritten the error); got: {}", + err + ); + assert!(app.pending_macro_steps.is_empty()); + } + + #[test] + fn test_drain_continues_when_steps_succeed() { + // Sanity: a clean chain runs every step. Two `help` steps are no-ops + // that don't set app.error; both must execute and leave the queue empty. + let mut app = App::new(); + app.error = None; + app.show_help_overlay = false; + app.pending_macro_steps + .push_back(MacroStep::Action(KeyAction::Help)); + app.pending_macro_steps + .push_back(MacroStep::Action(KeyAction::Help)); + + let mut terminal = test_terminal(); + drain_macro_steps(&mut terminal, &mut app); + + assert!(app.pending_macro_steps.is_empty()); + assert!(app.error.is_none()); + assert!(app.show_help_overlay); + } + + #[test] + fn test_drain_preserves_preexisting_error_and_still_runs_step() { + // Edge case: if `app.error` is already set before a step runs, the + // step itself didn't set it — we must not treat it as a step failure. + // (In practice this can't happen because handle_events clears app.error + // on the next keypress, but the drain function shouldn't depend on that + // invariant.) + let mut app = App::new(); + app.error = Some("stale error".to_string()); + app.show_help_overlay = false; + app.pending_macro_steps + .push_back(MacroStep::Action(KeyAction::Help)); + app.pending_macro_steps + .push_back(MacroStep::Action(KeyAction::Help)); + + let mut terminal = test_terminal(); + drain_macro_steps(&mut terminal, &mut app); + + // Both steps ran despite the preexisting error. + assert!(app.pending_macro_steps.is_empty()); + assert!(app.show_help_overlay); + } +} diff --git a/src/ui/modals.rs b/src/ui/modals.rs index 239bbc4..2261dcb 100644 --- a/src/ui/modals.rs +++ b/src/ui/modals.rs @@ -1,5 +1,5 @@ use crate::app::{App, InputMode, LinkType, TimeFilter, View}; -use crate::keybindings::{key_display, KeyAction}; +use crate::keybindings::{binding_display, key_display, KeyAction, MacroStep}; use crate::ui::utils::{centered_rect_with_min, truncate_str}; use crate::ui::ColorScheme; use ratatui::{ @@ -803,6 +803,39 @@ pub(super) fn render_help_overlay(f: &mut Frame, app: &App, color } } + if !app.macros.is_empty() { + lines.push(Line::from("")); + lines.push(separator.clone()); + lines.push(Line::from("")); + let prefix_label = binding_display(&app.macro_options.prefix); + lines.push(Line::from(Span::styled( + format!(" Macros (press {} then key)", prefix_label), + section_style, + ))); + lines.push(Line::from("")); + for mac in &app.macros { + let trigger_str = format!("{}{}", prefix_label, binding_display(&mac.trigger)); + let summary = match &mac.description { + Some(d) => d.clone(), + None => mac + .steps + .iter() + .map(|s| match s { + MacroStep::Action(a) => a.as_str().to_string(), + MacroStep::PipeTo { argv_template, .. } => { + format!("pipe-to {}", argv_template.join(" ")) + } + MacroStep::Exec { argv_template } => { + format!("exec {}", argv_template.join(" ")) + } + }) + .collect::>() + .join(" ; "), + }; + add_key(&trigger_str, &summary, &mut lines); + } + } + lines.push(Line::from("")); lines.push(separator); lines.push(Line::from(""));