From ae294af8b9d37e7b14dd95c48866cf2766156fb8 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 29 Apr 2026 10:34:38 -0700 Subject: [PATCH 1/2] refactor(list): collapse rendering modes into a single `RenderTarget` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `wt list` entry point carried three independently-resolved booleans (`format`, `show_progress`, `render_table`) plus a `RenderMode` enum to encode just three real modes: JSON, buffered table, progressive table. Replace the boolean soup with a `RenderTarget` enum (`Json` | `Table { progressive: bool }`) resolved once from the CLI flags + stdout TTY in `handle_list`, threaded through to `collect::collect`. `TableRenderPlan` no longer needs the `is_tty` fallback — `RenderTarget` factors TTY detection in up front, so the progressive table only exists when it's actually going to be used. The `Some(t)` → `finalize`, `None` → buffered split is now the only path. `RowCache` shrinks to its load-bearing role: the `has_data` bitmap that tells the reveal-tick renderer which rows still need the skeleton vs the full row. The `set_result` dedup is gone — `ProgressiveTable::update_row` already dedups internally and the picker handler writes through an idempotent `Mutex`. The remaining helper inlines as a free `render_reveal` function. Net: −76 lines across the four touched list files; no observable behavior change (snapshot tests untouched). Co-Authored-By: Claude --- src/commands/list/collect/mod.rs | 196 +++++++++---------------- src/commands/list/mod.rs | 39 ++--- src/commands/list/progressive.rs | 87 ++++++----- src/commands/list/progressive_table.rs | 18 --- src/commands/picker/mod.rs | 6 +- src/main.rs | 4 +- 6 files changed, 137 insertions(+), 213 deletions(-) diff --git a/src/commands/list/collect/mod.rs b/src/commands/list/collect/mod.rs index 5d913bae9..9cb5b6d2f 100644 --- a/src/commands/list/collect/mod.rs +++ b/src/commands/list/collect/mod.rs @@ -242,6 +242,7 @@ use worktrunk::styling::{ use crate::commands::is_worktree_at_expected_path; use super::model::{CommitDetails, DisplayFields, ItemKind, ListItem, StatusSymbols, WorktreeData}; +use super::progressive::RenderTarget; use super::progressive_table::ProgressiveTable; // Re-exports for sibling modules (columns.rs, render.rs, layout.rs) @@ -265,11 +266,7 @@ struct TableRenderPlan { impl TableRenderPlan { fn render(mut self) -> anyhow::Result<()> { if let Some(mut table) = self.progressive_table.take() { - if table.is_tty() { - table.finalize(self.rows, self.summary)?; - } else { - print_buffered_table(&self.header, &self.rows, &self.summary); - } + table.finalize(self.rows, self.summary)?; } else { print_buffered_table(&self.header, &self.rows, &self.summary); } @@ -388,64 +385,28 @@ pub enum ShowConfig { }, } -/// Per-row render cache shared by the `wt list` progressive table and the -/// picker's `PickerProgressHandler`. Both sinks write through the same dedup -/// path so one rendering pass serves both. -/// -/// `set_result` records a new render and returns `Some(line)` only when it -/// differs from the cached value. -/// -/// `set_reveal` runs after `layout.placeholder` is promoted from blank to -/// `·`. Every row is re-rendered (skeleton for rows with no data yet to -/// avoid surfacing seeded defaults like "55y"; `format_list_item_line` for -/// rows that received at least one result, so still-pending cells pick up -/// the promoted `·`). Dedup against the cache keeps emitted updates minimal. -struct RowCache { - last: Vec, - has_data: Vec, -} - -impl RowCache { - fn new(n: usize) -> Self { - Self { - last: vec![String::new(); n], - has_data: vec![false; n], - } - } - - fn set_result(&mut self, idx: usize, rendered: String) -> Option { - self.has_data[idx] = true; - if self.last[idx] == rendered { - None - } else { - self.last[idx] = rendered.clone(); - Some(rendered) - } - } - - fn set_reveal( - &mut self, - items: &[super::model::ListItem], - layout: &super::layout::LayoutConfig, - ) -> Vec> { - items - .iter() - .enumerate() - .map(|(idx, item)| { - let new = if self.has_data[idx] { - layout.format_list_item_line(item) - } else { - layout.render_skeleton_row(item).render() - }; - if self.last[idx] == new { - None - } else { - self.last[idx] = new.clone(); - Some(new) - } - }) - .collect() - } +/// On the reveal tick, every row is re-rendered. Rows that have already +/// received at least one task result use `format_list_item_line` so still- +/// pending cells pick up the promoted `·`; rows with no data yet stay on +/// the skeleton renderer to avoid flashing seeded defaults like "55y". The +/// `has_data` bitmap is the only state needed to make that choice. +fn render_reveal( + has_data: &[bool], + items: &[super::model::ListItem], + layout: &super::layout::LayoutConfig, +) -> Vec> { + items + .iter() + .enumerate() + .map(|(idx, item)| { + let line = if has_data[idx] { + layout.format_list_item_line(item) + } else { + layout.render_skeleton_row(item).render() + }; + Some(line) + }) + .collect() } /// Build the progressive-table footer shown while the drain is stalled. @@ -475,18 +436,23 @@ fn format_stall_footer( ) } -/// Collect worktree data with optional progressive rendering. +/// Collect worktree data with rendering driven by `render_target`. /// -/// When `show_progress` is true, renders a skeleton immediately and updates as data arrives. -/// When false, behavior depends on `render_table`: -/// - If `render_table` is true: renders final table (buffered mode) -/// - If `render_table` is false: returns data without rendering (JSON mode) +/// - [`RenderTarget::Table { progressive: true }`]: renders a skeleton +/// immediately and updates rows in place as data arrives, then morphs the +/// skeleton into the final table. +/// - [`RenderTarget::Table { progressive: false }`]: collects silently, then +/// prints the final table once. +/// - [`RenderTarget::Json`]: collects silently and returns data without +/// writing to stdout. Used by `--format=json` and the picker (which has its +/// own progressive UI driven via `ShowConfig::Resolved::progressive_handler`). pub fn collect( repo: &Repository, show_config: ShowConfig, - show_progress: bool, - render_table: bool, + render_target: RenderTarget, ) -> anyhow::Result> { + let show_progress = matches!(render_target, RenderTarget::Table { progressive: true }); + let render_table = matches!(render_target, RenderTarget::Table { .. }); worktrunk::shell_exec::trace_instant("List collect started"); // Determine what to fetch speculatively in the parallel phase. @@ -972,9 +938,9 @@ pub fn collect( // Picker mirrors `wt list`'s blank→`·` reveal. The placeholder starts // blank so fast completions don't flash loading dots; the Reveal event - // below promotes it to `·` at 200ms. `show_progress=false` (the picker - // path today) skips the block above, so set it here unconditionally - // when a handler is present. + // below promotes it to `·` at 200ms. The picker passes + // `RenderTarget::Json`, which skips the block above, so set the + // placeholder here unconditionally when a handler is present. if progressive_handler.is_some() { layout.placeholder.set(super::render::PLACEHOLDER_BLANK); } @@ -1227,7 +1193,7 @@ pub fn collect( first_result_traced: false, }) }); - let mut row_cache = RowCache::new(n_items); + let mut has_data = vec![false; n_items]; let drain_deadline = collect_deadline.unwrap_or_else(|| std::time::Instant::now() + results::DRAIN_TIMEOUT); @@ -1249,8 +1215,12 @@ pub fn collect( match event { results::DrainEvent::Result { item_idx, item } => { + // `update_row` and the picker handler both write through + // an idempotent slot (terminal line / `Mutex`), + // so forwarding every result without dedup is safe; the + // table dedups internally against its previous render. let rendered = layout.format_list_item_line(item); - let changed = row_cache.set_result(item_idx, rendered); + has_data[item_idx] = true; if let Some(state_cell) = progressive_state.as_ref() { let mut s = state_cell.borrow_mut(); @@ -1275,25 +1245,20 @@ pub fn collect( "{INFO_SYMBOL} {dim}{footer_base} ({completed}/{total_results} loaded){dim:#}" ); s.table.update_footer(footer_msg); - - if let Some(line) = &changed { - s.table.update_row(item_idx, line.clone()); - } + s.table.update_row(item_idx, rendered.clone()); if let Err(e) = s.table.flush() { log::debug!("Progressive table flush failed: {}", e); } } - if let Some(handler) = progressive_handler.as_ref() - && let Some(line) = changed - { - handler.on_update(item_idx, line); + if let Some(handler) = progressive_handler.as_ref() { + handler.on_update(item_idx, rendered); } } results::DrainEvent::Reveal { items } => { layout.placeholder.set(super::render::PLACEHOLDER); - let updates = row_cache.set_reveal(items, &layout); + let updates = render_reveal(&has_data, items, &layout); if let Some(state_cell) = progressive_state.as_ref() { let mut s = state_cell.borrow_mut(); @@ -1484,11 +1449,13 @@ pub fn collect( // all_items now contains both worktrees and branches (if requested) let items = all_items; - // Table rendering complete (when render_table=true): - // - Progressive + TTY: rows morphed in place, footer became summary - // - Progressive + Non-TTY: rendered final table (no intermediate output) - // - Buffered: rendered final table - // JSON mode (render_table=false): no rendering, data returned for serialization + // Table rendering complete: + // - `RenderTarget::Table { progressive: true }`: rows morphed in place, + // footer became summary + // - `RenderTarget::Table { progressive: false }`: rendered final table + // - `RenderTarget::Json`: no stdout rendering; data returned for the + // caller to serialize (`wt list --format=json`) or feed into its own + // UI (picker via `progressive_handler`) worktrunk::shell_exec::trace_instant("List collect complete"); Ok(Some(super::model::ListData { items })) @@ -1787,13 +1754,14 @@ mod tests { ); } - /// `set_result` marks the row as having data and dedups by comparing - /// against the cached render; `set_reveal` picks skeleton-vs-format - /// per row based on `has_data` and also dedups. These two behaviors - /// are load-bearing for the picker's partial-row reveal correctness - /// (see the RowCache doc comment). + /// `render_reveal` picks `format_list_item_line` for rows with data and + /// `render_skeleton_row` for rows without — the choice is load-bearing + /// for the picker's partial-row reveal correctness, since the + /// post-reveal placeholder swap has to reach pending cells without + /// surfacing seeded defaults like "55y" on rows that haven't received + /// any task results. #[test] - fn test_row_cache_dedup_and_reveal() { + fn test_render_reveal_picks_renderer_per_row() { use super::super::layout::calculate_layout_with_width; use super::super::model::ListItem; use std::collections::HashSet; @@ -1806,38 +1774,16 @@ mod tests { let skip_tasks: HashSet = HashSet::new(); let layout = calculate_layout_with_width(&items, &skip_tasks, 80, Path::new("/tmp"), None); - let mut cache = RowCache::new(2); - - // First set_result: cache was empty, so the new line is emitted. - let first = cache.set_result(0, "row-zero-line-v1".into()); - assert_eq!(first.as_deref(), Some("row-zero-line-v1")); - - // Same render again → dedup: None. - let dup = cache.set_result(0, "row-zero-line-v1".into()); - assert_eq!(dup, None); - - // Different render → Some again. - let changed = cache.set_result(0, "row-zero-line-v2".into()); - assert_eq!(changed.as_deref(), Some("row-zero-line-v2")); - - // set_reveal after the placeholder flip. Row 0 has data: use - // format_list_item_line; the result is different from the cached - // synthetic string above so it's emitted as Some. Row 1 has no - // data: use render_skeleton_row; cache was empty so it's emitted. layout.placeholder.set(super::super::render::PLACEHOLDER); - let updates = cache.set_reveal(&items, &layout); + + // Row 0 has data → format_list_item_line; row 1 doesn't → skeleton. + let has_data = vec![true, false]; + let updates = render_reveal(&has_data, &items, &layout); assert_eq!(updates.len(), 2); - assert!( - updates[0].is_some(), - "row 0 had data but cached string was synthetic; reveal must emit new render" - ); - assert!( - updates[1].is_some(), - "row 1 had no data; reveal must emit skeleton render" - ); - // Second reveal with no intervening changes: both rows dedup to None. - let updates2 = cache.set_reveal(&items, &layout); - assert_eq!(updates2, vec![None, None]); + let row_zero = updates[0].as_deref().unwrap(); + let row_one = updates[1].as_deref().unwrap(); + assert_eq!(row_zero, layout.format_list_item_line(&items[0])); + assert_eq!(row_one, layout.render_skeleton_row(&items[1]).render()); } } diff --git a/src/commands/list/mod.rs b/src/commands/list/mod.rs index 12c8516e2..ecde4ad79 100644 --- a/src/commands/list/mod.rs +++ b/src/commands/list/mod.rs @@ -135,7 +135,7 @@ mod spacing_test; use anstyle::Style; use anyhow::Context; use model::{ListData, ListItem}; -use progressive::RenderMode; +use progressive::RenderTarget; use worktrunk::git::Repository; use worktrunk::styling::INFO_SYMBOL; @@ -149,21 +149,9 @@ pub fn handle_list( cli_branches: bool, cli_remotes: bool, cli_full: bool, - render_mode: RenderMode, + progressive_flag: Option, ) -> anyhow::Result<()> { - // Progressive rendering only for table format with Progressive mode - let show_progress = match format { - crate::OutputFormat::Table | crate::OutputFormat::ClaudeCode => { - render_mode == RenderMode::Progressive - } - crate::OutputFormat::Json => false, // JSON never shows progress - }; - - // Render table in collect() for all table modes (progressive + buffered) - let render_table = matches!( - format, - crate::OutputFormat::Table | crate::OutputFormat::ClaudeCode - ); + let render_target = RenderTarget::detect(format, progressive_flag); let list_data = collect::collect( &repo, @@ -172,27 +160,20 @@ pub fn handle_list( cli_remotes, cli_full, }, - show_progress, - render_table, + render_target, )?; let Some(ListData { items, .. }) = list_data else { return Ok(()); }; - match format { - crate::OutputFormat::Json => { - // Convert to new JSON structure - let json_items = json_output::to_json_items(&items, &repo); - let json = - serde_json::to_string_pretty(&json_items).context("Failed to serialize to JSON")?; - println!("{}", json); - } - crate::OutputFormat::Table | crate::OutputFormat::ClaudeCode => { - // Table and summary already rendered in collect() for all modes - // Nothing to do here - collect() handles the complete table rendering - } + if matches!(render_target, RenderTarget::Json) { + let json_items = json_output::to_json_items(&items, &repo); + let json = + serde_json::to_string_pretty(&json_items).context("Failed to serialize to JSON")?; + println!("{}", json); } + // Table modes already rendered inside `collect()`. Ok(()) } diff --git a/src/commands/list/progressive.rs b/src/commands/list/progressive.rs index 37c892e24..6e2ba71fb 100644 --- a/src/commands/list/progressive.rs +++ b/src/commands/list/progressive.rs @@ -1,37 +1,37 @@ use std::io::IsTerminal; -/// Rendering mode for list command +use crate::OutputFormat; + +/// Resolved rendering decision for `wt list`. Collapses output format, +/// `--progressive`/`--no-progressive` flags, and stdout TTY detection into +/// a single value the rest of the pipeline can match on. +/// +/// `Table { progressive: true }` is only set when stdout is a TTY — an +/// explicit `--progressive` flag on a piped stdout still resolves to +/// `progressive: false` because the in-place updates can't reach the user. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum RenderMode { - /// Buffered: collect all data, then render (traditional) - Buffered, - /// Progressive: show rows immediately, update as data arrives - Progressive, +pub enum RenderTarget { + /// Caller serializes data themselves (JSON output, picker UI). `collect` + /// returns data without writing to stdout. + Json, + /// Render a table to stdout. `progressive` controls whether intermediate + /// rows are streamed (`true`) or only the final table is written (`false`). + Table { progressive: bool }, } -impl RenderMode { - /// Determine rendering mode based on CLI flags and TTY status - /// - /// # Arguments - /// - /// * `progressive` - Rendering mode (Some(true) = --progressive, Some(false) = --no-progressive, None = auto) - /// - /// Table output goes to stdout, so we check stdout's TTY status. When piped - /// (`wt list | grep`), we buffer; when interactive, we render progressively. - pub fn detect(progressive: Option) -> Self { - // Priority 1: Explicit CLI flag - match progressive { - Some(true) => return RenderMode::Progressive, - Some(false) => return RenderMode::Buffered, - None => {} // Fall through to auto-detection - } - - // Priority 2: Auto-detect based on stdout TTY - if std::io::stdout().is_terminal() { - // TODO: Check for pager in environment - RenderMode::Progressive - } else { - RenderMode::Buffered +impl RenderTarget { + /// Resolve the target from CLI inputs and the current stdout TTY state. + pub fn detect(format: OutputFormat, progressive_flag: Option) -> Self { + match format { + OutputFormat::Json => RenderTarget::Json, + OutputFormat::Table | OutputFormat::ClaudeCode => { + let is_tty = std::io::stdout().is_terminal(); + let progressive = match progressive_flag { + Some(p) => p && is_tty, + None => is_tty, + }; + RenderTarget::Table { progressive } + } } } } @@ -41,13 +41,28 @@ mod tests { use super::*; #[test] - fn test_render_mode_detect_explicit_flags() { - // --progressive (Some(true)) should force progressive mode - assert_eq!(RenderMode::detect(Some(true)), RenderMode::Progressive); - - // --no-progressive (Some(false)) should force buffered mode - assert_eq!(RenderMode::detect(Some(false)), RenderMode::Buffered); + fn json_format_always_resolves_to_json() { + assert_eq!( + RenderTarget::detect(OutputFormat::Json, None), + RenderTarget::Json + ); + assert_eq!( + RenderTarget::detect(OutputFormat::Json, Some(true)), + RenderTarget::Json + ); + assert_eq!( + RenderTarget::detect(OutputFormat::Json, Some(false)), + RenderTarget::Json + ); + } - // None should auto-detect (tested via TTY checks in runtime) + #[test] + fn explicit_no_progressive_disables_progressive() { + // In test runs stdout isn't a TTY, but we assert the explicit-false + // branch regardless of TTY state. + assert_eq!( + RenderTarget::detect(OutputFormat::Table, Some(false)), + RenderTarget::Table { progressive: false } + ); } } diff --git a/src/commands/list/progressive_table.rs b/src/commands/list/progressive_table.rs index 89b0b73f6..38282ec26 100644 --- a/src/commands/list/progressive_table.rs +++ b/src/commands/list/progressive_table.rs @@ -286,11 +286,6 @@ impl ProgressiveTable { self.flush() } } - - /// Check if output is going to a TTY. - pub fn is_tty(&self) -> bool { - self.is_tty - } } #[cfg(test)] @@ -364,19 +359,6 @@ mod tests { assert_eq!(table.lines.last().unwrap(), &footer); } - #[test] - fn test_is_tty_returns_value() { - let table = ProgressiveTable::new( - "header".to_string(), - vec!["row".to_string()], - "footer".to_string(), - 80, - ); - - // In test environment, stdout is typically not a TTY - let _ = table.is_tty(); - } - #[test] fn test_row_count_tracking() { let table = ProgressiveTable::new( diff --git a/src/commands/picker/mod.rs b/src/commands/picker/mod.rs index 08dda0541..e0e3925c4 100644 --- a/src/commands/picker/mod.rs +++ b/src/commands/picker/mod.rs @@ -114,6 +114,7 @@ use super::handle_switch::{ }; use super::hooks::{execute_hook, spawn_background_hooks}; use super::list::collect; +use super::list::progressive::RenderTarget; use super::repository_ext::{RemoveTarget, RepositoryCliExt}; use super::worktree::hooks::PostRemoveContext; use super::worktree::{ @@ -634,8 +635,9 @@ pub fn handle_picker( list_width: Some(skim_list_width), progressive_handler: Some(bg_handler), }, - false, // show_progress (picker renders its own UI) - false, // render_table + // Picker renders its own UI through `progressive_handler`; + // collect must not write to stdout. + RenderTarget::Json, ); }) .context("Failed to spawn picker-collect thread")?; diff --git a/src/main.rs b/src/main.rs index 3e5888b7d..3878e9ea1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,7 +17,6 @@ use worktrunk::styling::{ use commands::command_approval::approve_hooks; use commands::command_executor::CommandContext; -use commands::list::progressive::RenderMode; use commands::worktree::RemoveResult; mod cli; @@ -505,14 +504,13 @@ fn handle_list_command(args: ListArgs) -> anyhow::Result<()> { } None => { let (repo, _recovered) = current_or_recover()?; - let render_mode = RenderMode::detect(flag_pair(args.progressive, args.no_progressive)); handle_list( repo, args.format, args.branches, args.remotes, args.full, - render_mode, + flag_pair(args.progressive, args.no_progressive), ) } } From 79b1a5ef9ae99c61b87f7a9707a6af65fae99bdf Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 29 Apr 2026 10:46:40 -0700 Subject: [PATCH 2/2] refactor(list): drop vestigial `Option` from `on_reveal` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the `RowCache` removal, `render_reveal` always returns a real line for every row (skeleton renderer for rows with no data, full renderer for rows with data) — the `Some`/`None` semantics the trait described no longer exist in production. Tighten `PickerProgressHandler::on_reveal` to `Vec` and drop the unwrap branches at the call sites; slot writes are idempotent through `Mutex`, so unconditionally rewriting every row at reveal time is safe. Co-Authored-By: Claude --- src/commands/list/collect/mod.rs | 30 ++++++++++------------ src/commands/picker/progressive_handler.rs | 17 +++++------- 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/commands/list/collect/mod.rs b/src/commands/list/collect/mod.rs index 9cb5b6d2f..6890bc349 100644 --- a/src/commands/list/collect/mod.rs +++ b/src/commands/list/collect/mod.rs @@ -348,10 +348,12 @@ pub trait PickerProgressHandler: Send + Sync { /// up on the next heartbeat. fn on_update(&self, idx: usize, rendered: String); - /// Fired at the 200ms reveal deadline. Entry per row: `Some(line)` for - /// rows still at skeleton state (placeholder needs promoting to `·`), - /// `None` for rows that already received real data via `on_update`. - fn on_reveal(&self, rendered: Vec>); + /// Fired at the 200ms reveal deadline. One pre-rendered line per row, + /// with the placeholder promoted from blank to `·`: rows that have + /// received real data use `format_list_item_line`, rows still at + /// skeleton state use the skeleton renderer. The handler writes every + /// slot — slot writes are idempotent. + fn on_reveal(&self, rendered: Vec); } /// Controls how show flags (branches/remotes/full) are determined in [`collect`]. @@ -394,17 +396,16 @@ fn render_reveal( has_data: &[bool], items: &[super::model::ListItem], layout: &super::layout::LayoutConfig, -) -> Vec> { +) -> Vec { items .iter() .enumerate() .map(|(idx, item)| { - let line = if has_data[idx] { + if has_data[idx] { layout.format_list_item_line(item) } else { layout.render_skeleton_row(item).render() - }; - Some(line) + } }) .collect() } @@ -1262,10 +1263,8 @@ pub fn collect( if let Some(state_cell) = progressive_state.as_ref() { let mut s = state_cell.borrow_mut(); - for (idx, update) in updates.iter().enumerate() { - if let Some(line) = update { - s.table.update_row(idx, line.clone()); - } + for (idx, line) in updates.iter().enumerate() { + s.table.update_row(idx, line.clone()); } if let Err(e) = s.table.flush() { log::debug!("Progressive table reveal flush failed: {}", e); @@ -1780,10 +1779,7 @@ mod tests { let has_data = vec![true, false]; let updates = render_reveal(&has_data, &items, &layout); assert_eq!(updates.len(), 2); - - let row_zero = updates[0].as_deref().unwrap(); - let row_one = updates[1].as_deref().unwrap(); - assert_eq!(row_zero, layout.format_list_item_line(&items[0])); - assert_eq!(row_one, layout.render_skeleton_row(&items[1]).render()); + assert_eq!(updates[0], layout.format_list_item_line(&items[0])); + assert_eq!(updates[1], layout.render_skeleton_row(&items[1]).render()); } } diff --git a/src/commands/picker/progressive_handler.rs b/src/commands/picker/progressive_handler.rs index 7d90ca015..0ea04befe 100644 --- a/src/commands/picker/progressive_handler.rs +++ b/src/commands/picker/progressive_handler.rs @@ -113,14 +113,12 @@ impl PickerProgressHandler for PickerHandler { } } - fn on_reveal(&self, rendered: Vec>) { + fn on_reveal(&self, rendered: Vec) { let Some(slots) = self.rendered_slots.get() else { return; }; for (slot, line) in slots.iter().zip(rendered) { - if let Some(line) = line { - *slot.lock().unwrap() = strip_osc8_hyperlinks(&line); - } + *slot.lock().unwrap() = strip_osc8_hyperlinks(&line); } } } @@ -238,14 +236,11 @@ mod tests { assert_eq!(*slots[0].lock().unwrap(), "skel-one", "row 0 untouched"); assert_eq!(*slots[1].lock().unwrap(), "updated-two"); - // on_reveal: Some entries rewrite the slot, None entries leave it. - handler.on_reveal(vec![Some("rev-one".into()), None]); + // on_reveal rewrites every slot — slot writes are idempotent + // through `Mutex`, so unconditional updates are safe. + handler.on_reveal(vec!["rev-one".into(), "rev-two".into()]); assert_eq!(*slots[0].lock().unwrap(), "rev-one"); - assert_eq!( - *slots[1].lock().unwrap(), - "updated-two", - "row 1 had data — reveal must not clobber it" - ); + assert_eq!(*slots[1].lock().unwrap(), "rev-two"); } /// Header + items get published in order. `output()` of the