From 6e20f823602bdd87726e831c529d53fd2b46ab24 Mon Sep 17 00:00:00 2001 From: squareSphere29 <18124696+squareSphere29@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:02:42 +0800 Subject: [PATCH] LR-LIB-STACK: deleting a stacked photo drops it from the stack so i/n badges update (fixes #1493) Soft-delete and remove-folder now unstack in the same undo step as Recently Deleted, so leftover members show 1/2 not 3/3. Restore-from-trash does not restack. Co-authored-by: Cursor --- crates/engine/src/cmd/library.rs | 21 +++++++++++++++++---- crates/engine/src/tests_folders.rs | 15 +++++++++++++++ crates/engine/src/tests_import.rs | 2 +- crates/engine/src/tests_organize.rs | 23 +++++++++++++++++++++++ docs/parity-checklist.md | 6 +++--- 5 files changed, 59 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 3a5575ea8..917600580 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); @@ -436,8 +449,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; } @@ -779,9 +791,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 ed4cc9162..44172b555 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 76d91ba3e..38daac7ab 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (LR-LIB-STACK / LR-LIB-VIRTUALCOPY: deleting a stacked photo or removing its folder drops it from the stack so `i/n` badges update (#1493); LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -105,7 +105,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-LIB-LABEL | Colour labels | P1 | ✅ | `cmd:photo.label`, `cmd:label.setNames`, `cmd:label.names`, `cmd:dialog.labelNames`, keys 6–9 in `crates/ui-egui/src/shortcuts.rs` | Photo menu and grid context menu (coloured, named), Info-panel swatches, label dot plus translucent colour on square-grid/filmstrip surrounds and photo-grid footers (photo pixels unchanged); colour-label toasts have pale matching backgrounds; setting/clearing through keys (including Shift), menus or Info swatches shows the same bottom toast as ratings, with the colour/custom name; editable label names (undoable, journaled); no purple key (as in Lightroom), no custom extra labels | | LR-LIB-KEYWORD | Keywords | P0 | ✅ | `cmd:panel.keywords`, `cmd:photo.setMeta` (`addKeywords`/`removeKeywords`), `cmd:keyword.list`, `cmd:keyword.rename`, `cmd:keyword.delete`, `cmd:keyword.merge`, `cmd:keyword.suggest`, `crates/catalog/src/keywords.rs`, `crates/ui-egui/src/panels/left.rs` | left-panel keyword tree with counts (click filters, children included; context menu rename / merge / delete / add / remove); library-wide ops are one undo step and replay from the op log; suggestions (co-occurring / most used / completions) in the Keywords panel; keywords reorganize by drag and drop in the Keyword List (LRC-LIB-KEYWORDS) | | LR-LIB-PEOPLE | People / faces | P2 | 🟡 | `cmd:faces.detect`, `cmd:view.people`, `cmd:view.faceBoxes`, `cmd:photo.removeRegion`, `cmd:photo.setRegion`, `cmd:library.filter`, `crates/ui-egui/src/panels/people.rs`, `crates/catalog/src/query.rs` (`people`), `crates/engine/src/media.rs` (`face_job`), `crates/meta/src/xmp.rs` | MVP: named face regions read from XMP (MWG-RS, read-only, see `docs/xmp-interop.md`; clipped to the photo; Lightroom's `Rotation` of ±π/2 and π handled, mirrored orientations taken as written; boxes follow Rotate Left/Right and flips; a sidecar stating `mwg-rs:Regions`, even empty, replaces the photo's on re-read, one without leaves them) become cards in the People view (View ▸ People, or the People button in the toolbar): a close-up of the person's largest face, name and photo count; a click filters the grid (`person` in `library.filter`, `person:` search token, filter chip). Face boxes in the loupe can be switched off (View ▸ Face Boxes, toolbar) resized with eight drag handles and removed with the × shown on hover (catalog-only and undoable, one undo step per drag; the sidecar is never rewritten for either); no moving a box by dragging it yet. Photo ▸ Detect Faces (`cmd:faces.detect`) finds faces with the YuNet detector (pure Rust, about 200 ms a photo, matches OpenCV's own YuNet on a 45-photo test set; the 232 KB model is a one-time download offered in the same step, never part of LightCraft) and adds them as unnamed boxes; names are suggested from the faces already named (recognition models are opt-in downloads, run on the CPU in a background thread; a face is named only when you confirm it); a person's page shows their cropped faces and the unnamed faces that look like them (LRC-LIB-PEOPLE); with recognition on, a background scan finds faces in photos that have none and embeds every face (one to three worker threads, raws through their embedded preview); an Unnamed faces section orders faces by likeness and names a selected group at once; no separated clusters, no manual tagging, no writing regions back | -| LR-LIB-STACK | Stacks | P1 | ✅ | `cmd:stack.group`, `cmd:stack.ungroup`, `cmd:stack.toggle`, `cmd:stack.setTop`, `cmd:stack.remove`, `cmd:stack.auto`, `crates/catalog/src/stacks.rs` | grid/filmstrip count badges, expand/collapse, auto-stack by capture time; no visual-similarity auto-stack | +| LR-LIB-STACK | Stacks | P1 | ✅ | `cmd:stack.group`, `cmd:stack.ungroup`, `cmd:stack.toggle`, `cmd:stack.setTop`, `cmd:stack.remove`, `cmd:stack.auto`, `cmd:photo.delete`, `crates/catalog/src/stacks.rs` | grid/filmstrip count badges, expand/collapse, auto-stack by capture time; delete / remove folder drop the photo from the stack so `i/n` updates (#1493); no visual-similarity auto-stack | | LR-LIB-VERSIONS | Versions | P1 | ✅ | `cmd:version.create` | see section M | | LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | Delete and Delete Permanently retain surviving selected photos; if none remain, select the next visible survivor (previous at the end) in the current source/filter/sort; permanent deletion removes catalog records, not files on disk; Restore and Delete Permanently in the Photo menu and the photo context menus for photos in Recently Deleted (before, the UI had no way to restore); importing a file that is in Recently Deleted again restores the photo with its edits, rating and albums (issues #298, #706: before, the import skipped it as "already in the library" while the grid showed it deleted) — the review dialog checks it, badges it "Will be restored", counts it as "in Recently Deleted" rather than "already in the library", and its choice Restore them / Import as new / Leave them starts on Restore; with Leave them such a file starts unchecked and Check All and the file-type toggles leave it out, but a click still checks it and it is restored (issue #1285: before, the click did nothing and the "In Recently Deleted" badge was drawn over the checkbox, which now sits above it); dropped files restore too ("Restored 1 photo from Recently Deleted"); a deleted photo matched by content whose file is gone is relinked to the imported file; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` restore (default) / skip / fresh for files in the trash (`cmd:library.import`; the report lists `restored`, and with skip flags the duplicate `existingDeleted`); browsing a folder in Local (`cmd:library.browse`), Auto Import and a Lightroom catalog import never restore a deleted photo; no confirmation dialog, no auto-purge after N days (needs a deleted-at date in the catalog); Delete asks first ("Move this photo to Recently Deleted?") from the key, the Photo menu and the photo context menu, on by default and switched off in Settings ▸ General (#1239: before, it was off by default and the context menu never asked); no auto-purge after N days (needs a deleted-at date in the catalog) | | LR-LIB-REMOVE-ALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos` | | @@ -117,7 +117,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)