From c9b4a04a4fd4e6b77aace97cac306f0a1de8d33e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Wed, 7 Oct 2026 03:17:16 -0400 Subject: [PATCH] =?UTF-8?q?Panics=20=C2=B7=20P27=20Files:=20gone=20owner?= =?UTF-8?q?=20picks=20replace=20local=20ones=20and=20mark=20none,=20stale?= =?UTF-8?q?=20path=20folders=20go=20unmarked,=20duplicate=20marks=20never?= =?UTF-8?q?=20take=20a=20last=20copy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/files/columns.rs | 6 +- src/files/duplicates.rs | 47 +++++++++++---- src/files/explorer.rs | 20 +++---- src/files/grid.rs | 7 +-- src/files/operation.rs | 8 +-- src/files/tests/explorer.rs | 19 ------ src/files/tests/find.rs | 11 ---- src/files/tests/mod.rs | 21 +++++-- src/files/tests/stale.rs | 115 ++++++++++++++++++++++++++++++++++++ src/lists/directory.rs | 21 ++----- tasks/panics.md | 3 +- 11 files changed, 192 insertions(+), 86 deletions(-) create mode 100644 src/files/tests/stale.rs diff --git a/src/files/columns.rs b/src/files/columns.rs index 937f0db..8038069 100644 --- a/src/files/columns.rs +++ b/src/files/columns.rs @@ -63,7 +63,11 @@ pub(super) fn columns( }, ); let lit = match depth < last { - true => Some(path[depth + 1].clone()), + true => Some(path[depth + 1].clone()).filter(|name| { + entries + .iter() + .any(|entry| entry.is_folder() && entry.name() == name) + }), false => selected.clone(), }; let (go, select, path, picked) = diff --git a/src/files/duplicates.rs b/src/files/duplicates.rs index 5acaec3..82c8eb4 100644 --- a/src/files/duplicates.rs +++ b/src/files/duplicates.rs @@ -70,19 +70,22 @@ impl DuplicateFinder { /// The copies marked to remove, by path; each group keeps one. pub fn marked(mut self, paths: impl IntoIterator>) -> Self { - let paths: Vec = paths.into_iter().map(Into::into).collect(); - for path in &paths { - assert!( - self.groups.iter().any(|group| group.paths.contains(path)), - "no copy {path}" - ); - } + let mut paths: Vec = paths.into_iter().map(Into::into).collect(); + paths.retain(|path| { + let listed = self.groups.iter().any(|group| group.paths.contains(path)); + if !listed { + log::error!("duplicate finder: no copy {path}; its mark is dropped"); + } + listed + }); for group in &self.groups { - assert!( - group.paths.iter().any(|path| !paths.contains(path)), - "every copy of {} is marked", - group.name - ); + if group.paths.iter().all(|path| paths.contains(path)) { + log::error!( + "duplicate finder: every copy of {} is marked; none of them stays marked", + group.name + ); + paths.retain(|path| !group.paths.contains(path)); + } } self.marked = paths; self @@ -219,3 +222,23 @@ impl RenderOnce for DuplicateFinder { .children(groups) } } + +#[cfg(test)] +mod tests { + use super::{DuplicateFinder, Duplicates}; + + #[test] + fn marks_on_no_copy_or_on_every_copy_are_dropped() { + let group = |name: &str, paths: [&str; 2]| Duplicates { + name: name.to_string().into(), + size: 1, + paths: paths.map(Into::into).to_vec(), + }; + let finder = DuplicateFinder::new( + "dupes", + [group("a.jpg", ["x", "y"]), group("b.jpg", ["p", "q"])], + ) + .marked(["x", "y", "p", "gone"]); + assert_eq!(finder.marked, ["p"]); + } +} diff --git a/src/files/explorer.rs b/src/files/explorer.rs index b8b3dd4..466d4b2 100644 --- a/src/files/explorer.rs +++ b/src/files/explorer.rs @@ -64,13 +64,12 @@ impl FileExplorer { levels.len() ); for (depth, name) in path.iter().enumerate().skip(1) { - assert!( - levels[depth - 1] - .iter() - .any(|entry| entry.is_folder() && entry.name() == name), - "{name} is no folder in {}", - path[depth - 1] - ); + let listed = levels[depth - 1] + .iter() + .any(|entry| entry.is_folder() && entry.name() == name); + if !listed { + log::error!("file explorer: {name} is no folder in {}", path[depth - 1]); + } } Self { id: id.into(), @@ -89,10 +88,9 @@ impl FileExplorer { pub fn selected(mut self, name: impl Into) -> Self { let name = name.into(); let here = self.levels.last().expect("a listed folder"); - assert!( - here.iter().any(|entry| *entry.name() == name), - "no entry {name}" - ); + if !here.iter().any(|entry| *entry.name() == name) { + log::error!("file explorer: no entry {name}; none selected"); + } self.selected = Some(name); self } diff --git a/src/files/grid.rs b/src/files/grid.rs index b73cc05..2f2b535 100644 --- a/src/files/grid.rs +++ b/src/files/grid.rs @@ -65,10 +65,9 @@ impl FileGrid { /// The entry picked, by name. pub fn selected(mut self, name: impl Into) -> Self { let name = name.into(); - assert!( - self.entries.iter().any(|entry| *entry.name() == name), - "no entry {name}" - ); + if !self.entries.iter().any(|entry| *entry.name() == name) { + log::error!("file grid: no entry {name}; none selected"); + } self.selected = Some(name); self } diff --git a/src/files/operation.rs b/src/files/operation.rs index ffcdf43..696c9ac 100644 --- a/src/files/operation.rs +++ b/src/files/operation.rs @@ -72,10 +72,10 @@ impl FileOperationProgress { (moved, total): (u64, u64), state: TransferState, ) -> Self { - assert!( - items > 0 && moved <= total, - "{items} items, {moved} of {total} bytes" - ); + assert!(items > 0, "no items, {moved} of {total} bytes"); + if moved > total { + log::error!("file operation: {moved} of {total} bytes; the bar fills"); + } Self { id: id.into(), operation, diff --git a/src/files/tests/explorer.rs b/src/files/tests/explorer.rs index 21cdcdb..d384ed5 100644 --- a/src/files/tests/explorer.rs +++ b/src/files/tests/explorer.rs @@ -168,13 +168,6 @@ fn each_folder_on_the_path_is_listed() { ); } -#[test] -#[should_panic(expected = "no entry Ghost.md")] -fn the_pick_is_in_the_folder_shown() { - let here = vec![DirEntry::file("Brief.md", 10, then())]; - let _ = FileExplorer::new("explorer", ["Home"], [here], FileView::List).selected("Ghost.md"); -} - /// An explorer that goes where it is sent, over a small tree, and the files it opened. struct Walking { path: Vec, @@ -242,15 +235,3 @@ fn a_new_folder_at_a_depth_starts_its_column_fresh(cx: &mut TestAppContext) { ["Contract.docx"] ); } - -#[test] -#[should_panic(expected = "Projects is no folder in Home")] -fn each_folder_on_the_path_is_listed_in_its_parent() { - let root = vec![DirEntry::file("Projects", 10, then())]; - let _ = FileExplorer::new( - "explorer", - ["Home", "Projects"], - [root, Vec::new()], - FileView::List, - ); -} diff --git a/src/files/tests/find.rs b/src/files/tests/find.rs index 1401e17..53ae805 100644 --- a/src/files/tests/find.rs +++ b/src/files/tests/find.rs @@ -255,17 +255,6 @@ fn the_last_copy_stays_and_remove_takes_the_marked(cx: &mut TestAppContext) { ); } -#[test] -#[should_panic(expected = "every copy of a.jpg is marked")] -fn a_group_keeps_a_copy() { - let group = Duplicates { - name: "a.jpg".into(), - size: 1, - paths: vec!["x".into(), "y".into()], - }; - let _ = DuplicateFinder::new("dupes", [group]).marked(["x", "y"]); -} - /// An archive of four files in two folders, and the files it handed over. struct Unpacking { opened: Vec, diff --git a/src/files/tests/mod.rs b/src/files/tests/mod.rs index 0cea779..42c7b2c 100644 --- a/src/files/tests/mod.rs +++ b/src/files/tests/mod.rs @@ -9,6 +9,7 @@ use crate::{lists::DirEntry, primitives::FocusNext, theme::Theme}; mod explorer; mod find; +mod stale; mod transfers; pub(super) fn setup(cx: &mut TestAppContext) { @@ -113,6 +114,20 @@ fn the_arrows_move_by_tile_and_by_row_and_enter_opens(cx: &mut TestAppContext) { assert_eq!(opened, ["g"]); } +#[gpui::test] +fn an_owner_pick_gone_from_the_grid_replaces_the_local_one(cx: &mut TestAppContext) { + let (view, cx) = tiled(None, cx); + cx.update(|window, cx| window.focus_next(cx)); + press("right", cx); + view.update(cx, |view, cx| { + view.selected = Some("Ghost.md".into()); + cx.notify(); + }); + settle(cx); + press("enter", cx); + assert_eq!(heard(&view, cx).1, Vec::::new()); +} + #[gpui::test] fn the_keys_start_from_the_tile_picked(cx: &mut TestAppContext) { let (view, cx) = tiled(Some("c"), cx); @@ -155,12 +170,6 @@ fn a_name_is_listed_once() { ); } -#[test] -#[should_panic(expected = "no entry z")] -fn the_pick_is_one_of_the_entries() { - let _ = FileGrid::new("grid", [DirEntry::file("a", 1, then())]).selected("z"); -} - /// One tile whose name runs long. struct Named; diff --git a/src/files/tests/stale.rs b/src/files/tests/stale.rs new file mode 100644 index 0000000..86b11b9 --- /dev/null +++ b/src/files/tests/stale.rs @@ -0,0 +1,115 @@ +use gpui::{ + Context, IntoElement, Modifiers, ParentElement, Render, Styled, TestAppContext, Window, div, + point, px, +}; + +use super::{settle, setup, then}; +use crate::{ + files::{ + FileExplorer, FileGrid, FileOperation, FileOperationProgress, FileView, TransferState, + }, + lists::DirEntry, +}; + +/// A path its folders no longer hold, gone picks, and bytes past their total. +struct Lagging; + +impl Render for Lagging { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let root = vec![DirEntry::file("Projects", 10, then())]; + let here = vec![DirEntry::file("Brief.md", 10, then())]; + div() + .w(px(640.0)) + .h(px(480.0)) + .child( + FileExplorer::new( + "explorer", + ["Home", "Projects"], + [root, here.clone()], + FileView::List, + ) + .selected("Ghost.md"), + ) + .child(FileGrid::new("grid", here).selected("Ghost.md")) + .child(FileOperationProgress::new( + "copy", + FileOperation::Copy, + 3, + "Backup", + (12, 10), + TransferState::Moving { rate: 4 }, + )) + } +} + +#[gpui::test] +fn stale_paths_picks_and_overruns_draw(cx: &mut TestAppContext) { + setup(cx); + let (_, cx) = cx.add_window_view(|_, _| Lagging); + settle(cx); +} + +/// Columns down a path whose folder became a file of the same name. +struct Replaced; + +impl Render for Replaced { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let root = vec![DirEntry::file("Projects", 10, then())]; + let here = vec![DirEntry::file("Brief.md", 10, then())]; + div().w(px(640.0)).h(px(480.0)).child(FileExplorer::new( + "explorer", + ["Home", "Projects"], + [root, here], + FileView::Columns, + )) + } +} + +#[gpui::test] +fn a_file_where_a_path_folder_stood_is_not_marked(cx: &mut TestAppContext) { + setup(cx); + let (_, cx) = cx.add_window_view(|_, _| Replaced); + settle(cx); + assert!(cx.debug_bounds("item-selected-Projects").is_none()); +} + +/// A list the owner may point at an entry the folder lost. +struct Listing { + selected: Option<&'static str>, +} + +impl Render for Listing { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let here = vec![ + DirEntry::file("Brief.md", 10, then()), + DirEntry::file("Notes.md", 10, then()), + ]; + let explorer = FileExplorer::new("explorer", ["Home"], [here], FileView::List); + div().w(px(640.0)).h(px(480.0)).child(match self.selected { + Some(name) => explorer.selected(name), + None => explorer, + }) + } +} + +#[gpui::test] +fn an_owner_pick_gone_from_the_list_replaces_the_local_one(cx: &mut TestAppContext) { + setup(cx); + let (view, cx) = cx.add_window_view(|_, _| Listing { selected: None }); + cx.update(|window, _| window.activate_window()); + settle(cx); + let header = cx.debug_bounds("listing-name").expect("the name header"); + let first = point(header.center().x, header.bottom() + px(12.0)); + cx.simulate_click(first, Modifiers::none()); + settle(cx); + assert!( + cx.debug_bounds("item-selected-Brief.md").is_some(), + "the press picks it" + ); + view.update(cx, |view, cx| { + view.selected = Some("Ghost.md"); + cx.notify(); + }); + settle(cx); + assert!(cx.debug_bounds("item-selected-Brief.md").is_none()); +} diff --git a/src/lists/directory.rs b/src/lists/directory.rs index 288512c..6e97a4e 100644 --- a/src/lists/directory.rs +++ b/src/lists/directory.rs @@ -149,20 +149,16 @@ impl DirectoryListing { } } - /// The entry picked, by name. + /// The entry picked, by name; one no longer listed marks none, yet still replaces a local pick. pub fn selected(mut self, name: impl Into) -> Self { let name = name.into(); - self.selected = self - .entries - .iter() - .any(|entry| entry.name == name) - .then(|| name.clone()); - if self.selected.is_none() { + if !self.entries.iter().any(|entry| entry.name == name) { log::error!( "directory listing {:?}: no entry {name}; none picked", self.id ); } + self.selected = Some(name); self } @@ -356,16 +352,7 @@ impl RenderOnce for DirectoryListing { mod tests { use jiff::Timestamp; - use super::{Column, DirEntry, DirectoryListing, sorted}; - - #[test] - fn a_selected_name_gone_from_the_entries_picks_none() { - let entries = || [DirEntry::file("a.txt", 1, Timestamp::UNIX_EPOCH)]; - let gone = DirectoryListing::new("dir", ["home"], entries()).selected("b.txt"); - assert_eq!(gone.selected, None); - let kept = DirectoryListing::new("dir", ["home"], entries()).selected("a.txt"); - assert_eq!(kept.selected.as_deref(), Some("a.txt")); - } + use super::{Column, DirEntry, sorted}; fn names(entries: Vec) -> Vec { entries diff --git a/tasks/panics.md b/tasks/panics.md index 214988e..51da3c8 100644 --- a/tasks/panics.md +++ b/tasks/panics.md @@ -41,7 +41,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca - [x] P24 Agent — `src/agent` - [x] P25 Generative — `src/generative` - [x] P26 Media — `src/media` -- [ ] P27 Files — `src/files` +- [x] P27 Files — `src/files` - [ ] P28 Messaging — `src/messaging` - [ ] P29 Mail — `src/mail` - [ ] P30 Calendar — `src/calendar` @@ -92,3 +92,4 @@ One line per item: what changed, what stayed and why, review rounds. - P24: a checkpoint or browser frame past its list logs and marks none; progress past its total and memory past its limit log and fill their bars, the text keeping the real counts; three `should_panic` tests became render tests. Kept: the artifact's preview-or-source builder check, one-value contracts (cost, cpu, pointer, totals and limits above zero), registry lookups over a constant table. Review: 1 round. - P25: a verdict, ratio, style, shot, voice, variation or prompt version gone from its list logs and marks none (no picture, no Vary or Upscale, no diff); embedding points in a gone group stay out; an epoch and GPU memory past their whole log and peg. A seed field holds a seed: each field handed in is fitted, and text that is no seed clears; in `TextInput`, a fit starts a fresh undo history, undo keeps and `set_text` fits against the committed text, never an input method's unfinished one (`committed`), and a lifted mask stroke stays on its picture. Kept: one-value contracts, unique group names, one snapshot's loss lists, two to four sides, the legend's lookup. Review: 5 rounds, the cap; round 5's finding (`set_text` over a composition) is fixed and tested, and P26's review checks it. - P26: a live position past its media logs and sits at the end (`media::played`); loaded time, a trim and chapters past the length peg or stay out; a gone device or track, or a refused trim, leaves none and clears the last; a dropped root path shows whole; subtitle times no clock holds read as none; a rail layout left no width gives no share (`scrubber::along`), so hover, press and drag skip it; waveform buckets count in `u64`, which a 32-bit web `usize` outgrows. Kept: non-zero lengths, one-value contracts, one list's uniqueness and order, lookups by construction. Review: 4 rounds; it also passed P25's last fix. +- P27: an owner's pick gone from a grid, an explorer folder or a directory listing logs and is kept as given, so it replaces a local pick and marks none (P13's listing now does the same); the columns mark a path name only while its parent holds it as a folder; moved bytes past a total log, the bar already capped; duplicate marks on no copy, or on every copy of a group, are dropped, so Remove never takes a group's last copy. Kept: one list's uniqueness, a snapshot's path and levels, items and downloads, lookups by construction, `datetime` with a constant pattern. Review: 3 rounds.