diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index f1ce020e..04ec01f0 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -7,6 +7,20 @@ use serde_json::{Value, json}; use super::{CommandSpec, always, bad, bool_or, cmd, has_active, has_selection, ok, str_param}; use crate::{LibrarySource, Result, Selection, Session}; +fn select_after_deletion(s: &mut Session, before: &[PhotoId]) { + let at = before.iter().position(|id| Some(*id) == s.selection.active); + let remaining: std::collections::HashSet<_> = s.visible_cloned().into_iter().collect(); + s.selection.ids.retain(|id| remaining.contains(id)); + s.selection.active = s.selection.active.filter(|id| remaining.contains(id)).or_else(|| s.selection.ids.first().copied()); + if s.selection.active.is_none() + && let Some(at) = at + { + // Keep culling near the deleted photo: next visible survivor, or the previous one at the end. + let neighbour = before.iter().skip(at.saturating_add(1)).chain(before.iter().take(at).rev()).find(|id| remaining.contains(id)); + s.selection = neighbour.map(|id| Selection::single(*id)).unwrap_or_default(); + } +} + /// The auto-import folder's visible files with their sizes (the file-system half of /// `library.autoImportScan`; the app lists on a worker thread and passes `listing`). pub fn list_auto_import_folder(folder: &str) -> std::result::Result, String> { @@ -686,18 +700,8 @@ 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 at = before.iter().position(|id| Some(*id) == s.selection.active); let v = for_targets(s, p, "Delete", |id| Some(Op::SetDeleted { id, deleted: true }))?; - let remaining: std::collections::HashSet<_> = s.visible_cloned().into_iter().collect(); - s.selection.ids.retain(|id| remaining.contains(id)); - s.selection.active = s.selection.active.filter(|id| remaining.contains(id)).or_else(|| s.selection.ids.first().copied()); - if s.selection.active.is_none() - && let Some(at) = at - { - // Keep culling near the deleted photo: next visible survivor, or the previous one at the end. - let neighbour = before.iter().skip(at.saturating_add(1)).chain(before.iter().take(at).rev()).find(|id| remaining.contains(id)); - s.selection = neighbour.map(|id| Selection::single(*id)).unwrap_or_default(); - } + select_after_deletion(s, &before); Ok(v) }), cmd!("photo.restore", "Restore", ["Photo"], None, "{ids?}", has_selection, |s, p| for_targets(s, p, "Restore", |id| Some(Op::SetDeleted { @@ -723,10 +727,11 @@ pub fn specs() -> Vec { } ), cmd!("photo.deletePermanently", "Delete Permanently", ["Photo"], None, "{ids?}", has_selection, |s, p| { + let before = s.visible_cloned(); let t = s.targets(p); let op = s.catalog.delete_photos_permanently_ops(&t); s.commit("Delete Permanently", op)?; - s.selection = Selection::default(); + select_after_deletion(s, &before); Ok(json!({"deleted": t.len()})) }), // ---- metadata diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index b4fdc1cc..d51122e3 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -199,6 +199,60 @@ fn deleting_explicit_targets_keeps_the_surviving_selection() { assert_eq!(s.selection, Selection::single(visible[8]), "partial deletion retains a selected survivor"); } +#[test] +fn permanently_deleting_photos_selects_a_neighbour_in_recently_deleted() { + for params in [json!({}), json!({"key": "fileName", "ascending": false}), json!({"key": "random", "seed": 123})] { + for at_end in [false, true] { + let mut s = demo(); + s.execute("library.selectAll", &json!({})).unwrap(); + s.execute("photo.delete", &json!({})).unwrap(); + s.execute("library.source", &json!({"kind": "recentlyDeleted"})).unwrap(); + s.execute("library.sort", ¶ms).unwrap(); + s.execute("library.filter", &json!({"rating": 2})).unwrap(); + let visible = s.visible_cloned(); + assert!(visible.len() > 5); + let at = if at_end { visible.len() - 2 } else { visible.len() / 2 }; + let deleted = &visible[at..at + 2]; + s.execute("library.select", &json!({"ids": deleted, "active": deleted[0]})).unwrap(); + let snapshot = s.catalog.to_snapshot(); + let undo_len = s.undo.len(); + let r = s.execute("photo.deletePermanently", &json!({})).unwrap(); + assert_eq!(r["deleted"], 2); + let expected = if at_end { visible[at - 1] } else { visible[at + 2] }; + assert_eq!(s.selection, Selection::single(expected), "sort {params}, end {at_end}"); + assert!(deleted.iter().all(|id| s.catalog.photo(*id).is_none())); + assert_eq!(s.undo.len(), undo_len + 1); + let removed = s.catalog.to_snapshot(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.catalog.to_snapshot(), snapshot); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!(s.catalog.to_snapshot(), removed); + } + } + let mut s = demo(); + s.execute("library.selectAll", &json!({})).unwrap(); + s.execute("photo.deletePermanently", &json!({})).unwrap(); + assert!(s.visible().is_empty()); + assert_eq!(s.selection, Selection::default()); +} + +#[test] +fn permanently_deleting_explicit_targets_keeps_the_surviving_selection() { + let mut s = demo(); + let visible = s.visible_cloned(); + s.execute("library.select", &json!({"ids": [visible[8], visible[9]], "active": visible[9]})).unwrap(); + let before = s.selection.clone(); + s.execute("photo.deletePermanently", &json!({"ids": [visible[3]]})).unwrap(); + assert_eq!(s.selection, before, "deleting an unselected photo doesn't change selection"); + s.execute("photo.deletePermanently", &json!({"ids": [visible[9]]})).unwrap(); + assert_eq!(s.selection, Selection::single(visible[8]), "partial deletion retains a selected survivor"); + let before = s.selection.clone(); + let snapshot = s.catalog.to_snapshot(); + assert!(s.execute("photo.deletePermanently", &json!({"ids": [999_999]})).is_err()); + assert_eq!(s.selection, before, "a refused deletion doesn't change selection"); + assert_eq!(s.catalog.to_snapshot(), snapshot); +} + #[test] fn masks_crop_and_render() { let mut s = demo(); diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index 4a4cad53..4937010a 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -974,8 +974,27 @@ fn emptying_trashed_photos_of_one_album_and_stack_leaves_nothing_dangling() { fn deleting_several_photos_permanently_leaves_nothing_dangling() { let (mut s, src, ids, al) = two_trashed_in_album_and_stack("trash-perm-refs"); s.execute("library.select", &json!({"ids": ids})).unwrap(); + let files = [src.join("a.png"), src.join("b.png")].map(|path| { + let bytes = std::fs::read(&path).unwrap(); + (path, bytes) + }); + let snapshot = s.catalog.to_snapshot(); + s.drain_log(); s.execute("photo.deletePermanently", &json!({"ids": ids})).unwrap(); assert!(s.catalog.album(lightcraft_catalog::AlbumId(al)).unwrap().photos.is_empty()); + assert!(ids.iter().all(|i| s.catalog.photo(lightcraft_catalog::PhotoId(*i)).is_none())); + let removed = s.catalog.to_snapshot(); + let log: String = s.drain_log().iter().map(lightcraft_catalog::Catalog::op_to_log_line).collect(); + let mut reopened = lightcraft_catalog::Catalog::from_snapshot(&snapshot).unwrap(); + reopened.replay(&log).unwrap(); + assert_eq!(reopened.to_snapshot(), removed); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.catalog.to_snapshot(), snapshot, "one undo restores photos, album and stack"); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!(s.catalog.to_snapshot(), removed); + for (path, bytes) in files { + assert_eq!(std::fs::read(path).unwrap(), bytes, "permanent deletion only removes catalog records"); + } let _ = std::fs::remove_dir_all(&src); } diff --git a/crates/ui-egui/src/tests_scroll.rs b/crates/ui-egui/src/tests_scroll.rs index 6ccb0d33..1eff1ec7 100644 --- a/crates/ui-egui/src/tests_scroll.rs +++ b/crates/ui-egui/src/tests_scroll.rs @@ -111,6 +111,44 @@ fn deleting_photos_does_not_jump_the_grid_to_the_top() { } } +#[test] +fn permanently_deleting_photos_keeps_the_current_view_neighbour_in_sight() { + for view in ["photoGrid", "squareGrid", "detail"] { + for count in [1, 3] { + let mut h = demo(view); + for (command, params) in + [("library.selectAll", json!({})), ("photo.delete", json!({})), ("library.source", json!({"kind": "recentlyDeleted"}))] + { + let r = h.request("engine.execute", json!({"command": command, "params": params}), T); + assert_eq!(r["ok"], true, "{r}"); + } + let ids = h.app.session.visible_cloned(); + let at = ids.len() / 2; + let r = h.request("engine.execute", json!({"command": "library.select", "params": {"ids": &ids[at..at + count]}}), T); + assert_eq!(r["ok"], true, "{r}"); + idle(&mut h, 30); + let scroll = |h: &Headless| if view == "detail" { film_x(h) } else { grid_y(h) }; + let before = scroll(&h); + assert!(before > 600.0, "selection is far from the beginning: {view}, {before}"); + let r = h.request("ui.menu.invoke", json!({"id": "photo.deletePermanently"}), T); + assert_eq!(r["ok"], true, "{r}"); + idle(&mut h, 60); + let next = ids[at + count]; + assert_eq!(h.app.session.active(), Some(next), "{view}, count {count}"); + assert!(scroll(&h) > 600.0, "deleting scrolled to the beginning: {view}, {before} → {}", scroll(&h)); + let cell = widget(&h, &format!("{}:{}", if view == "detail" { "film" } else { "thumb" }, next.0)); + if view == "detail" { + assert!(cell.left() >= 0.0 && cell.right() <= 1200.0, "the next survivor is in view: {cell:?}"); + } else { + assert!(cell.intersects(h.app.canvas_rect.unwrap()), "the next survivor is in view"); + } + let after = scroll(&h); + idle(&mut h, 60); + assert_eq!(scroll(&h), after, "idle frames preserve the new position"); + } + } +} + #[test] fn filmstrip_wheel_scrolls_and_keeps_its_position() { let mut h = demo("detail"); diff --git a/docs/parity.md b/docs/parity.md index 1e9f1a41..b8c71608 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -168,7 +168,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | 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-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` | Deletion retains surviving selected photos; if none remain, selects the next visible survivor (previous at the end) in the current source/filter/sort instead of jumping to the first photo; 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); adding a file that is in Recently Deleted again selects it there (side panel opened) and says how to restore it or import it afresh; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` skip / restore / fresh for files in the trash (`cmd:library.import`; the report flags `existingDeleted` and lists `restored`; the import review dialog offers Leave them / Restore them / Import as new for files that are in the trash; dropped files and Add Photos without a review still skip and point at Restore); no confirmation dialog, no auto-purge after N days (needs a deleted-at date in the catalog) | +| 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); adding a file that is in Recently Deleted again selects it there (side panel opened) and says how to restore it or import it afresh; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` skip / restore / fresh for files in the trash (`cmd:library.import`; the report flags `existingDeleted` and lists `restored`; the import review dialog offers Leave them / Restore them / Import as new for files that are in the trash; dropped files and Add Photos without a review still skip and point at Restore); no confirmation dialog, 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` | | | LR-LIB-DUPLICATE | Duplicate a photo | P2 | ✅ | `cmd:photo.duplicate` | Photo ▸ Duplicate: a real `-copy` file with the same settings, metadata and albums (virtual copies share the file) | | LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:photo.renameTokens`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` (`TOKENS`) | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming and import rename); a Tags picker beside every template field (Rename Photos, import Rename, export File name) lists each tag with meaning + live example, the date directives and the rules, and inserts the clicked tag at the text cursor; unknown tags are flagged; preview; renames files on disk with their XMP sidecars (a stem sidecar another file still shares, e.g. raw + JPEG, is copied rather than taken), never overwriting (-1, -2… suffixes; a change of letter case only is checked by file identity, so on case-sensitive volumes `img_1.jpg` next to `IMG_1.JPG` is a collision), rolls back on failure (a file that can't be moved back is reported old → new and the library follows it as an undoable partial rename; the same for undo/redo); undo/redo move the files; virtual copies follow |