From ce3e2215fef9e55b388d5fadbcec46a5729bffef Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:04:21 -0400 Subject: [PATCH] Issue #22: Local: recognise equivalent folder paths Local compared folder paths as raw strings, so a built-in location built as "{home}/{sub}" and the same folder returned by library.browse with native separators could be listed twice. Folder identity now goes through catalog::query::folder_key (both separators, trailing/doubled separators, ./.., verbatim and UNC prefixes, drive-letter case; whole path case-folded on Windows): the sidebar's root list, the highlighted and expanded-on-the-way folder, saved Browse Folder roots and local.hide / local.restoreHidden all compare by it. Built-in paths are joined with the platform separator. Co-Authored-By: Claude Opus 5.5 --- crates/catalog/src/query.rs | 46 ++++++++++++++++++ crates/catalog/src/tests.rs | 33 +++++++++++++ crates/ui-egui/src/menus.rs | 4 +- crates/ui-egui/src/panels/left.rs | 79 ++++++++++++++++++++++++------- docs/parity.md | 2 +- 5 files changed, 145 insertions(+), 19 deletions(-) diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index bac31f48..417d6bf0 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -346,6 +346,52 @@ pub fn in_folder(path: &str, dir: &str, deep: bool) -> bool { !rest.is_empty() && (deep || !rest.contains(['/', '\\'])) } +/// A folder path's identity, for telling whether two spellings name the same folder: `/` and +/// `\\` are both separators, repeated separators, `.` and a trailing separator are dropped, +/// `..` is resolved lexically, a Windows verbatim prefix (`\\?\`) is removed and the drive +/// letter lower-cased; on Windows (case-insensitive file names) the whole path is lower-cased. +/// No file-system access: the paths compared should already be absolute. +pub fn folder_key(path: &str) -> String { + let mut s = path.replace('\\', "/"); + if let Some(rest) = s.strip_prefix("//?/").or_else(|| s.strip_prefix("//./")) { + s = match rest.strip_prefix("UNC/") { + Some(unc) => format!("//{unc}"), + None => rest.to_string(), + }; + } + // the part `..` can't climb above: `//server/share`, `c:` or `/` + let (prefix, rest) = if let Some(unc) = s.strip_prefix("//").filter(|u| !u.is_empty() && !u.starts_with('/')) { + let mut it = unc.splitn(3, '/'); + let (server, share, rest) = (it.next().unwrap_or(""), it.next().unwrap_or(""), it.next().unwrap_or("")); + (format!("//{server}/{share}"), rest.to_string()) + } else if s.len() >= 2 && s.as_bytes()[1] == b':' && s.as_bytes()[0].is_ascii_alphabetic() { + (s[..2].to_ascii_lowercase(), s[2..].to_string()) + } else { + (String::new(), s.clone()) + }; + let absolute = rest.starts_with('/') || !prefix.is_empty(); + let mut parts: Vec<&str> = Vec::new(); + for c in rest.split('/') { + match c { + "" | "." => {} + ".." if parts.last().is_some_and(|p| *p != "..") => { + parts.pop(); + } + ".." if absolute => {} + c => parts.push(c), + } + } + let key = if absolute { format!("{prefix}/{}", parts.join("/")) } else { parts.join("/") }; + if cfg!(windows) { key.to_lowercase() } else { key } +} + +/// Whether `path` is folder `root` or lies somewhere inside it, however either is spelled +/// (see [`folder_key`]). +pub fn folder_within(path: &str, root: &str) -> bool { + let (p, r) = (folder_key(path), folder_key(root)); + p == r || (p.starts_with(&r) && (r.ends_with('/') || p[r.len()..].starts_with('/'))) +} + /// The kind of Photo Merge result a file is, from the name merges give it (`IMG_1-HDR.dng`, /// `IMG_1-Pano.dng`, `IMG_1-HDR-Pano.dng` — also the names other raw editors use): /// `hdr`, `panorama` or `hdrPanorama`. diff --git a/crates/catalog/src/tests.rs b/crates/catalog/src/tests.rs index 94004eb7..86442d2a 100644 --- a/crates/catalog/src/tests.rs +++ b/crates/catalog/src/tests.rs @@ -264,3 +264,36 @@ fn preview_only_survives_serde_and_set_content_undo() { assert_eq!(c.photo(a).unwrap().preview_only.as_deref(), Some("Nikon Huffman-compressed NEF")); assert_eq!(c.photo(a).unwrap().width, 6000); } + +/// Equivalent spellings of one folder (separators, trailing or doubled separators, `.`/`..`, +/// drive-letter case, verbatim and UNC prefixes) share an identity; different folders don't. +#[test] +fn folder_identity_ignores_spelling() { + use crate::query::{folder_key, folder_within}; + let same = |a: &str, b: &str| assert_eq!(folder_key(a), folder_key(b), "{a} vs {b}"); + let differ = |a: &str, b: &str| assert_ne!(folder_key(a), folder_key(b), "{a} vs {b}"); + for (a, b) in [ + ("D:/Example/Photos", "D:\\Example\\Photos"), + ("D:/Example/Photos", "d:\\Example\\Photos\\"), + ("D:\\Example/Photos", "D:/Example//Photos/."), + ("D:/Example/Photos", "D:/Example/Other/../Photos"), + ("D:/Example/Photos", "\\\\?\\D:\\Example\\Photos"), + ("\\\\server\\share\\Photos", "//server/share/Photos/"), + ("\\\\?\\UNC\\server\\share\\Photos", "//server/share/Photos"), + ("/home/me/Photos", "/home/me/Photos/"), + ("/home/me/Photos", "/home//me/./Photos"), + ("/", "//"), + ("C:\\", "c:/"), + ] { + same(a, b); + } + differ("D:/Example/Photos", "D:/Example/Photos2"); + differ("D:/Example/Photos", "E:/Example/Photos"); + differ("/home/me/Photos", "/home/me"); + assert_eq!(folder_key("/a/../../b"), "/b", "`..` stops at the root"); + assert!(folder_within("D:\\Example\\Photos\\2026", "D:/Example/Photos/")); + assert!(folder_within("D:/Example/Photos", "d:\\Example\\Photos")); + assert!(!folder_within("D:/Example/Photos2", "D:/Example/Photos")); + assert!(folder_within("/a/b", "/")); + assert!(folder_within("C:\\x", "c:\\")); +} diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index fde1e087..6f29a310 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -557,7 +557,7 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option match p.get("path").and_then(Value::as_str) { Some(path) => { - if !app.ui.hidden_locations.iter().any(|h| h == path) { + if !app.ui.hidden_locations.iter().any(|h| crate::panels::left::same_folder(h, path)) { app.ui.hidden_locations.push(path.to_string()); } Ok(json!({"hidden": app.ui.hidden_locations})) @@ -567,7 +567,7 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { // one path, or (no path) every hidden location match p.get("path").and_then(Value::as_str) { - Some(path) => app.ui.hidden_locations.retain(|h| h != path), + Some(path) => app.ui.hidden_locations.retain(|h| !crate::panels::left::same_folder(h, path)), None => app.ui.hidden_locations.clear(), } Ok(json!({"hidden": app.ui.hidden_locations})) diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index 72516322..bd4201c8 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -203,27 +203,19 @@ fn local_section(app: &mut LightcraftApp, ui: &mut egui::Ui) { let (lr, _) = ui.allocate_exact_size(vec2(ui.available_width(), 34.0), Sense::hover()); ui.painter().text(pos2(lr.left() + 18.0, lr.center().y), Align2::LEFT_CENTER, "Local", t.semibold(13.5), t.text_label); let home = std::env::var("HOME").or_else(|_| std::env::var("USERPROFILE")).unwrap_or_default(); - let mut places: Vec<(String, String)> = Vec::new(); + let mut builtin: Vec<(String, String)> = Vec::new(); if !home.is_empty() { for (name, sub) in [("Pictures", "Pictures"), ("Desktop", "Desktop"), ("Downloads", "Downloads"), ("Home", "")] { - let p = if sub.is_empty() { home.clone() } else { format!("{home}/{sub}") }; + // joined with the platform's separator, like the paths browsing gives back + let p = if sub.is_empty() { home.clone() } else { std::path::Path::new(&home).join(sub).to_string_lossy().to_string() }; if std::path::Path::new(&p).is_dir() { - places.push((name.to_string(), p)); + builtin.push((name.to_string(), p)); } } } let browsing = app.session.browse.clone().filter(|_| app.session.source == LibrarySource::Folder); - // picked folders stay listed; the folder being browsed is listed even when it wasn't picked - let roots = app.ui.local_roots.iter().cloned().chain(browsing.iter().map(|b| b.path.clone())); - for path in roots { - if !places.iter().any(|(_, p)| *p == path) { - let name = std::path::Path::new(&path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.clone()); - places.push((name, path)); - } - } let current = browsing.as_ref().map(|b| b.path.clone()); - // hidden locations stay reachable: breadcrumbs, Browse Folder…, and the row below - places.retain(|(_, p)| !app.ui.hidden_locations.contains(p)); + let places = local_places(builtin, &app.ui.local_roots, current.as_deref(), &app.ui.hidden_locations); for (name, path) in places { folder_tree(app, ui, &name, &path, 0.0, current.as_deref()); } @@ -235,7 +227,7 @@ fn local_section(app: &mut LightcraftApp, ui: &mut egui::Ui) { match app.run("library.browse", json!({"path": path})) { Ok(r) => { let dir = r["path"].as_str().unwrap_or(&path).to_string(); - if !app.ui.local_roots.contains(&dir) { + if !app.ui.local_roots.iter().any(|p| same_folder(p, &dir)) { app.ui.local_roots.push(dir); } } @@ -256,6 +248,28 @@ fn local_section(app: &mut LightcraftApp, ui: &mut egui::Ui) { ui.add_space(10.0); } +/// Whether two paths name the same folder, however they are spelled (separators, trailing +/// slash, `.`/`..`, drive-letter case; see `lightcraft_catalog::query::folder_key`). +pub(crate) fn same_folder(a: &str, b: &str) -> bool { + a == b || lightcraft_catalog::query::folder_key(a) == lightcraft_catalog::query::folder_key(b) +} + +/// Local's top-level folders, in order: the built-in places, the folders picked with Browse +/// Folder…, then the folder being browsed when it isn't one of those. Each folder is listed +/// once however its path is spelled (the first spelling wins), and hidden ones are left out — +/// they stay reachable through breadcrumbs and Browse Folder…. +pub(crate) fn local_places(builtin: Vec<(String, String)>, saved: &[String], browsing: Option<&str>, hidden: &[String]) -> Vec<(String, String)> { + let mut places = builtin; + for path in saved.iter().map(String::as_str).chain(browsing) { + if !places.iter().any(|(_, p)| same_folder(p, path)) { + let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string()); + places.push((name, path.to_string())); + } + } + places.retain(|(_, p)| !hidden.iter().any(|h| same_folder(h, p))); + places +} + /// The subfolders of `path` (not hidden ones), sorted; listed at most every 2 s per folder. fn subfolders(ui: &egui::Ui, path: &str) -> Vec<(String, String)> { let id = egui::Id::new(("subfolders", path.to_string())); @@ -286,9 +300,9 @@ fn subfolders(ui: &egui::Ui, path: &str) -> Vec<(String, String)> { fn folder_tree(app: &mut LightcraftApp, ui: &mut egui::Ui, name: &str, path: &str, indent: f32, current: Option<&str>) { let t = Tokens::get(ui.ctx()); let open_id = egui::Id::new(("folder-open", path.to_string())); - let on_the_way = current.is_some_and(|c| c != path && std::path::Path::new(c).starts_with(path)); + let sel = current.is_some_and(|c| same_folder(c, path)); + let on_the_way = !sel && current.is_some_and(|c| lightcraft_catalog::query::folder_within(c, path)); let mut open: bool = ui.data(|d| d.get_temp(open_id)).unwrap_or(on_the_way); - let sel = current == Some(path); let resp = row(app, ui, &format!("local:{path}"), Icon::Folder, name, None, sel, indent + 12.0).on_hover_text(path); let c = pos2(resp.rect.left() + 10.0 + indent, resp.rect.center().y); let tri = Rect::from_center_size(c, vec2(14.0, 14.0)); @@ -597,3 +611,36 @@ fn keyword_rows(app: &mut LightcraftApp, ui: &mut egui::Ui, nodes: &[KeywordNode } } } + +#[cfg(test)] +mod tests { + use super::local_places; + + fn names(v: &[(String, String)]) -> Vec<&str> { + v.iter().map(|(n, _)| n.as_str()).collect() + } + + /// A built-in location browsed (or picked) under another spelling of its path is the same + /// row, not a second root — for every built-in, not just named ones. + #[test] + fn equivalent_paths_are_one_local_location() { + for (home, sep) in [("D:\\Example", '\\'), ("D:/Example", '/'), ("/home/example", '/')] { + let builtin: Vec<(String, String)> = + ["Pictures", "Desktop", "Downloads"].iter().map(|s| (s.to_string(), format!("{home}{sep}{s}"))).collect(); + for (_, p) in builtin.clone() { + // the browsed spelling: other separators, a trailing one, a different drive-letter case + let flipped = p.replace(['/', '\\'], if sep == '/' { "\\" } else { "/" }); + for browsing in [flipped.clone(), format!("{p}{sep}"), p.replacen("D:", "d:", 1)] { + let places = local_places(builtin.clone(), std::slice::from_ref(&browsing), Some(&browsing), &[]); + assert_eq!(names(&places), ["Pictures", "Desktop", "Downloads"], "{p} browsed as {browsing}"); + } + // hiding under one spelling hides the other + let places = local_places(builtin.clone(), &[], None, std::slice::from_ref(&flipped)); + assert_eq!(places.len(), 2, "hidden {flipped}"); + } + } + // a different folder is still added + let places = local_places(vec![("Pictures".into(), "/home/example/Pictures".into())], &[], Some("/home/example/Pictures2"), &[]); + assert_eq!(names(&places), ["Pictures", "Pictures2"]); + } +} diff --git a/docs/parity.md b/docs/parity.md index a33caa46..2d1b7392 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -108,7 +108,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-IMP-RAWDEFAULT | Raw defaults | P1 | ✅ | `cmd:library.preferences`, `crates/engine/src/import.rs`, `crates/ui-egui/src/panels/settings.rs` | LightCraft default / a preset / per camera (make + model); non-raw default too; the preset look counts as unedited and Reset returns to it | | LR-IMP-MIGRATE | Migrate other catalogs | OOS | 🚫 | | | | LR-IMP-PROFILES | Import profiles & presets | P1 | ✅ | `cmd:file.importPresets`, `cmd:preset.import`, `cmd:profile.import`, `crates/engine/src/preset_import.rs`, `crates/engine/src/preset_luminar.rs` | presets: .lcpreset, XMP, classic .lrtemplate, photos carrying edits ("DNG presets"), Luminar looks (.lmp, .mplumpack collections; sliders with a counterpart), .zip bundles, folders (folder → group), drag & drop; masks carried over; unmapped settings reported; profiles: .cube 3D LUTs; Adobe profile formats deliberately unsupported | -| LR-IMP-LOCAL | Work on files in place | P0 | ✅ | `cmd:library.browse`, `cmd:photo.addToLibrary`, `cmd:library.import` (mode add), `cmd:local.hide`, `cmd:local.restoreHidden`, `crates/engine/src/cmd/browse.rs`, `crates/ui-egui/src/panels/left.rs` (`local_section`), `crates/ui-egui/src/panels/grid.rs` (`folder_header`) | Local: browse Pictures / Desktop / Downloads / Home or any folder without adding it (breadcrumb, Include subfolders, Add N to My Photos); browsed photos stay out of All Photos, albums and counts; edits go to XMP sidecars; importing promotes them; right-click a top-level location → Remove from Local hides the shortcut only (nothing on disk or in the catalog changes), saved with the UI state, and “Show N hidden locations” (or Browse Folder… on it) brings it back | +| LR-IMP-LOCAL | Work on files in place | P0 | ✅ | `cmd:library.browse`, `cmd:photo.addToLibrary`, `cmd:library.import` (mode add), `cmd:local.hide`, `cmd:local.restoreHidden`, `crates/engine/src/cmd/browse.rs`, `crates/ui-egui/src/panels/left.rs` (`local_section`), `crates/ui-egui/src/panels/grid.rs` (`folder_header`) | Local: browse Pictures / Desktop / Downloads / Home or any folder without adding it (breadcrumb, Include subfolders, Add N to My Photos); browsed photos stay out of All Photos, albums and counts; edits go to XMP sidecars; importing promotes them; right-click a top-level location → Remove from Local hides the shortcut only (nothing on disk or in the catalog changes), saved with the UI state, and “Show N hidden locations” (or Browse Folder… on it) brings it back; a folder is one location however its path is spelled (`/` or `\`, trailing separator, `.`/`..`, drive-letter case; `folder_key` in `crates/catalog/src/query.rs`) | | LR-IMP-SIDECAR-SPLIT | Separate XMP sidecar variants | P2 | ⬜ | `cmd:library.xmpPreferences` | sidecar naming option exists (stem/full), no split sidecars | | LR-IMP-FORMATS | Supported formats | P0 | 🟡 | `crates/codecs/src/lib.rs`, `crates/raw/src/lib.rs` | JPEG, PNG, TIFF, WebP, JXL, PSD, GIF, BMP; DNG (incl. lossy / Smart Preview DNG), CR2, ARW, NEF, RAF, RW2, PEF, ORF. Missing: CR3, compressed NEF/RAF/ORF, RW2 v4 (preview only: imported with `preview_only` = the decoder's reason, rendered as a rendered JPEG, flagged by a grid badge and loupe/Edit/Info notices, `previewOnly` in `catalog.query`; Reload clears it once the file decodes), HEIC/AVIF decode | | LR-IMP-CULL-AT-IMPORT | Culling analysis at import | P2 | 🟡 | `cmd:photo.analyze` | run Assisted Culling on the imported photos (they're selected after an import); not automatic |