diff --git a/crates/engine/src/cmd/missing.rs b/crates/engine/src/cmd/missing.rs index 05bea65f..bf35f22e 100644 --- a/crates/engine/src/cmd/missing.rs +++ b/crates/engine/src/cmd/missing.rs @@ -1,8 +1,9 @@ //! Missing originals: photos whose file is no longer where the library expects it (moved, //! renamed, on an unplugged drive), and relinking them — one at a time (`photo.relink`) or by -//! searching a folder for files with the same name and size (`library.findMissing`). +//! searching a folder for files with the same name and size, or the same content +//! (`library.findMissing`). -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::Path; use lightcraft_catalog::{Catalog, Op, Photo, PhotoId, Source}; @@ -83,8 +84,29 @@ fn relink(s: &mut Session, p: &Value) -> Result { Ok(json!({"id": id.0, "path": abs})) } -/// Search `folder` (recursively) for each missing photo's file name — and its size when the -/// library knows it — and relink every match in one undoable step. +/// The stored content hash of a photo, without the `:dupN` suffix duplicates get. +fn stored_hash(p: &Photo) -> Option<&str> { + p.content_hash.as_deref().map(|h| h.split(':').next().unwrap_or(h)).filter(|h| !h.is_empty()) +} + +/// The content hash of the file at `path` (as import computes it), if it can be read. +fn file_hash(path: &str) -> Option { + std::fs::read(path).ok().map(|b| lightcraft_preview::hash_bytes(&b).to_string()) +} + +/// Search `folder` (recursively) for each missing photo's file and relink every match in one +/// undoable step. +/// +/// Matching, per photo: +/// 1. a file with the same name (any letter case), the same size and the same content hash; +/// 2. renamed: a file of the same size with the same content hash (only same-size files are +/// read, so this stays cheap); +/// 3. without a hash to confirm (or the file was edited in place): the one file with the same +/// name and size. Several such files are ambiguous: the photo is skipped and reported in +/// `ambiguous`, never guessed (two cameras' `IMG_0001.CR3` of one size are different photos). +/// +/// Files that are already some photo's original, or that another photo was just matched with, +/// are not used. fn find_missing(s: &mut Session, p: &Value) -> Result { const C: &str = "library.findMissing"; let folder = str_param(p, "folder").ok_or_else(|| bad(C, "missing `folder`"))?; @@ -93,27 +115,72 @@ fn find_missing(s: &mut Session, p: &Value) -> Result { } let lost = missing(s); if lost.is_empty() { - return Ok(json!({"found": [], "missing": 0})); + return Ok(json!({"found": [], "missing": 0, "ambiguous": []})); } - // file name (lower case) → candidate paths + let in_use: HashSet = s.catalog.photos().filter_map(|p| checked_path(p).map(str::to_string)).collect(); + // file name (lower case) → candidate paths; size → paths (filled lazily) let mut by_name: HashMap> = HashMap::new(); - for f in crate::import::expand(&[folder.to_string()], None) { - if let Some(n) = Path::new(&f).file_name() { - by_name.entry(n.to_string_lossy().to_lowercase()).or_default().push(f); + let files: Vec = crate::import::expand(&[folder.to_string()], None).into_iter().filter(|f| !in_use.contains(f)).collect(); + for f in &files { + if let Some(n) = Path::new(f).file_name() { + by_name.entry(n.to_string_lossy().to_lowercase()).or_default().push(f.clone()); } } + let mut by_size: Option>> = None; + let mut hashes: HashMap> = HashMap::new(); + let mut hash_of = |f: &str| hashes.entry(f.to_string()).or_insert_with(|| file_hash(f)).clone(); + let size_of = |f: &str| std::fs::metadata(f).map(|m| m.len()).ok(); + let mut used: HashSet = HashSet::new(); let mut ops = Vec::new(); let mut found = Vec::new(); + let mut ambiguous = Vec::new(); for (id, old) in &lost { + let Some(ph) = s.catalog.photo(*id) else { continue }; let name = Path::new(old).file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default(); - let size = s.catalog.photo(*id).map(|p| p.file_size).unwrap_or(0); - let hit = by_name.get(&name).and_then(|c| { - c.iter().find(|f| size == 0 || std::fs::metadata(f).is_ok_and(|m| m.len() == size)).or_else(|| (c.len() == 1 && size == 0).then(|| &c[0])) - }); - if let Some(path) = hit { - ops.push(relink_op(*id, path)); - found.push(json!({"id": id.0, "from": old, "to": path})); + let size = ph.file_size; + let hash = stored_hash(ph).map(str::to_string); + let named: Vec<&String> = by_name + .get(&name) + .map(|c| c.iter().filter(|f| !used.contains(*f) && (size == 0 || size_of(f) == Some(size))).collect()) + .unwrap_or_default(); + // 1. same name and size, confirmed by the content hash + let mut hit: Option<(String, &str)> = + hash.as_deref().and_then(|h| named.iter().find(|f| hash_of(f).as_deref() == Some(h))).map(|f| (f.to_string(), "name")); + // 2. renamed: same size and content + if hit.is_none() + && let Some(h) = &hash + && size > 0 + { + let sizes = by_size.get_or_insert_with(|| { + let mut m: HashMap> = HashMap::new(); + for f in &files { + if let Some(n) = size_of(f) { + m.entry(n).or_default().push(f.clone()); + } + } + m + }); + hit = sizes + .get(&size) + .and_then(|c| c.iter().find(|f| !used.contains(*f) && hash_of(f).as_deref() == Some(h.as_str()))) + .map(|f| (f.clone(), "content")); } + // 3. same name and size only (no hash, or the file was edited in place): one candidate + // is taken, several are ambiguous and skipped rather than guessed + if hit.is_none() { + match named.len() { + 1 => hit = Some((named[0].clone(), "name")), + 0 => {} + _ => { + ambiguous.push(json!({"id": id.0, "from": old, "candidates": named})); + continue; + } + } + } + let Some((path, by)) = hit else { continue }; + ops.push(relink_op(*id, &path)); + found.push(json!({"id": id.0, "from": old, "to": path, "by": by})); + used.insert(path); } let still = lost.len() - found.len(); if !ops.is_empty() { @@ -124,7 +191,7 @@ fn find_missing(s: &mut Session, p: &Value) -> Result { } } } - Ok(json!({"found": found, "missing": still})) + Ok(json!({"found": found, "missing": still, "ambiguous": ambiguous})) } pub fn specs() -> Vec { @@ -146,7 +213,7 @@ pub fn specs() -> Vec { "Find Missing Photos", [], None, - "{folder} — relink every missing photo whose file (same name and size) is somewhere in the folder → {found: [{id, from, to}], missing}", + "{folder} — relink every missing photo whose file is somewhere in the folder: same name and size (and content, when the library knows its hash), or — renamed — same size and content → {found: [{id, from, to, by: name|content}], missing, ambiguous: [{id, from, candidates}] (several same-name candidates and no hash to tell them apart: skipped)}", always, find_missing ), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index b02e3d54..b86d5880 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -379,15 +379,22 @@ impl Session { Ok(e.label) } - /// Apply an undo/redo op, first moving the files its renames imply (all or nothing). + /// Apply an undo/redo op, first moving the files its renames imply (all or nothing). Files + /// that can't be moved back after a failure are reported and the library follows them (a + /// logged change outside the undo history), so none goes missing. fn apply_with_files(&mut self, op: &Op) -> Result { + let fs = rename::RealFs; let moves = self.file_moves(op); - Session::move_files(&moves)?; + if let Err(e) = rename::move_all(&fs, &moves) { + return Err(EngineError::Other(format!("can't move the files back: {}", self.follow_stuck(op, e, false)))); + } match self.catalog.apply(op.clone()) { Ok(inv) => Ok(inv), Err(e) => { let back: Vec<(String, String)> = moves.iter().rev().map(|(a, b)| (b.clone(), a.clone())).collect(); - let _ = Session::move_files(&back); + if let Err(be) = rename::move_all(&fs, &back) { + return Err(EngineError::Other(format!("{e}; the files could not all be moved back: {}", be.message))); + } Err(e.into()) } } diff --git a/crates/engine/src/rename.rs b/crates/engine/src/rename.rs index 266f76e0..6e363ed3 100644 --- a/crates/engine/src/rename.rs +++ b/crates/engine/src/rename.rs @@ -386,31 +386,48 @@ fn move_one(fs: &dyn MoveFs, a: &Path, b: &Path) -> std::io::Result<()> { .find(|t| !fs.exists(t)) .ok_or_else(|| std::io::Error::other("no free temporary name"))?; fs.rename_no_replace(a, &tmp)?; - fs.rename_no_replace(&tmp, b).inspect_err(|_| { - let _ = fs.rename_no_replace(&tmp, a); + fs.rename_no_replace(&tmp, b).map_err(|e| match fs.rename_no_replace(&tmp, a) { + Ok(()) => e, + Err(e2) => std::io::Error::other(format!("{e}; it could not be given its old name back either ({e2}): it is now {}", tmp.display())), }) } -/// Move `from` to `to` (never over an existing file) together with its sidecars. On error nothing -/// is left moved. -pub fn move_file(from: &str, to: &str) -> std::result::Result<(), String> { +/// Why [`move_file`] failed, and what it left behind. +#[derive(Clone, Debug, PartialEq)] +pub struct MoveError { + /// What went wrong, including every step that could not be undone and where that file is now. + pub message: String, + /// The file itself ended up at the new path: it moved, a sidecar didn't, and moving the file + /// back failed too. Its sidecars that did move stay with it. + pub moved: bool, +} + +impl std::fmt::Display for MoveError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +/// Move `from` to `to` (never over an existing file) together with its sidecars. On error the +/// moves are undone; whatever can't be undone is named in the error (see [`MoveError`]). +pub fn move_file(from: &str, to: &str) -> std::result::Result<(), MoveError> { move_file_with(&RealFs, from, to) } /// [`move_file`] on the file system `fs`. -pub(crate) fn move_file_with(fs: &dyn MoveFs, from: &str, to: &str) -> std::result::Result<(), String> { +pub(crate) fn move_file_with(fs: &dyn MoveFs, from: &str, to: &str) -> std::result::Result<(), MoveError> { if from == to { return Ok(()); } + let fail = |message: String| MoveError { message, moved: false }; let (f, t) = (Path::new(from), Path::new(to)); // `to` may be this very file spelled differently (case-insensitive volume); any other file // there — e.g. `img.jpg` next to `IMG.JPG` on a case-sensitive volume — is never replaced if fs.exists(t) && !is_respelling(fs, f, t) { - return Err(format!("{to} already exists")); + return Err(fail(format!("{to} already exists"))); } - let mut done: Vec<(PathBuf, PathBuf)> = Vec::new(); - move_one(fs, f, t).map_err(|e| format!("rename {from}: {e}"))?; - done.push((f.to_path_buf(), t.to_path_buf())); + move_one(fs, f, t).map_err(|e| fail(format!("rename {from}: {e}")))?; + let mut done: Vec<(PathBuf, PathBuf)> = Vec::new(); // sidecars moved for (sc, naming, upper) in sidecars(from) { if !fs.is_file(&sc) { continue; @@ -423,16 +440,73 @@ pub(crate) fn move_file_with(fs: &dyn MoveFs, from: &str, to: &str) -> std::resu continue; // never overwrite: the old sidecar stays where it was } if let Err(e) = move_one(fs, &sc, &dst) { - for (a, b) in done.iter().rev() { - let _ = move_one(fs, b, a); + let mut message = format!("rename {}: {e}", sc.display()); + // the file first: if it can't go back, the sidecars that moved stay with it + if let Err(e) = move_one(fs, t, f) { + message.push_str(&format!("; {from} could not be moved back ({e}): it is now {to}")); + if !done.is_empty() { + let names: Vec = done.iter().map(|(_, b)| b.display().to_string()).collect(); + message.push_str(&format!(" with its sidecar {}", names.join(", "))); + } + return Err(MoveError { message, moved: true }); } - return Err(format!("rename {}: {e}", sc.display())); + for (a, b) in done.iter().rev() { + if let Err(e) = move_one(fs, b, a) { + message.push_str(&format!("; sidecar {} could not be moved back ({e}): it is now {}", a.display(), b.display())); + } + } + return Err(fail(message)); } done.push((sc, dst)); } Ok(()) } +/// What a failed batch of moves ([`move_all`]) left behind. +#[derive(Debug)] +pub(crate) struct BatchMoveError { + /// The first failure and every move that could not be undone. + pub message: String, + /// Moves that stayed done, (from, to): those files are at `to` now. + pub stuck: Vec<(String, String)>, +} + +/// Move the files `moves` names, all or nothing: after a failure the earlier moves are undone. +/// Moves that can't be undone (e.g. the network share went away) are reported, not ignored. +pub(crate) fn move_all(fs: &dyn MoveFs, moves: &[(String, String)]) -> std::result::Result<(), BatchMoveError> { + let mut done: Vec<&(String, String)> = Vec::new(); + for m in moves { + let Err(e) = move_file_with(fs, &m.0, &m.1) else { + done.push(m); + continue; + }; + let mut message = e.message; + let mut stuck = Vec::new(); + if e.moved { + stuck.push(m.clone()); + } + for d in done.iter().rev() { + if let Err(e) = move_file_with(fs, &d.1, &d.0) { + message.push_str(&format!("; {} could not be moved back to {}: {}", d.1, d.0, e.message)); + if !e.moved { + stuck.push((*d).clone()); + } + } + } + return Err(BatchMoveError { message, stuck }); + } + Ok(()) +} + +/// The `SetFile` ops in `op` (nested batches included) that point a photo at one of `to`. +fn set_file_ops_to(op: &Op, to: &HashSet<&str>, out: &mut Vec) { + match op { + Op::Batch { ops } => ops.iter().for_each(|o| set_file_ops_to(o, to, out)), + Op::SetFile { source: Source::File { path }, .. } if to.contains(path.as_str()) => out.push(op.clone()), + _ => {} + } +} + impl Session { /// Plan renaming `ids` with `template` (sequence numbers from `start`), resolving collisions. pub fn plan_rename(&self, ids: &[PhotoId], template: &str, start: usize) -> Vec { @@ -509,21 +583,16 @@ impl Session { /// Carry out `plans`: move the files (rolled back on failure), then commit one undoable op /// that updates the photos (and their virtual copies). + /// + /// When a failure can't be fully rolled back, the files that kept their new names are + /// committed as a smaller undoable rename, so the library points at them (nothing goes + /// missing), and the error lists them (old → new). pub fn apply_rename(&mut self, plans: &[RenamePlan]) -> Result { - let mut moved: Vec<(String, String)> = Vec::new(); - for pl in plans { - if let (Some(a), Some(b)) = (&pl.from_path, &pl.to_path) - && a != b - { - if let Err(e) = move_file(a, b) { - for (a, b) in moved.iter().rev() { - let _ = move_file(b, a); - } - return Err(EngineError::Other(e)); - } - moved.push((a.clone(), b.clone())); - } - } + self.apply_rename_with(&RealFs, plans) + } + + /// [`Session::apply_rename`] on the file system `fs`. + pub(crate) fn apply_rename_with(&mut self, fs: &dyn MoveFs, plans: &[RenamePlan]) -> Result { let mut ops = Vec::new(); let by_path: HashMap<&str, &RenamePlan> = plans.iter().filter_map(|p| p.from_path.as_deref().map(|f| (f, p))).collect(); for p in self.catalog.photos() { @@ -541,17 +610,66 @@ impl Session { } } let n = ops.len(); + let op = Op::Batch { ops }; + let moves: Vec<(String, String)> = plans + .iter() + .filter_map(|pl| match (&pl.from_path, &pl.to_path) { + (Some(a), Some(b)) if a != b => Some((a.clone(), b.clone())), + _ => None, + }) + .collect(); + if let Err(e) = move_all(fs, &moves) { + return Err(EngineError::Other(self.follow_stuck(&op, e, true))); + } if n > 0 - && let Err(e) = self.commit(&format!("Rename {n} Photo{}", if n == 1 { "" } else { "s" }), Op::Batch { ops }) + && let Err(err) = self.commit(&format!("Rename {n} Photo{}", if n == 1 { "" } else { "s" }), op) { - for (a, b) in moved.iter().rev() { - let _ = move_file(b, a); + let back: Vec<(String, String)> = moves.iter().rev().map(|(a, b)| (b.clone(), a.clone())).collect(); + if let Err(e) = move_all(fs, &back) { + return Err(EngineError::Other(format!("{err}; the renamed files could not all be moved back: {}", e.message))); } - return Err(e); + return Err(err); } Ok(n) } + /// After a failed batch of moves for `op`: point the photos whose files stayed moved at + /// their new paths (an undoable "partly" rename when `undoable`, otherwise a plain logged + /// change), so the library matches the disk. Returns the message for the user. + pub(crate) fn follow_stuck(&mut self, op: &Op, e: BatchMoveError, undoable: bool) -> String { + let mut msg = e.message; + if e.stuck.is_empty() { + return msg; + } + let to: HashSet<&str> = e.stuck.iter().map(|m| m.1.as_str()).collect(); + let mut keep = Vec::new(); + set_file_ops_to(op, &to, &mut keep); + let list: Vec = e.stuck.iter().map(|(a, b)| format!("{a} → {b}")).collect(); + let n = e.stuck.len(); + let s = if n == 1 { "" } else { "s" }; + let fix = Op::Batch { ops: keep }; + let applied = if undoable { + self.commit(&format!("Rename {n} Photo{s} (partly)"), fix) + } else { + match self.catalog.apply(fix.clone()) { + Ok(_) => { + self.pending_log.push(fix); + Ok(()) + } + Err(e) => Err(e.into()), + } + }; + match applied { + Ok(()) => msg + .push_str(&format!(". {n} file{s} could not be moved back and kept the new name; the library now points there: {}", list.join(", "))), + Err(err) => msg.push_str(&format!( + ". {n} file{s} could not be moved back and the library could not follow ({err}); relink with Find Missing Photos: {}", + list.join(", ") + )), + } + msg + } + /// File moves an undo/redo op implies: `SetFile` ops whose source path differs from the photo's /// current one. pub(crate) fn file_moves(&self, op: &Op) -> Vec<(String, String)> { @@ -573,21 +691,6 @@ impl Session { rec(self, op, &mut v); v } - - /// Move the files `moves` names; all or nothing. - pub(crate) fn move_files(moves: &[(String, String)]) -> Result<()> { - let mut done: Vec<&(String, String)> = Vec::new(); - for m in moves { - if let Err(e) = move_file(&m.0, &m.1) { - for (a, b) in done.iter().rev().map(|m| (&m.0, &m.1)) { - let _ = move_file(b, a); - } - return Err(EngineError::Other(format!("can't undo the rename: {e}"))); - } - done.push(m); - } - Ok(()) - } } #[cfg(test)] @@ -657,7 +760,7 @@ mod tests { fn case_only_renames_never_replace_another_file() { // case-sensitive: IMG_1.JPG and img_1.JPG are two photos let fs = FakeFs::new(false, &[("/p/IMG_1.JPG", "first"), ("/p/img_1.JPG", "second"), ("/p/IMG_1.xmp", "sc1"), ("/p/img_1.xmp", "sc2")]); - assert!(move_file_with(&fs, "/p/IMG_1.JPG", "/p/img_1.JPG").unwrap_err().contains("already exists")); + assert!(move_file_with(&fs, "/p/IMG_1.JPG", "/p/img_1.JPG").unwrap_err().message.contains("already exists")); assert_eq!(fs.listing().len(), 4); assert!(fs.listing().contains(&("/p/img_1.JPG".into(), "second".into()))); // the planner picks a free name instead @@ -693,6 +796,59 @@ mod tests { assert!(move_file_with(&fs, "/p/A.JPG", "/p/B.JPG").is_err()); } + /// Issue #105: a rollback that fails is reported, and the library follows the files that kept + /// their new names (an undoable partial rename), so nothing goes missing. + #[test] + fn failed_rollbacks_are_reported_and_followed() { + let fs = FakeFs::new(false, &[("/p/a.jpg", "A"), ("/p/a.xmp", "As"), ("/p/b.jpg", "B"), ("/p/c.jpg", "C")]); + let mut s = Session::new(); + for (i, f) in ["/p/a.jpg", "/p/b.jpg", "/p/c.jpg"].iter().enumerate() { + s.catalog.apply(Op::AddPhoto { photo: Box::new(file_photo(i as u64 + 1, f)) }).unwrap(); + } + let ids = [PhotoId(1), PhotoId(2), PhotoId(3)]; + let plans = s.plan_rename_with(&fs, &ids, "Trip-{seq}", 1); + // c can't be renamed (the share went away), and neither can Trip-1 be moved back + fs.fail.borrow_mut().extend(["c.jpg".to_string(), "Trip-1".to_string()]); + let err = s.apply_rename_with(&fs, &plans).unwrap_err().to_string(); + assert!(err.contains("rename /p/c.jpg"), "{err}"); + assert!(err.contains("/p/Trip-1.jpg could not be moved back"), "{err}"); + assert!(err.contains("/p/a.jpg → /p/Trip-1.jpg"), "lists what stayed renamed: {err}"); + assert_eq!( + fs.listing(), + vec![ + ("/p/Trip-1.jpg".into(), "A".into()), + ("/p/Trip-1.xmp".into(), "As".into()), + ("/p/b.jpg".into(), "B".into()), + ("/p/c.jpg".into(), "C".into()) + ] + ); + // the catalog matches the disk + let path = |s: &Session, id: u64| match &s.catalog.photo(PhotoId(id)).unwrap().source { + Source::File { path } => path.clone(), + Source::Demo { .. } => String::new(), + }; + assert_eq!(path(&s, 1), "/p/Trip-1.jpg"); + assert_eq!(s.catalog.photo(PhotoId(1)).unwrap().file_name, "Trip-1.jpg"); + assert_eq!((path(&s, 2), path(&s, 3)), ("/p/b.jpg".into(), "/p/c.jpg".into())); + assert_eq!(s.undo.last().map(|e| e.label.as_str()), Some("Rename 1 Photo (partly)")); + + // a sidecar that won't move, and a file that won't go back: the file keeps its new name + // with the sidecars that did move, and says so + let fs = FakeFs::new(false, &[("/p/d.jpg", "D"), ("/p/d.xmp", "Ds"), ("/p/d.jpg.xmp", "Df")]); + fs.fail.borrow_mut().extend(["d.jpg.xmp".to_string(), "/p/e.jpg".to_string()]); + let e = move_file_with(&fs, "/p/d.jpg", "/p/e.jpg").unwrap_err(); + assert!(e.moved, "{e}"); + assert!(e.message.contains("it is now /p/e.jpg with its sidecar /p/e.xmp"), "{e}"); + assert_eq!(fs.listing(), vec![("/p/d.jpg.xmp".into(), "Df".into()), ("/p/e.jpg".into(), "D".into()), ("/p/e.xmp".into(), "Ds".into())]); + // … and when the file does go back, its sidecars follow it + let fs = FakeFs::new(false, &[("/p/d.jpg", "D"), ("/p/d.xmp", "Ds"), ("/p/d.jpg.xmp", "Df")]); + fs.fail.borrow_mut().push("d.jpg.xmp".to_string()); + let e = move_file_with(&fs, "/p/d.jpg", "/p/e.jpg").unwrap_err(); + assert!(!e.moved); + assert_eq!(fs.listing().len(), 3); + assert!(fs.listing().iter().all(|(p, _)| p.starts_with("/p/d."))); + } + /// Issue #95 on the real disk, whichever kind of volume the temp folder is on. #[test] fn case_only_rename_on_disk() { diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index cd5b0e92..a719ca5f 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -144,6 +144,45 @@ fn browsing_a_folder_lists_its_photos_without_adding_them() { let _ = std::fs::remove_dir_all(&dir); } +/// Issue #105: Find Missing finds renamed files by content, prefers the content match over a +/// same-name same-size impostor, and skips (reports) photos it can't tell apart. +#[test] +fn find_missing_matches_renamed_files_by_content() { + let dir = temp_dir("missing-content"); + std::fs::create_dir_all(dir.join("old")).unwrap(); + std::fs::create_dir_all(dir.join("moved/real")).unwrap(); + for (n, seed) in [("a.png", 1), ("b.png", 2), ("c.png", 3)] { + write_png(&dir.join("old").join(n), seed); + } + let mut s = Session::new().with_fs(); + s.execute("library.import", &json!({"paths": [dir.join("old").to_string_lossy()]})).unwrap(); + // an impostor: same name and size, other bytes + let impostor = |src: &Path, dst: &Path| { + let mut b = std::fs::read(src).unwrap(); + let k = b.len() - 20; + b[k] ^= 0xff; + std::fs::write(dst, b).unwrap(); + }; + // a: renamed; b: renamed, with an impostor under its old name; c: gone, two impostors + std::fs::rename(dir.join("old/a.png"), dir.join("moved/Trip-001.png")).unwrap(); + impostor(&dir.join("old/b.png"), &dir.join("moved/b.png")); + std::fs::rename(dir.join("old/b.png"), dir.join("moved/real/Trip-002.png")).unwrap(); + impostor(&dir.join("old/c.png"), &dir.join("moved/c.png")); + impostor(&dir.join("old/c.png"), &dir.join("moved/real/c.png")); + std::fs::remove_file(dir.join("old/c.png")).unwrap(); + let r = s.execute("library.findMissing", &json!({"folder": dir.join("moved").to_string_lossy()})).unwrap(); + let found = r["found"].as_array().unwrap(); + assert_eq!(found.len(), 2, "{r}"); + let to = |name: &str| { + found.iter().find(|f| f["from"].as_str().unwrap().ends_with(name)).map(|f| (f["to"].as_str().unwrap().to_string(), f["by"].clone())) + }; + assert_eq!(to("a.png"), Some((dir.join("moved/Trip-001.png").to_string_lossy().to_string(), json!("content")))); + assert_eq!(to("b.png"), Some((dir.join("moved/real/Trip-002.png").to_string_lossy().to_string(), json!("content"))), "not the impostor"); + assert_eq!(r["missing"], 1); + assert_eq!(r["ambiguous"][0]["candidates"].as_array().map(Vec::len), Some(2), "{r}"); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn missing_files_are_found_and_relinked() { let dir = std::env::temp_dir().join(format!("lc-missing-{}", std::process::id())); diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 795c7383..b4ef0ab3 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -932,7 +932,12 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option 0 { format!(" ({unsure} with several look-alike files: use Locate)") } else { String::new() }; + app.toast( + &egui::Context::default(), + format!("Found {n} missing photo{}; {left} still missing{unsure}", if n == 1 { "" } else { "s" }), + ); } r } diff --git a/docs/parity.md b/docs/parity.md index 1dcc3806..78eca9be 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -153,7 +153,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | no confirmation dialog, no auto-purge after N days, no "Empty" | | 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, 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; undo/redo move the files; virtual copies follow | +| 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, 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 | | LR-LIB-CAPTURETIME | Edit capture time | P1 | ✅ | `cmd:photo.setCaptureTime`, `cmd:dialog.captureTime`, `crates/catalog/src/dates.rs` | set (the other selected photos shift by the same amount, or `each`), shift by days/hours/minutes, time-zone shift; one undo step, journaled; Info panel button; no “revert to original capture time” | | LR-LIB-SHOWFINDER | Reveal original in file manager | P0 | ✅ | `cmd:app.showInFinder` | ⌘R (see MENU-FILE-SHOWFINDER) | | LR-LIB-COVER | Album cover | P2 | ✅ | `cmd:album.setCover` | | @@ -727,7 +727,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-LIB-PEOPLE | People view | P2 | ⬜ | | | | LRC-LIB-COMMENTS | Comments panel | P2 | ⬜ | | | | LRC-LIB-VISUALSEARCH | Find similar photos | P2 | ✅ | `cmd:library.findSimilar`, `crates/pipeline/src/cull.rs` (`signature`) | photo menu / Photo ▸ Find Similar Photos: look-alike photos (composition and tones), most similar first, as a filter (`only`) | -| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate`, `crates/engine/src/cmd/missing.rs` (`checked_path`) | File → Find Missing Photos… (same name and size anywhere in a folder, one undo step), photo menu → Locate Missing File…; unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. Missing Photos in the sidebar (counted on a worker thread); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike | +| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate`, `crates/engine/src/cmd/missing.rs` (`checked_path`) | File → Find Missing Photos… (anywhere in a folder: same name, size and content hash; renamed files by size + content hash; same name and size alone only when unambiguous — look-alikes are skipped and reported; one undo step), photo menu → Locate Missing File…; unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. Missing Photos in the sidebar (counted on a worker thread); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike | | LRC-LIB-CONVERT | Convert to DNG | P2 | ✅ | `cmd:photo.convertToDng`, `crates/engine/src/cmd/convert.rs` | Photo ▸ Convert to DNG: lossless DNG next to the raw with the settings embedded, photo (and its virtual copies) relinked; originals kept; undoable; the DNG is decoded and compared with the raw data, written to a temp file, synced, read back and only then given a free name (never replacing a file) — a failed write leaves no DNG and the photo on its raw (issue #106) | | LRC-LIB-PREVIEWS | Build / discard previews | P1 | ✅ | `cmd:library.buildPreviews`, `cmd:library.previewProgress`, `cmd:library.cancelPreviews`, `cmd:library.clearPreviews`, `crates/engine/src/cmd/previews.rs`, `crates/preview/src/lib.rs` | File ▸ Previews: build standard-sized (Settings → Performance size) or 1:1 previews of the selected / visible photos in the background (progress toasts, stop), discard the cache; disk thumbnail + view cache; no smart previews (offline proxies) | | LRC-LIB-SLIDESHOW-IMPROMPTU | Impromptu slideshow | P2 | ✅ | `cmd:view.slideshow` | see LR-VIEW-SLIDESHOW | diff --git a/docs/whats-new.md b/docs/whats-new.md index c49c0b0e..9dd8f27c 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -42,6 +42,10 @@ - Rename Photos never overwrites another photo when only the letter case changes (issue #95): on case-sensitive volumes (Linux, case-sensitive APFS) `img_1.JPG` next to `IMG_1.JPG` is a different photo and the renamed one gets `img_1-1.JPG`; on case-insensitive volumes the case change still goes through. +- Rename Photos reports files it could not move back after a failure (issue #105), e.g. when a network share drops + mid-batch: the error lists them (old → new) and the library points at their new names (an undoable partial rename), + so none shows as missing. Find Missing Photos also finds renamed files by their content, prefers a content match + over a same-name same-size look-alike, and skips (and reports) photos it can't tell apart instead of guessing. - Smart albums with a rule editor: match all / any / none, nested groups, 26 fields. - Quick Collection and target album (B in the grid), keyword sets (⌥1–⌥9), colour-label sets. - Colour-label filter with several labels at once; expandable folder tree in Local.