LR-IMP-ADD-DIALOG: check rename sequence exhaustion

This commit is contained in:
ronibalthazar
2026-10-25 17:28:56 +00:00
parent 03c88c166c
commit cff89260f9
7 changed files with 243 additions and 13 deletions
+6 -2
View File
@@ -89,6 +89,10 @@ pub fn import_params(s: &Session, p: &Value) -> Result<ImportRequest> {
}
None => Default::default(),
};
let rename_start = match p.get("renameStart").and_then(Value::as_u64) {
Some(n) => usize::try_from(n).map_err(|_| bad("library.import", "renameStart is too large for this platform"))?,
None => 1,
};
let opts = crate::import::ImportOptions {
on_deleted,
mode,
@@ -97,7 +101,7 @@ pub fn import_params(s: &Session, p: &Value) -> Result<ImportRequest> {
destination: str_param(p, "destination").map(str::to_string),
organize,
rename: str_param(p, "rename").map(str::to_string),
rename_start: p.get("renameStart").and_then(Value::as_u64).unwrap_or(1) as usize,
rename_start,
metadata_preset,
convert_dng: bool_or(p, "dng", false),
local: bool_or(p, "local", false),
@@ -1635,7 +1639,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Import Photos",
["File"],
Some("Cmd+Shift+I"),
"{paths: [file or folder (recursive)], mode?: add|copy|move (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/; move = as copy, then each original and its XMP sidecars are removed from the source — only after the copy is verified (a hard link on the same volume, else copied, synced and compared byte for byte) and its catalog record is saved; failed, duplicate and unchecked files keep their sources; a taken name gets -1, -2…; undo removes the photos from the library but leaves the files at the destination), destination?: folder for copies / moves, organize?: date (YYYY/YYYY-MM-DD) | month (YYYY/YYYY-MM) | flat | a folder template, e.g. `{date:%Y}/{date:%Y%m%d}` → 2026/20260114 (the template's `/` make the folders, each level expanded with the rename tokens and made a safe folder name: never outside the destination; must be relative, no `..`; a level with missing metadata is `unknown`) — dated by capture time, else the import time, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG; copy only), local?: bool (browsing: the photos stay out of the library, like library.browse; not with move), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..], onDeleted?: restore|skip|fresh (a file that is in Recently Deleted: restore (default) = bring the photo back with its edits — relinked to the file when its own is gone, skip = leave it there and say so, fresh = delete the trashed record and import the file as new)} → {imported, duplicates: [{path, existing, reason, existingDeleted?}], restored?: [photoId], failed, warnings?: [{path, code: cameraProfile|uprightGeometry|unmappedXmp, profile?, mode?: off|auto|level|vertical|full|guided|unknown, fields?}], moved?: [{from, to, sidecars?}], kept?: [{path, reason}] (move: sources left in place and why), album?}",
"{paths: [file or folder (recursive)], mode?: add|copy|move (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/; move = as copy, then each original and its XMP sidecars are removed from the source — only after the copy is verified (a hard link on the same volume, else copied, synced and compared byte for byte) and its catalog record is saved; failed, duplicate and unchecked files keep their sources; a taken name gets -1, -2…; undo removes the photos from the library but leaves the files at the destination), destination?: folder for copies / moves, organize?: date (YYYY/YYYY-MM-DD) | month (YYYY/YYYY-MM) | flat | a folder template, e.g. `{date:%Y}/{date:%Y%m%d}` → 2026/20260114 (the template's `/` make the folders, each level expanded with the rename tokens and made a safe folder name: never outside the destination; must be relative, no `..`; a level with missing metadata is `unknown`) — dated by capture time, else the import time, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1 (must fit this platform; later files are reported failed if the sequence runs out), metadataPreset?: name, dng?: bool (copy raws as DNG; copy only), local?: bool (browsing: the photos stay out of the library, like library.browse; not with move), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..], onDeleted?: restore|skip|fresh (a file that is in Recently Deleted: restore (default) = bring the photo back with its edits — relinked to the file when its own is gone, skip = leave it there and say so, fresh = delete the trashed record and import the file as new)} → {imported, duplicates: [{path, existing, reason, existingDeleted?}], restored?: [photoId], failed, warnings?: [{path, code: cameraProfile|uprightGeometry|unmappedXmp, profile?, mode?: off|auto|level|vertical|full|guided|unknown, fields?}], moved?: [{from, to, sidecars?}], kept?: [{path, reason}] (move: sources left in place and why), album?}",
always,
|s, p| {
let req = import_params(s, p)?;
+19 -7
View File
@@ -18,6 +18,8 @@
//! 7. Optionally apply a preset and add keywords (the import dialog's options).
//! 8. Commit all new photos as one undoable op. (Undoing a move removes the catalog records;
//! the files stay at the destination — undo never moves files back over a card.)
//! 9. Copy and Move rename sequences use checked platform-sized numbers: the last number is usable once, and any
//! further renamed files are reported as failed without losing earlier completed transfers.
//!
//! The import dialog first calls [`scan`] (`library.importPreview`): the same expansion, probing
//! and duplicate detection without adding anything, so the user can review the candidates. The
@@ -831,7 +833,8 @@ pub struct ImportJob {
cache: HashMap<String, ProbeInfo>,
naming: crate::sidecar::SidecarNaming,
now: String,
seq: usize,
/// Next rename number; `None` once the final representable value has been reserved.
seq: Option<usize>,
/// Files found so far (folders expanded) and handled so far, for progress.
pub total: std::sync::Arc<std::sync::atomic::AtomicUsize>,
pub done: std::sync::Arc<std::sync::atomic::AtomicUsize>,
@@ -964,7 +967,7 @@ impl ImportJob {
}
let fresh = opts.on_deleted == OnDeleted::Fresh;
Ok(ImportJob {
seq: opts.rename_start.max(1),
seq: Some(opts.rename_start.max(1)),
opts,
mode,
lib_dir,
@@ -1144,11 +1147,20 @@ impl ImportJob {
let mut q = Photo::new(PhotoId(0), Source::File { path: path.clone() }, &own, &info.format, 0, 0, &self.now);
q.captured = info.captured.clone();
q.meta = info.meta.clone();
let name = opts.rename.as_deref().filter(|t| !t.trim().is_empty()).map(|t| {
let n = crate::rename::expand(t, &q, self.seq);
self.seq += 1;
n
});
let name = if let Some(template) = opts.rename.as_deref().filter(|t| !t.trim().is_empty()) {
let Some(seq) = self.seq else {
self.done.fetch_add(1, Relaxed);
plan.slots.push(Slot::Item(PreparedItem::Failed(
path,
"rename sequence exhausted; choose a lower renameStart or leave rename blank".into(),
)));
continue;
};
self.seq = seq.checked_add(1);
Some(crate::rename::expand(template, &q, seq))
} else {
None
};
let folders = opts.organize.folders(&q);
let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f));
// the name this file gets depends on what a pending transfer of a like-named file
+204
View File
@@ -461,6 +461,210 @@ fn copy_with_destination_organize_rename_and_metadata_preset() {
}
}
#[test]
fn import_rename_sequence_uses_the_last_value_once() {
let base = temp_dir("rename-sequence-last");
let (src, dest, lib) = (base.join("source"), base.join("destination"), base.join("library"));
write_png(&src.join("one.png"), 1);
let mut s = Session::new().with_fs();
s.open_library(&lib, false).unwrap();
let last = usize::MAX;
let r = s
.execute(
"library.import",
&json!({
"paths": [src.join("one.png").to_string_lossy()],
"mode": "copy", "destination": dest.to_string_lossy(), "organize": "flat",
"rename": "Shot-{seq}", "renameStart": last as u64
}),
)
.unwrap();
assert_eq!(ids(&r, "imported"), 1, "{r}");
assert_eq!(r["failed"].as_array().map(Vec::len), Some(0), "{r}");
assert!(dest.join(format!("Shot-{last}.png")).is_file());
assert_eq!(s.catalog.len(), 1);
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(s.catalog.len(), 0, "the import remains one undo step");
assert!(dest.join(format!("Shot-{last}.png")).is_file(), "undo leaves a successful copy on disk");
let _ = std::fs::remove_dir_all(base);
}
#[test]
fn import_rename_sequence_reports_exhaustion_and_keeps_prior_successes() {
let base = temp_dir("rename-sequence-exhausted");
let (src, dest, lib) = (base.join("source"), base.join("destination"), base.join("library"));
let paths: Vec<_> = (1..=3)
.map(|i| {
let path = src.join(format!("{i:02}.png"));
write_png(&path, i as u8);
path
})
.collect();
let mut s = Session::new().with_fs();
s.open_library(&lib, false).unwrap();
let start = usize::MAX - 1;
let r = s
.execute(
"library.import",
&json!({
"paths": paths.iter().map(|p| p.to_string_lossy()).collect::<Vec<_>>(),
"mode": "copy", "destination": dest.to_string_lossy(), "organize": "flat",
"rename": "Shot-{seq}", "renameStart": start as u64
}),
)
.unwrap();
assert_eq!(ids(&r, "imported"), 2, "the final two representable values are used: {r}");
assert_eq!(r["failed"].as_array().map(Vec::len), Some(1), "{r}");
let failure = r["failed"][0][1].as_str().unwrap_or_default();
assert!(failure.contains("sequence") && failure.contains("renameStart"), "actionable failure: {failure}");
assert_eq!(s.catalog.len(), 2);
assert!(dest.join(format!("Shot-{start}.png")).is_file());
assert!(dest.join(format!("Shot-{}.png", usize::MAX)).is_file());
assert_eq!(std::fs::read_dir(&dest).unwrap().count(), 2, "the exhausted file gets no destination entry");
assert!(!paths[2].file_name().is_some_and(|name| dest.join(name).exists()), "the exhausted source gets no copy");
let _ = std::fs::remove_dir_all(base);
}
#[test]
fn import_rename_sequence_keeps_flushed_copy_and_move_results() {
for mode in ["copy", "move"] {
let base = temp_dir(&format!("rename-sequence-flush-{mode}"));
let (src, dest, lib) = (base.join("source"), base.join("destination"), base.join("library"));
let paths: Vec<_> = (1..=4)
.map(|i| {
let path = src.join(format!("{i:02}.png"));
write_png(&path, i as u8);
path
})
.collect();
let mut s = Session::new().with_fs();
s.open_library(&lib, false).unwrap();
let start = usize::MAX - 2;
let r = s
.execute(
"library.import",
&json!({
"paths": paths.iter().map(|p| p.to_string_lossy()).collect::<Vec<_>>(),
"mode": mode, "destination": dest.to_string_lossy(), "organize": "flat",
"rename": "Holiday", "renameStart": start as u64
}),
)
.unwrap();
assert_eq!(ids(&r, "imported"), 3, "{mode}: earlier transfers survive sequence exhaustion: {r}");
assert_eq!(r["failed"].as_array().map(Vec::len), Some(1), "{mode}: {r}");
assert_eq!(s.catalog.len(), 3, "{mode}: all successful transfers are catalogued");
let mut output_names: Vec<_> = std::fs::read_dir(&dest)
.unwrap()
.filter_map(std::result::Result::ok)
.map(|entry| entry.file_name().to_string_lossy().into_owned())
.collect();
output_names.sort();
assert_eq!(output_names, ["Holiday-1.png", "Holiday-2.png", "Holiday.png"], "{mode}");
match mode {
"copy" => assert!(paths.iter().all(|path| path.is_file()), "copy leaves every source in place"),
"move" => {
assert!(paths.iter().take(3).all(|path| !path.exists()), "successful moves remove sources after cataloguing");
assert!(paths[3].is_file(), "the failed source remains in place");
assert_eq!(r["moved"].as_array().map(Vec::len), Some(3), "{r}");
}
_ => unreachable!(),
}
let _ = std::fs::remove_dir_all(base);
}
}
#[test]
fn import_rename_sequence_exhaustion_persists_across_batches() {
let base = temp_dir("rename-sequence-batches");
let (src, dest, lib) = (base.join("source"), base.join("destination"), base.join("library"));
let first = src.join("one.png");
let second = src.join("two.png");
write_png(&first, 1);
write_png(&second, 2);
let mut s = Session::new().with_fs();
s.open_library(&lib, false).unwrap();
let opts = crate::import::ImportOptions {
mode: crate::import::ImportMode::Copy,
destination: Some(dest.to_string_lossy().into_owned()),
organize: crate::import::Organize::Flat,
rename: Some("Shot-{seq}".into()),
rename_start: usize::MAX,
..Default::default()
};
let mut job = crate::import::ImportJob::new(&mut s, opts).unwrap();
let cancel = std::sync::atomic::AtomicBool::new(false);
let prepared = job.prepare(std::slice::from_ref(&first.to_string_lossy().to_string()), &cancel);
let first_result = crate::import::commit_prepared(&mut s, &job.opts, job.now(), prepared).unwrap();
assert_eq!(ids(&serde_json::to_value(first_result).unwrap(), "imported"), 1);
let prepared = job.prepare(std::slice::from_ref(&second.to_string_lossy().to_string()), &cancel);
let second_result = crate::import::commit_prepared(&mut s, &job.opts, job.now(), prepared).unwrap();
let second_result = serde_json::to_value(second_result).unwrap();
assert_eq!(ids(&second_result, "imported"), 0, "the exhausted job does not restart its sequence");
assert_eq!(second_result["failed"].as_array().map(Vec::len), Some(1), "{second_result}");
assert_eq!(s.catalog.len(), 1);
assert!(dest.join(format!("Shot-{}.png", usize::MAX)).is_file());
assert!(!dest.join("Shot-1.png").exists(), "no wraparound or reuse");
assert_eq!(
(job.done.load(std::sync::atomic::Ordering::Relaxed), job.total.load(std::sync::atomic::Ordering::Relaxed)),
(2, 2),
"both the copied and exhausted files advance import progress"
);
let _ = std::fs::remove_dir_all(base);
}
#[test]
fn import_rename_sequence_controls_ignore_the_counter_when_not_renaming() {
let base = temp_dir("rename-sequence-controls");
let (src, dest, lib) = (base.join("source"), base.join("destination"), base.join("library"));
let added = src.join("add.png");
let copied = src.join("copy.png");
write_png(&added, 1);
write_png(&copied, 2);
let mut s = Session::new().with_fs();
s.open_library(&lib, false).unwrap();
let added_report = s
.execute(
"library.import",
&json!({
"paths": [added.to_string_lossy()], "mode": "add", "rename": "Ignored-{seq}", "renameStart": usize::MAX as u64
}),
)
.unwrap();
let copied_report = s
.execute(
"library.import",
&json!({
"paths": [copied.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": "flat",
"renameStart": usize::MAX as u64
}),
)
.unwrap();
assert_eq!(ids(&added_report, "imported"), 1, "Add does not use rename numbering: {added_report}");
assert_eq!(ids(&copied_report, "imported"), 1, "an empty rename template does not use the counter: {copied_report}");
assert!(dest.join("copy.png").is_file());
assert_eq!(s.catalog.len(), 2);
let _ = std::fs::remove_dir_all(base);
}
#[cfg(target_pointer_width = "32")]
#[test]
fn import_rename_sequence_rejects_a_start_outside_usize() {
let s = Session::new();
let too_large = u64::from(u32::MAX) + 1;
let parsed = crate::cmd::library::import_params(&s, &json!({"paths": ["unused.png"], "renameStart": too_large}));
assert!(parsed.is_err(), "32-bit import parameters must not truncate renameStart");
}
/// Copy with a custom folder template: `{date:%Y}/{date:%Y%m%d}` files a photo under
/// `2026/20260114/` (an undated file by the import time); templates that would leave the
/// destination are refused.
+2 -1
View File
@@ -1,6 +1,6 @@
# Gaps: where LightCraft falls short of Lightroom
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-24 · **Change:** minor (gap 30: the Info panel shows the selected photos' shared metadata and marks mixed fields; gap 28: ⌘⌥⇧C / ⌘⌥⇧V copy and paste metadata; gap 13: before a Lightroom catalog import the app asks merge or new library, #2672; 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 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; gap 28: ⌘L Enable Filters switches the filter off and on; gap 15: Japanese photo and postcard paper sizes in the Print page setup and PDF)
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-25 · **Change:** minor (import: checked rename-sequence exhaustion; gap 30: the Info panel shows the selected photos' shared metadata and marks mixed fields; gap 28: ⌘⌥⇧C / ⌘⌥⇧V copy and paste metadata; gap 13: before a Lightroom catalog import the app asks merge or new library, #2672; 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 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; gap 28: ⌘L Enable Filters switches the filter off and on; gap 15: Japanese photo and postcard paper sizes in the Print page setup and PDF)
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-25 | patch | Import Copy and Move report rename-sequence exhaustion per file without losing earlier transfers; no ranked gap or readiness changes |
| 2026-10-24 | minor | Gap 28: ⌘⌥⇧C / ⌘⌥⇧V (Ctrl+Alt+Shift+C / V on Windows and Linux) copy and paste metadata; no key leaves the entry; hours unchanged |
| 2026-10-24 | minor | Gap 30 shrinks: the Info panel's Show Metadata For (Target Photo / Selected Photos, `view.infoSelectedPhotos`) shows the selected photos' shared values and marks fields that differ as Mixed; the All Metadata dialog remains; 1–3 h (was 4–8) |
| 2026-10-23 | minor | Gap 13: before a Lightroom catalog import the app asks whether it merges into the open library or goes into a new one (opt-out in Settings, #2672); hours unchanged |
File diff suppressed because one or more lines are too long
+1
View File
@@ -26,6 +26,7 @@ The import runs in the background, with a progress row in the activity stack at
- Importing a folder scans it for photos. It does not add the folder to **Local**. To work in a folder without importing, use **Local** > **Browse Folder…**.
- Folder names and `{date}` use the capture time. A photo without one uses today's date.
- Copy and Move rename sequences stop at the platform's last representable number. If a sequence runs out, later files are reported as failed; completed transfers remain in the library.
- Raw formats LightCraft reads include DNG, Canon CR2 and CR3, Sony ARW, Nikon NEF, Fujifilm RAF, Panasonic RW2, Leica RWL, Pentax PEF and Olympus ORF. Some variants open as **Preview only**. See [A raw photo opens as "Preview only"](../troubleshooting/preview-only-raw.md).
## Next
+7
View File
@@ -1,5 +1,12 @@
# What's new in LightCraft
## 2026-10-25
### Import rename numbering stops safely at its limit
- Copy and Move imports can use the last representable rename number once. If more renamed files remain, each is
reported as failed with guidance to lower `renameStart` or leave `rename` blank; completed transfers stay
accounted for and catalogued. A `renameStart` value outside this platform's range is rejected before setup.
## 2026-10-24
### Relative-date filters and smart albums follow the clock