Panics · P27 Files: gone owner picks replace local ones and mark none, stale path folders go unmarked, duplicate marks never take a last copy

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) =

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<Item = impl Into<SharedString>>) -> Self {
-        let paths: Vec<SharedString> = 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<SharedString> = 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"]);
+    }
+}

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<SharedString>) -> 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
     }

src/files/grid.rs

@@ -65,10 +65,9 @@ impl FileGrid {
     /// The entry picked, by name.
     pub fn selected(mut self, name: impl Into<SharedString>) -> 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
     }

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,

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<SharedString>,
@@ -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,
-    );
-}

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<SharedString>,

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::<SharedString>::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;
 

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<Self>) -> 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<Self>) -> 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<Self>) -> 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());
+}

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<SharedString>) -> 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<DirEntry>) -> Vec<String> {
         entries