diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index ff91a89b8..1cc0453a9 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -167,6 +167,19 @@ fn album_targets_or_err(s: &Session, command: &str, targets: Vec) -> Re Ok((kept, skipped)) } +/// Move to Recently Deleted and drop the photos from stacks so leftover members do not keep +/// showing `i/n` (issue #1493). +fn mark_deleted(s: &mut Session, ids: &[PhotoId], label: &str) -> Result { + let mut ops: Vec = ids.iter().map(|id| Op::SetDeleted { id: *id, deleted: true }).collect(); + let n = ops.len(); + if n == 0 { + return Ok(0); + } + ops.extend(s.catalog.remove_from_stacks_ops(ids)); + s.commit(label, Op::Batch { ops })?; + Ok(n) +} + /// Apply one op per target as a single undo step. fn for_targets(s: &mut Session, p: &Value, label: &str, f: impl Fn(PhotoId) -> Option) -> Result { let targets = s.targets(p); @@ -459,8 +472,7 @@ pub fn specs() -> Vec { if covers_other_disks(s, path, &ids) && !bool_or(p, "disk", false) { return Err(bad(C, format!("{path} holds whole disks: pass `disk: true` to remove everything on them"))); } - let ops = ids.iter().map(|id| Op::SetDeleted { id: *id, deleted: true }).collect(); - s.commit("Remove Folder from Library", Op::Batch { ops })?; + mark_deleted(s, &ids, "Remove Folder from Library")?; if s.filter.library_folder.as_deref().is_some_and(|c| lightcraft_catalog::query::folder_within(c, path)) { s.filter.library_folder = None; } @@ -874,9 +886,10 @@ pub fn specs() -> Vec { // ---- delete / restore cmd!("photo.delete", "Delete Photo", ["Photo"], Some("Delete"), "{ids?} — moves to Recently Deleted", has_selection, |s, p| { let before = s.visible_cloned(); - let v = for_targets(s, p, "Delete", |id| Some(Op::SetDeleted { id, deleted: true }))?; + let targets = s.targets(p); + let n = mark_deleted(s, &targets, "Delete")?; select_after_deletion(s, &before); - Ok(v) + Ok(json!({"changed": n})) }), cmd!("photo.restore", "Restore", ["Photo"], None, "{ids?}", has_selection, |s, p| for_targets(s, p, "Restore", |id| Some(Op::SetDeleted { id, diff --git a/crates/engine/src/tests_folders.rs b/crates/engine/src/tests_folders.rs index 697c98087..34017fc97 100644 --- a/crates/engine/src/tests_folders.rs +++ b/crates/engine/src/tests_folders.rs @@ -106,6 +106,21 @@ fn removing_a_folder_moves_its_photos_to_recently_deleted_and_undo_brings_them_b assert_eq!(s.visible().len(), 3, "one undo step"); } +#[test] +fn removing_a_folder_drops_its_photos_from_stacks() { + let mut s = session(); + let trip: Vec<_> = + s.catalog.photos().filter(|p| matches!(&p.source, Source::File { path } if path.contains("/pics/trip/"))).map(|p| p.id).collect(); + assert_eq!(trip.len(), 2); + s.execute("library.select", &json!({"ids": [trip[0].0, trip[1].0], "active": trip[0].0})).unwrap(); + s.execute("stack.group", &json!({})).unwrap(); + assert_eq!(s.catalog.stack_of(trip[0]).unwrap().photos.len(), 2); + s.execute("library.removeFolder", &json!({"path": "/pics/trip"})).unwrap(); + assert!(s.catalog.stack_of(trip[0]).is_none() && s.catalog.stack_of(trip[1]).is_none()); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.catalog.stack_of(trip[0]).unwrap().photos.len(), 2); +} + #[test] fn removing_a_folder_that_holds_nothing_or_a_whole_disk_is_refused() { let mut s = session(); diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index d49426039..11fd72055 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -1119,7 +1119,7 @@ fn emptying_trashed_photos_of_one_album_and_stack_leaves_nothing_dangling() { s.execute("edit.undo", &json!({})).unwrap(); assert_eq!(s.catalog.len(), 2, "one undo step brings both back"); assert_eq!(s.catalog.album(lightcraft_catalog::AlbumId(al)).unwrap().photos.len(), 2); - assert!(s.catalog.stack_of(lightcraft_catalog::PhotoId(ids[0])).is_some()); + assert!(s.catalog.stack_of(lightcraft_catalog::PhotoId(ids[0])).is_none(), "soft-delete already left the stack"); assert!(s.catalog.photos().all(|p| p.deleted), "back in Recently Deleted"); let _ = std::fs::remove_dir_all(&src); } diff --git a/crates/engine/src/tests_organize.rs b/crates/engine/src/tests_organize.rs index 80d1ae546..8990acb4c 100644 --- a/crates/engine/src/tests_organize.rs +++ b/crates/engine/src/tests_organize.rs @@ -141,6 +141,29 @@ fn virtual_copies_are_independent_and_persist() { let _ = std::fs::remove_dir_all(&dir); } +/// Issue #1493: soft-delete must drop the copy from the stack so `1/2` does not linger, and +/// deleting `2/3` renumbers the rest to `1/2` `2/2`. Undo restores the stack. +#[test] +fn deleting_a_virtual_copy_drops_it_from_the_stack() { + let mut s = Session::with_demo(); + let src = s.visible_cloned()[0]; + s.execute("library.select", &json!({"ids": [src.0]})).unwrap(); + let c1 = lightcraft_catalog::PhotoId(s.execute("photo.virtualCopy", &json!({})).unwrap()["ids"][0].as_u64().unwrap()); + let c2 = lightcraft_catalog::PhotoId(s.execute("photo.virtualCopy", &json!({"ids": [src.0]})).unwrap()["ids"][0].as_u64().unwrap()); + assert_eq!(s.catalog.stack_of(src).unwrap().photos, vec![src, c2, c1]); + s.execute("photo.delete", &json!({"ids": [c2.0]})).unwrap(); + let left = s.catalog.stack_of(src).expect("two members remain stacked"); + assert_eq!(left.photos, vec![src, c1]); + assert_eq!((left.position(src), left.position(c1), left.photos.len()), (Some(0), Some(1), 2), "1/2 and 2/2"); + assert!(s.catalog.photo(c2).unwrap().deleted); + s.execute("photo.delete", &json!({"ids": [c1.0]})).unwrap(); + assert!(s.catalog.stack_of(src).is_none(), "the original is unstacked once it is the only member"); + assert!(s.catalog.photo(c1).unwrap().deleted); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(!s.catalog.photo(c1).unwrap().deleted); + assert_eq!(s.catalog.stack_of(src).unwrap().photos, vec![src, c1]); +} + #[test] fn stacks_group_collapse_and_replay() { let dir = temp_dir("stacks"); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 4e575aac7..0ce441574 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -121,7 +121,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-LIB-CULL | Assisted culling | P2 | 🟡 | `cmd:photo.analyze`, `cmd:dialog.cull`, `crates/pipeline/src/cull.rs` | Photo ▸ Assisted Culling…: focus score (0–100), clipping, burst grouping (look-alike shots within 10 s) with the sharpest marked; reject below a focus score, pick each burst's best; classical measures — no eyes-closed / expression detection | | LR-LIB-ACTIVITY | Comments & likes | OOS | 🚫 | | | | LR-LIB-QUICKCOLL | Quick collection [Classic] | P2 | ✅ | `cmd:album.toggleTarget`, `cmd:album.clearQuick` | B in the grids | -| LR-LIB-VIRTUALCOPY | Virtual copies [Classic] | P1 | ✅ | `cmd:photo.virtualCopy`, `crates/engine/src/cmd/organize.rs` | “Copy N” badge, stacked with the original, same albums, no XMP writes; no “Set Copy as Master” | +| LR-LIB-VIRTUALCOPY | Virtual copies [Classic] | P1 | ✅ | `cmd:photo.virtualCopy`, `cmd:photo.delete`, `crates/engine/src/cmd/organize.rs` | “Copy N” badge, stacked with the original, same albums, no XMP writes; deleting a copy dissolves or renumbers the stack (#1493); no “Set Copy as Master” | ## C. Views & navigation (VIEW)