mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-31 21:48:53 -06:00
LR-LIB-VIRTUALCOPY: check numbering before copy creation
This commit is contained in:
@@ -3,6 +3,7 @@
|
||||
|
||||
use lightcraft_catalog::{HistoryStep, Op, PhotoId, Stack, StackId};
|
||||
use serde_json::{Value, json};
|
||||
use std::collections::HashMap;
|
||||
|
||||
use super::{CommandSpec, always, bad, bool_or, cmd, f64_or, has_selection, str_param};
|
||||
use crate::{Result, Selection, Session};
|
||||
@@ -63,18 +64,53 @@ fn copy_number(name: Option<&str>) -> u32 {
|
||||
name.and_then(|n| n.strip_prefix("Copy ")).and_then(|n| n.trim().parse().ok()).unwrap_or(0)
|
||||
}
|
||||
|
||||
/// Resolve every requested name before creating copies, so an exhausted counter cannot leave a
|
||||
/// partially completed batch. Explicit names do not depend on the numeric sequence.
|
||||
fn virtual_copy_names(s: &Session, targets: &[PhotoId], name: Option<&str>) -> Result<Vec<Option<String>>> {
|
||||
if let Some(name) = name {
|
||||
return Ok(targets.iter().map(|id| s.catalog.photo(*id).map(|_| name.to_string())).collect());
|
||||
}
|
||||
|
||||
let mut last_by_master = HashMap::new();
|
||||
for id in targets {
|
||||
if let Some(photo) = s.catalog.photo(*id) {
|
||||
last_by_master.entry(photo.copy_of.unwrap_or(*id)).or_insert(0_u32);
|
||||
}
|
||||
}
|
||||
for photo in s.catalog.photos() {
|
||||
if let Some(master) = photo.copy_of
|
||||
&& let Some(last) = last_by_master.get_mut(&master)
|
||||
{
|
||||
*last = (*last).max(copy_number(photo.copy_name.as_deref()));
|
||||
}
|
||||
}
|
||||
|
||||
targets
|
||||
.iter()
|
||||
.map(|id| {
|
||||
let Some(photo) = s.catalog.photo(*id) else { return Ok(None) };
|
||||
let master = photo.copy_of.unwrap_or(*id);
|
||||
let last = last_by_master.get_mut(&master).ok_or_else(|| bad("photo.virtualCopy", "could not resolve the virtual-copy master"))?;
|
||||
let number = last.checked_add(1).ok_or_else(|| {
|
||||
bad("photo.virtualCopy", format!("default virtual-copy numbering is exhausted for master {}; provide an explicit name", master.0))
|
||||
})?;
|
||||
*last = number;
|
||||
Ok(Some(format!("Copy {number}")))
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Create one virtual copy of `src`: a new photo sharing its file, with the same settings and
|
||||
/// metadata, added to the same albums and stacked right after it (the stack is expanded so the
|
||||
/// copy is visible). `name` replaces the default "Copy N".
|
||||
fn virtual_copy(s: &mut Session, src: PhotoId, name: Option<&str>) -> Result<Option<PhotoId>> {
|
||||
/// copy is visible). `name` is resolved by the command before any copies are created.
|
||||
fn virtual_copy(s: &mut Session, src: PhotoId, name: &str) -> Result<Option<PhotoId>> {
|
||||
let Some(orig) = s.catalog.photo(src).cloned() else { return Ok(None) };
|
||||
let master = orig.copy_of.unwrap_or(src);
|
||||
let n = 1 + s.catalog.photos().filter(|p| p.copy_of == Some(master)).map(|p| copy_number(p.copy_name.as_deref())).max().unwrap_or(0);
|
||||
let id = s.catalog.alloc_photo_id();
|
||||
let mut c = (*orig).clone();
|
||||
c.id = id;
|
||||
c.copy_of = Some(master);
|
||||
c.copy_name = Some(name.map_or_else(|| format!("Copy {n}"), str::to_string));
|
||||
c.copy_name = Some(name.to_string());
|
||||
c.versions.clear();
|
||||
c.history = vec![HistoryStep { label: "Virtual Copy".into(), settings: c.develop.clone() }];
|
||||
c.deleted = false;
|
||||
@@ -113,9 +149,12 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
|s, p| {
|
||||
let targets = s.targets(p)?;
|
||||
let name = str_param(p, "name").filter(|n| !n.trim().is_empty()).map(str::to_string);
|
||||
let names = virtual_copy_names(s, &targets, name.as_deref())?;
|
||||
let mut made = Vec::new();
|
||||
for t in targets {
|
||||
if let Some(id) = virtual_copy(s, t, name.as_deref())? {
|
||||
for (t, name) in targets.into_iter().zip(names) {
|
||||
if let Some(name) = name
|
||||
&& let Some(id) = virtual_copy(s, t, &name)?
|
||||
{
|
||||
made.push(id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
use lightcraft_catalog::Flag;
|
||||
use serde_json::json;
|
||||
|
||||
use crate::{LibrarySource, Session};
|
||||
use crate::{EngineError, LibrarySource, Session};
|
||||
|
||||
fn temp_dir(tag: &str) -> std::path::PathBuf {
|
||||
let d = std::env::temp_dir().join(format!("lc-organize-{tag}-{}", std::process::id()));
|
||||
@@ -18,6 +18,13 @@ fn open(dir: &std::path::Path) -> Session {
|
||||
s
|
||||
}
|
||||
|
||||
fn leave_redo_entry(s: &mut Session, id: lightcraft_catalog::PhotoId) {
|
||||
let rating = if s.catalog.photo(id).unwrap().rating == 5 { 4 } else { 5 };
|
||||
s.execute("photo.rate", &json!({"ids": [id.0], "rating": rating})).unwrap();
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert!(!s.redo.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn smart_album_from_view_updates_live_and_persists() {
|
||||
let dir = temp_dir("smart");
|
||||
@@ -141,6 +148,119 @@ fn virtual_copies_are_independent_and_persist() {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn virtual_copy_explicit_name_bypasses_exhausted_counter() {
|
||||
let mut s = Session::with_demo();
|
||||
let visible = s.visible_cloned();
|
||||
let src = visible[0];
|
||||
s.execute("photo.virtualCopy", &json!({"ids": [src.0], "name": "Copy 4294967295"})).unwrap();
|
||||
|
||||
let result = s.execute("photo.virtualCopy", &json!({"ids": [src.0], "name": "Proof copy"}));
|
||||
assert!(result.is_ok(), "an explicit name does not need a numeric suffix: {result:?}");
|
||||
let id = lightcraft_catalog::PhotoId(result.unwrap()["ids"][0].as_u64().unwrap());
|
||||
let copy = s.catalog.photo(id).unwrap();
|
||||
assert_eq!(copy.copy_of, Some(src));
|
||||
assert_eq!(copy.copy_name.as_deref(), Some("Proof copy"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn virtual_copy_exhausted_counter_returns_bad_params() {
|
||||
let mut s = Session::with_demo();
|
||||
let visible = s.visible_cloned();
|
||||
let src = visible[0];
|
||||
s.execute("photo.virtualCopy", &json!({"ids": [src.0], "name": "Copy 4294967295"})).unwrap();
|
||||
leave_redo_entry(&mut s, visible[1]);
|
||||
|
||||
let before = s.catalog.to_snapshot();
|
||||
let revision = s.catalog.revision;
|
||||
let selection = (s.selection.ids.clone(), s.selection.active);
|
||||
let commits = s.commits();
|
||||
let undo = s.undo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>();
|
||||
let redo = s.redo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>();
|
||||
|
||||
let result = s.execute("photo.virtualCopy", &json!({"ids": [src.0]}));
|
||||
let actionable =
|
||||
matches!(&result, Err(EngineError::BadParams { cmd, msg }) if cmd == "photo.virtualCopy" && msg.contains("provide an explicit name"));
|
||||
assert_eq!(s.catalog.to_snapshot(), before, "exhaustion must not allocate IDs or mutate the catalog");
|
||||
assert_eq!(s.catalog.revision, revision);
|
||||
assert_eq!((s.selection.ids.clone(), s.selection.active), selection);
|
||||
assert_eq!(s.commits(), commits);
|
||||
assert_eq!(s.undo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>(), undo);
|
||||
assert_eq!(s.redo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>(), redo);
|
||||
assert!(actionable, "expected actionable parameter error, got {result:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn virtual_copy_numbering_failure_does_not_partially_change_batch() {
|
||||
let mut s = Session::with_demo();
|
||||
let visible = s.visible_cloned();
|
||||
let (first, second) = (visible[0], visible[1]);
|
||||
s.execute("photo.virtualCopy", &json!({"ids": [second.0], "name": "Copy 4294967295"})).unwrap();
|
||||
leave_redo_entry(&mut s, visible[2]);
|
||||
s.execute("library.select", &json!({"ids": [first.0, second.0], "active": first.0})).unwrap();
|
||||
|
||||
let before = s.catalog.to_snapshot();
|
||||
let revision = s.catalog.revision;
|
||||
let selection = (s.selection.ids.clone(), s.selection.active);
|
||||
let commits = s.commits();
|
||||
let undo = s.undo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>();
|
||||
let redo = s.redo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>();
|
||||
|
||||
let result = s.execute("photo.virtualCopy", &json!({"ids": [first.0, second.0]}));
|
||||
let bad_params = matches!(&result, Err(EngineError::BadParams { cmd, .. }) if cmd == "photo.virtualCopy");
|
||||
assert_eq!(s.catalog.to_snapshot(), before, "a later exhausted master must not leave earlier copies behind");
|
||||
assert_eq!(s.catalog.revision, revision);
|
||||
assert_eq!((s.selection.ids.clone(), s.selection.active), selection);
|
||||
assert_eq!(s.commits(), commits);
|
||||
assert_eq!(s.undo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>(), undo);
|
||||
assert_eq!(s.redo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>(), redo);
|
||||
assert!(bad_params, "expected batch preflight error, got {result:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn virtual_copy_batch_reserves_numbers_for_the_same_master() {
|
||||
let mut s = Session::with_demo();
|
||||
let visible = s.visible_cloned();
|
||||
let src = visible[0];
|
||||
let near_limit = lightcraft_catalog::PhotoId(
|
||||
s.execute("photo.virtualCopy", &json!({"ids": [src.0], "name": "Copy 4294967294"})).unwrap()["ids"][0].as_u64().unwrap(),
|
||||
);
|
||||
leave_redo_entry(&mut s, visible[1]);
|
||||
s.execute("library.select", &json!({"ids": [src.0, near_limit.0], "active": src.0})).unwrap();
|
||||
|
||||
let before = s.catalog.to_snapshot();
|
||||
let revision = s.catalog.revision;
|
||||
let selection = (s.selection.ids.clone(), s.selection.active);
|
||||
let commits = s.commits();
|
||||
let undo = s.undo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>();
|
||||
let redo = s.redo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>();
|
||||
|
||||
let result = s.execute("photo.virtualCopy", &json!({"ids": [src.0, near_limit.0]}));
|
||||
let bad_params = matches!(&result, Err(EngineError::BadParams { cmd, .. }) if cmd == "photo.virtualCopy");
|
||||
assert_eq!(s.catalog.to_snapshot(), before, "same-master reservations must be checked before either copy is added");
|
||||
assert_eq!(s.catalog.revision, revision);
|
||||
assert_eq!((s.selection.ids.clone(), s.selection.active), selection);
|
||||
assert_eq!(s.commits(), commits);
|
||||
assert_eq!(s.undo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>(), undo);
|
||||
assert_eq!(s.redo.iter().map(|entry| entry.label.clone()).collect::<Vec<_>>(), redo);
|
||||
assert!(bad_params, "a shared master has only one number left: {result:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn virtual_copy_last_valid_number_can_be_created_and_undone() {
|
||||
let mut s = Session::with_demo();
|
||||
let src = s.visible_cloned()[0];
|
||||
s.execute("photo.virtualCopy", &json!({"ids": [src.0], "name": "Copy 4294967294"})).unwrap();
|
||||
|
||||
let result = s.execute("photo.virtualCopy", &json!({"ids": [src.0]})).unwrap();
|
||||
let id = lightcraft_catalog::PhotoId(result["ids"][0].as_u64().unwrap());
|
||||
assert_eq!(s.catalog.photo(id).unwrap().copy_name.as_deref(), Some("Copy 4294967295"));
|
||||
assert_eq!(s.catalog.photo(id).unwrap().copy_of, Some(src));
|
||||
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert!(s.catalog.photo(id).is_none(), "one undo removes the last valid copy");
|
||||
}
|
||||
|
||||
/// 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]
|
||||
|
||||
+2
-1
@@ -1,6 +1,6 @@
|
||||
# Gaps: where LightCraft falls short of Lightroom
|
||||
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-23 · **Change:** minor (gap 28: ⌘L Enable Filters switches the filter off and on; gap 28: F8 shows / hides the right panel; import review album hierarchy, #2362; gap 20: Minolta MRW decodes; gap 10: 551 models verified; gap 15: Print Page Options — page numbers, page info, crop marks; gap 15: Print Page Options — page numbers, page info, crop marks; gap 28: `\` shows / hides the Filter Bar in the grids; gap 21: Indonesian interface shipped, every message but What's New; gap 27: the Windows x64 installer verified on Windows 11; the Settings → Apps uninstall confirmation remains)
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-24 · **Change:** patch (LR-LIB-VIRTUALCOPY: default number exhaustion is validated before copy creation; explicit names bypass suffix allocation; status and counts unchanged; gap 28: ⌘L Enable Filters switches the filter off and on; gap 28: F8 shows / hides the right panel; import review album hierarchy, #2362; gap 20: Minolta MRW decodes; gap 10: 551 models verified; gap 15: Print Page Options — page numbers, page info, crop marks; gap 15: Print Page Options — page numbers, page info, crop marks; gap 28: `\` shows / hides the Filter Bar in the grids; gap 21: Indonesian interface shipped, every message but What's New; gap 27: the Windows x64 installer verified on Windows 11; the Settings → Apps uninstall confirmation remains)
|
||||
|
||||
Every known shortfall, one entry each, ranked by user impact (how many Lightroom users hit it, and
|
||||
how hard it blocks them), then by effort. **Agents pick work from the top** (the first entry nobody is
|
||||
@@ -84,6 +84,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-24 | patch | LR-LIB-VIRTUALCOPY: default numbering exhaustion is an actionable validation error before batch mutation; explicit names bypass suffix allocation; ranked gaps and readiness counts unchanged |
|
||||
| 2026-10-22 | patch | XMP namespace-prefix collisions no longer overwrite foreign metadata; ranked gaps and estimates unchanged |
|
||||
| 2026-10-22 | patch | Issue #2519: PSD/PSB EXIF resource 1058 and XMP resource 1060 now use the common metadata extractor, and `.psb` files are accepted by import; no ranked gap or readiness count changes |
|
||||
| 2026-10-22 | minor | Gap 30 added: multi-select metadata editing — Info panel and All Metadata dialog show one photo's values with no mixed-value indication |
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Lightroom parity checklist
|
||||
|
||||
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-22 · **Change:** minor (MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
|
||||
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-24 · **Change:** patch (LR-LIB-VIRTUALCOPY: default numbering is preflighted across the requested batch and explicit names bypass suffix allocation; status and counts unchanged; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **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
|
||||
@@ -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`, `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” |
|
||||
| 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; default numbers are checked across the batch before copies are created, while explicit names bypass the counter; deleting a copy dissolves or renumbers the stack (#1493); no “Set Copy as Master” |
|
||||
|
||||
## C. Views & navigation (VIEW)
|
||||
|
||||
@@ -798,6 +798,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-24 | patch | LR-LIB-VIRTUALCOPY: copy names are resolved before batch mutation, numeric exhaustion returns BadParams, and explicit names bypass the counter; status and feature counts unchanged |
|
||||
| 2026-10-22 | patch | LR-META-XMP: namespace URI determines ownership; foreign namespaces using recognized prefixes survive saves and cannot supply native settings or ratings; status and counts unchanged |
|
||||
| 2026-10-22 | patch | LR-EXP-PSD, LR-META-EXIF-FULL: PSD/PSB imports and image resources 1058/1060 now supply EXIF and XMP to metadata readers and All Metadata; Original with Remove Location Info refuses detected PSD/PSB GPS; statuses and feature counts unchanged |
|
||||
| 2026-10-21 | patch | LR-IMP-ADD-DIALOG: reconcile #2362 with current main; deleted-album guard and hierarchical picker retained; counts unchanged |
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
# What's new in LightCraft
|
||||
|
||||
## 2026-10-24
|
||||
|
||||
### Virtual-copy numbering stops cleanly at its limit
|
||||
- Creating a numbered virtual copy now reports when the `Copy N` sequence is exhausted, before any copies in a
|
||||
batch are created. An explicit name remains available even when the numeric sequence is full.
|
||||
|
||||
## 2026-10-23
|
||||
|
||||
### Tab leaves the name field in New Album, New Smart Album and the name prompts
|
||||
|
||||
Reference in New Issue
Block a user