Merge pull request #394 from storytold/claude/integrate-batch-7
Integrate part of #374 (bounded folder walks, launch imports off the UI thread) and #385, #386, #387
This commit is contained in:
BFlat
GitHub
commit
ab9e8d372c
|
|
@ -469,6 +469,31 @@ ENVIRONMENT:
|
|||
<settings folder>/logs/lightcraft.log (previous runs: lightcraft.1.log, lightcraft.2.log)
|
||||
";
|
||||
|
||||
/// The launch arguments' files and folders to import (absolute), and a notice for each one that
|
||||
/// is not imported (see [`lightcraft_engine::import::launch_path`]; issue #374).
|
||||
fn launch_imports(args: &[String]) -> (Vec<String>, Vec<String>) {
|
||||
let cwd = std::env::current_dir().ok();
|
||||
let home = std::env::var_os(if cfg!(windows) { "USERPROFILE" } else { "HOME" }).filter(|h| !h.is_empty()).map(std::path::PathBuf::from);
|
||||
launch_imports_in(args, cwd.as_deref(), home.as_deref())
|
||||
}
|
||||
|
||||
fn launch_imports_in(args: &[String], cwd: Option<&std::path::Path>, home: Option<&std::path::Path>) -> (Vec<String>, Vec<String>) {
|
||||
use lightcraft_engine::import::{LaunchPath, launch_path};
|
||||
let (mut files, mut notices) = (Vec::new(), Vec::new());
|
||||
for a in args {
|
||||
match launch_path(a, cwd, home, lightcraft_engine::devices::dcim_in) {
|
||||
Some(LaunchPath::Import(p)) => files.push(p),
|
||||
Some(LaunchPath::Refused { path, why }) => {
|
||||
log::warn!("not importing {path} given at launch: {why}");
|
||||
notices
|
||||
.push(format!("{path} was not imported: it is {why}. To import from it, choose a folder inside it with File ▸ Import Photos…"));
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
}
|
||||
(files, notices)
|
||||
}
|
||||
|
||||
/// Where the log files live: `logs` in the settings folder, next to `ui.json` (see `logging`).
|
||||
/// None with `LIGHTCRAFT_NO_PREFS` (tests, scripts), so those runs don't rotate away the user's logs.
|
||||
fn log_dir() -> Option<std::path::PathBuf> {
|
||||
|
|
@ -541,6 +566,9 @@ fn main() -> eframe::Result {
|
|||
None => logger.no_file(),
|
||||
}
|
||||
}
|
||||
// what the arguments name, resolved now: an empty or relative argument never becomes a drive
|
||||
// root, and a whole drive or home folder is not walked (issue #374)
|
||||
let (files, launch_notices) = launch_imports(&files);
|
||||
let (prefs, prefs_warning, keep_prefs_file) = load_prefs(in_memory);
|
||||
// the saved language from the start, so a failed start is reported in it too
|
||||
if let Some(ui) = &prefs {
|
||||
|
|
@ -593,6 +621,7 @@ fn main() -> eframe::Result {
|
|||
lightcraft_ui_egui::i18n::set_language(app.ui.language);
|
||||
app.integrated_titlebar = cfg!(target_os = "macos");
|
||||
app.notices.extend(prefs_warning);
|
||||
app.notices.extend(launch_notices);
|
||||
// what's on disk now: only changes are written
|
||||
let writer = PrefsWriter {
|
||||
path: if in_memory { None } else { prefs_path() },
|
||||
|
|
@ -615,7 +644,12 @@ fn main() -> eframe::Result {
|
|||
p.pending_import = files;
|
||||
app.library_problem = Some(p);
|
||||
} else if !files.is_empty() {
|
||||
let _ = app.run("library.import", serde_json::json!({"paths": files}));
|
||||
// listed and read on the import worker once the window shows (issue #374: never
|
||||
// here, before the first frame, where a large folder kept the window from
|
||||
// appearing)
|
||||
if let Err(e) = lightcraft_ui_egui::import::start_paths(&mut app, files) {
|
||||
log::warn!("import at launch: {e}");
|
||||
}
|
||||
app.ui.view = lightcraft_ui_egui::state::ViewMode::PhotoGrid;
|
||||
}
|
||||
// native menu bar generated from the command registry (macOS; elsewhere the menus are
|
||||
|
|
@ -665,6 +699,24 @@ mod tests {
|
|||
d
|
||||
}
|
||||
|
||||
/// Issue #374: launch arguments that would walk a whole drive or home folder are reported,
|
||||
/// not imported; empty ones are ignored; the rest are imported as absolute paths.
|
||||
#[test]
|
||||
fn launch_arguments_never_walk_a_drive_or_the_home_folder() {
|
||||
let home = dir("launch");
|
||||
std::fs::create_dir_all(home.join("Trip")).unwrap();
|
||||
let args: Vec<String> = ["", " ", ".", "Trip", &home.to_string_lossy()].iter().map(|s| s.to_string()).collect();
|
||||
let (files, notices) = launch_imports_in(&args, Some(&home), Some(&home));
|
||||
assert_eq!(files, [home.join("Trip").to_string_lossy().to_string()]);
|
||||
assert_eq!(notices.len(), 2, "{notices:?}");
|
||||
assert!(notices.iter().all(|n| n.contains("was not imported")), "{notices:?}");
|
||||
// the working folder is a drive root (a launcher's choice): `.` is never the whole drive
|
||||
let root = home.ancestors().last().unwrap().to_path_buf();
|
||||
let (files, notices) = launch_imports_in(&[".".to_string()], Some(&root), None);
|
||||
assert!(files.iter().all(|f| f.ends_with("DCIM")) && files.len() + notices.len() == 1, "{files:?} {notices:?}");
|
||||
let _ = std::fs::remove_dir_all(&home);
|
||||
}
|
||||
|
||||
/// Issue #103: a damaged ui.json (which holds the library location) is kept aside and
|
||||
/// reported, not silently replaced by the defaults.
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -525,6 +525,17 @@ pub fn folder_within(path: &str, root: &str) -> bool {
|
|||
key_within(&folder_key(path), &folder_key(root))
|
||||
}
|
||||
|
||||
/// The names below folder `root` that lead to `path`, as the caller spelled them (`None` when
|
||||
/// `path` is not at or inside `root`; empty when it is `root`). Compared by [`folder_key`], but
|
||||
/// the names keep their case, which the key lowers on Windows.
|
||||
pub fn folder_rest(path: &str, root: &str) -> Option<Vec<String>> {
|
||||
if !key_within(&folder_key(path), &folder_key(root)) {
|
||||
return None;
|
||||
}
|
||||
let depth = split_path(root, false).parts.len();
|
||||
Some(split_path(path, false).parts.into_iter().skip(depth).collect())
|
||||
}
|
||||
|
||||
/// Whether a library photo (not one only browsed in Local) lies in the folder whose
|
||||
/// [`folder_key`] is `root`. A plain POSIX path (no `\`, `.`, `..`, repeated or trailing
|
||||
/// separators) is its own identity, so it is read without building a key for it: the check runs
|
||||
|
|
|
|||
|
|
@ -1179,15 +1179,17 @@ mod tests {
|
|||
assert!(fit_pairs(&sensor, &reference).is_none());
|
||||
}
|
||||
|
||||
/// X-Trans and 16-bit Bayer RAFs use the same fixed fit at thumbnail and export sizes;
|
||||
/// a poor reference keeps the fallback. Skips cleanly without these CC0 corpus files.
|
||||
/// X-Trans and 16-bit Bayer RAFs use the same fixed fit at thumbnail and export sizes.
|
||||
/// The X-T20 fails the gates on all pixels (held-out RMS 0.114 against 0.10) and passes away
|
||||
/// from edges (0.056, issue #232), so it is fitted too. Skips cleanly without these CC0
|
||||
/// corpus files.
|
||||
#[test]
|
||||
fn corpus_raf_colour_and_white_balance() {
|
||||
let dir = std::env::var_os("LIGHTCRAFT_CORPUS")
|
||||
.map(std::path::PathBuf::from)
|
||||
.unwrap_or_else(|| std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus"))
|
||||
.join("raw");
|
||||
for (name, accepted) in [("raf-fuji-xt2-865.raf", true), ("raf-fuji-gfx100s-4503.raf", true), ("raf-fuji-xt20-compressed.raf", false)] {
|
||||
for (name, accepted) in [("raf-fuji-xt2-865.raf", true), ("raf-fuji-gfx100s-4503.raf", true), ("raf-fuji-xt20-compressed.raf", true)] {
|
||||
let Ok(bytes) = std::fs::read(dir.join(name)) else { continue };
|
||||
let header = crate::files::probe_bytes(name, &bytes).unwrap();
|
||||
assert_eq!(header.as_shot_wb, Some((6500.0, 0.0)), "{name}");
|
||||
|
|
|
|||
|
|
@ -268,13 +268,9 @@ pub(crate) fn follow_folder(s: &mut Session, from: &str, to: &str) {
|
|||
/// the folder itself.
|
||||
fn follow_library_folder(chosen: &mut Option<String>, from: &str, to: &str) {
|
||||
let Some(c) = chosen.as_deref() else { return };
|
||||
let (ck, fk) = (lightcraft_catalog::query::folder_key(c), lightcraft_catalog::query::folder_key(from));
|
||||
if fk.is_empty() {
|
||||
return;
|
||||
}
|
||||
if let Some(rest) = ck.strip_prefix(fk.as_str()).filter(|r| r.is_empty() || r.starts_with('/') || fk.ends_with('/')) {
|
||||
let rest = rest.trim_start_matches('/');
|
||||
*chosen = Some(if rest.is_empty() { to.to_string() } else { format!("{}/{rest}", to.trim_end_matches(['/', '\\'])) });
|
||||
if let Some(rest) = lightcraft_catalog::query::folder_rest(c, from) {
|
||||
// joined by the platform, so the result is spelled like `to` and like a path read from disk
|
||||
*chosen = Some(rest.iter().fold(std::path::PathBuf::from(to), |p, name| p.join(name)).to_string_lossy().to_string());
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -41,22 +41,11 @@ fn cube_files(paths: &[String]) -> Vec<(String, String, Option<String>)> {
|
|||
for p in paths {
|
||||
let path = Path::new(p);
|
||||
if path.is_dir() {
|
||||
let mut stack = vec![path.to_path_buf()];
|
||||
while let Some(d) = stack.pop() {
|
||||
let Ok(rd) = std::fs::read_dir(&d) else { continue };
|
||||
for e in rd.flatten() {
|
||||
let ep = e.path();
|
||||
if ep.file_name().is_some_and(|n| n.to_string_lossy().starts_with('.')) {
|
||||
continue;
|
||||
}
|
||||
if ep.is_dir() {
|
||||
stack.push(ep);
|
||||
} else if ep.extension().is_some_and(|x| x.eq_ignore_ascii_case("cube"))
|
||||
&& let Ok(t) = std::fs::read_to_string(&ep)
|
||||
{
|
||||
let group = ep.parent().and_then(|d| d.file_name()).map(|n| n.to_string_lossy().to_string());
|
||||
out.push((ep.file_stem().map(|n| n.to_string_lossy().to_string()).unwrap_or_default(), t, group));
|
||||
}
|
||||
let cube = |f: &Path| f.extension().is_some_and(|x| x.eq_ignore_ascii_case("cube"));
|
||||
for ep in crate::walk::files_in(path, None, crate::walk::Limits::default(), cube).files {
|
||||
if let Ok(t) = std::fs::read_to_string(&ep) {
|
||||
let group = ep.parent().and_then(|d| d.file_name()).map(|n| n.to_string_lossy().to_string());
|
||||
out.push((ep.file_stem().map(|n| n.to_string_lossy().to_string()).unwrap_or_default(), t, group));
|
||||
}
|
||||
}
|
||||
} else if path.extension().is_some_and(|x| x.eq_ignore_ascii_case("zip")) {
|
||||
|
|
|
|||
|
|
@ -191,7 +191,7 @@ pub fn devices_in(parents: &[std::path::PathBuf], extra: Vec<std::path::PathBuf>
|
|||
|
||||
/// The volume's `DCIM` folder: the usual spellings are checked directly, so a volume's root is
|
||||
/// only listed (other spellings on a case-sensitive volume) off Windows, whose drives aren't.
|
||||
fn dcim_in(root: &std::path::Path) -> Option<std::path::PathBuf> {
|
||||
pub fn dcim_in(root: &std::path::Path) -> Option<std::path::PathBuf> {
|
||||
if let Some(p) = ["DCIM", "dcim"].iter().map(|n| root.join(n)).find(|p| p.is_dir()) {
|
||||
return Some(p);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -317,32 +317,96 @@ pub fn is_supported(path: &Path) -> bool {
|
|||
}
|
||||
|
||||
/// Expand files and folders (recursively) into supported files. `skip` (e.g. the library folder)
|
||||
/// is never descended into.
|
||||
/// is never descended into. Each folder's walk is bounded ([`crate::walk::Limits::default`]); a
|
||||
/// walk that stops at a bound is logged.
|
||||
pub fn expand(paths: &[String], skip: Option<&Path>) -> Vec<String> {
|
||||
fn walk(p: &Path, skip: Option<&Path>, out: &mut Vec<String>, top: bool) {
|
||||
let hidden = p.file_name().is_some_and(|n| n.to_string_lossy().starts_with('.'));
|
||||
if (hidden && !top) || skip.is_some_and(|s| p == s) {
|
||||
return;
|
||||
expand_within(paths, skip, crate::walk::Limits::default()).0
|
||||
}
|
||||
|
||||
/// [`expand`] with the walk bounded by `limits` → (files, whether a walk stopped at a bound).
|
||||
pub fn expand_within(paths: &[String], skip: Option<&Path>, limits: crate::walk::Limits) -> (Vec<String>, bool) {
|
||||
let mut out = Vec::new();
|
||||
let mut truncated = false;
|
||||
for p in paths {
|
||||
let p = Path::new(p);
|
||||
if skip.is_some_and(|s| p == s) {
|
||||
continue;
|
||||
}
|
||||
if p.is_dir() {
|
||||
let Ok(rd) = std::fs::read_dir(p) else { return };
|
||||
let mut v: Vec<PathBuf> = rd.flatten().map(|e| e.path()).collect();
|
||||
v.sort();
|
||||
for c in v {
|
||||
walk(&c, skip, out, false);
|
||||
let w = crate::walk::files_in(p, skip, limits, is_supported);
|
||||
if w.truncated {
|
||||
log::warn!(
|
||||
"import: stopped looking in {} after {} entries (limits: {} entries, {} folder levels)",
|
||||
p.display(),
|
||||
w.entries,
|
||||
limits.max_entries,
|
||||
limits.max_depth
|
||||
);
|
||||
}
|
||||
} else if top || is_supported(p) {
|
||||
truncated |= w.truncated;
|
||||
out.extend(w.files.into_iter().map(|f| f.to_string_lossy().to_string()));
|
||||
} else {
|
||||
// explicitly named files are attempted even with an unknown extension (sniffed)
|
||||
out.push(p.to_string_lossy().to_string());
|
||||
}
|
||||
}
|
||||
let mut out = Vec::new();
|
||||
for p in paths {
|
||||
walk(Path::new(p), skip, &mut out, true);
|
||||
}
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
out.retain(|p| seen.insert(p.clone()));
|
||||
out
|
||||
(out, truncated)
|
||||
}
|
||||
|
||||
/// What a path given on the command line at launch becomes ([`launch_path`]).
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub enum LaunchPath {
|
||||
/// Import this (absolute) file or folder, in the background.
|
||||
Import(String),
|
||||
/// Not imported: `path` (absolute) is a whole drive or the whole home folder, which is never
|
||||
/// walked without being chosen in the app (issue #374).
|
||||
Refused { path: String, why: String },
|
||||
}
|
||||
|
||||
/// Resolve a launch argument for importing (issue #374). An empty argument is ignored (`None`);
|
||||
/// a relative one (`.`, `..`, `photos`; `/` or `\` on Windows, the current drive's root) is
|
||||
/// resolved against `cwd` — the working folder a launcher picked, often a drive root or
|
||||
/// `C:\Windows\System32` — and refused when that leaves no folder (`cwd` unknown). A drive
|
||||
/// or file-system root is replaced by its camera folder (`DCIM`, from `dcim`: a memory card) or
|
||||
/// refused, and so is the home folder `home`: either would walk most of the disk.
|
||||
pub fn launch_path(arg: &str, cwd: Option<&Path>, home: Option<&Path>, dcim: impl Fn(&Path) -> Option<PathBuf>) -> Option<LaunchPath> {
|
||||
if arg.trim().is_empty() {
|
||||
return None;
|
||||
}
|
||||
let given = Path::new(arg);
|
||||
let joined = if given.is_absolute() {
|
||||
given.to_path_buf()
|
||||
} else {
|
||||
match cwd {
|
||||
Some(c) => c.join(given),
|
||||
None => return Some(LaunchPath::Refused { path: arg.to_string(), why: "a relative path, and the working folder is unknown".into() }),
|
||||
}
|
||||
};
|
||||
// `.` and `..` resolved by name (no file-system access)
|
||||
let mut abs = PathBuf::new();
|
||||
for c in joined.components() {
|
||||
match c {
|
||||
std::path::Component::CurDir => {}
|
||||
std::path::Component::ParentDir => {
|
||||
abs.pop();
|
||||
}
|
||||
other => abs.push(other.as_os_str()),
|
||||
}
|
||||
}
|
||||
let shown = abs.to_string_lossy().to_string();
|
||||
if abs.parent().is_none() {
|
||||
return Some(match dcim(&abs) {
|
||||
Some(d) => LaunchPath::Import(d.to_string_lossy().to_string()),
|
||||
None => LaunchPath::Refused { path: shown, why: "a whole drive".into() },
|
||||
});
|
||||
}
|
||||
let key = lightcraft_catalog::query::folder_key;
|
||||
if home.is_some_and(|h| key(&h.to_string_lossy()) == key(&shown)) {
|
||||
return Some(LaunchPath::Refused { path: shown, why: "the whole home folder".into() });
|
||||
}
|
||||
Some(LaunchPath::Import(shown))
|
||||
}
|
||||
|
||||
/// Probe files (headers + content hash) as an import would.
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ pub mod segment;
|
|||
pub mod sidecar;
|
||||
pub mod smart;
|
||||
mod view;
|
||||
pub mod walk;
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
|
|
|
|||
|
|
@ -83,28 +83,22 @@ pub fn parse_preset_file(name: &str, bytes: &[u8]) -> Result<Vec<Preset>, String
|
|||
Ok(out)
|
||||
}
|
||||
|
||||
/// Expand files/folders (recursively) into preset files (`.lcpreset`, `.xmp`).
|
||||
/// Expand files/folders (recursively, bounded: see [`crate::walk`]) into preset files
|
||||
/// (`.lcpreset`, `.xmp`).
|
||||
pub fn expand_preset_paths(paths: &[String]) -> Vec<String> {
|
||||
fn walk(p: &Path, out: &mut Vec<String>, top: bool) {
|
||||
if p.is_dir() {
|
||||
let Ok(rd) = std::fs::read_dir(p) else { return };
|
||||
let mut v: Vec<_> = rd.flatten().map(|e| e.path()).collect();
|
||||
v.sort();
|
||||
for c in v {
|
||||
if !c.file_name().is_some_and(|n| n.to_string_lossy().starts_with('.')) {
|
||||
walk(&c, out, false);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
let ext = p.extension().map(|e| e.to_string_lossy().to_ascii_lowercase()).unwrap_or_default();
|
||||
if top || ext == LCPRESET_EXT || ["xmp", "lrtemplate", "zip", "lmp", "mplumpack"].contains(&ext.as_str()) {
|
||||
out.push(p.to_string_lossy().to_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
let preset_file = |p: &Path| {
|
||||
let ext = p.extension().map(|e| e.to_string_lossy().to_ascii_lowercase()).unwrap_or_default();
|
||||
ext == LCPRESET_EXT || ["xmp", "lrtemplate", "zip", "lmp", "mplumpack"].contains(&ext.as_str())
|
||||
};
|
||||
let mut out = Vec::new();
|
||||
for p in paths {
|
||||
walk(Path::new(p), &mut out, true);
|
||||
let path = Path::new(p);
|
||||
if path.is_dir() {
|
||||
let w = crate::walk::files_in(path, None, crate::walk::Limits::default(), preset_file);
|
||||
out.extend(w.files.into_iter().map(|f| f.to_string_lossy().to_string()));
|
||||
} else {
|
||||
out.push(p.clone());
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
|
|
|||
|
|
@ -189,6 +189,12 @@ fn the_folder_filter_takes_only_a_text_path() {
|
|||
assert_eq!(s.visible().len(), 0, "a path that names no folder shows nothing, not everything");
|
||||
}
|
||||
|
||||
/// Folders are compared by identity: a path read back may be spelled with the platform's separator.
|
||||
fn same_folder(actual: Option<&str>, expected: &str, why: &str) {
|
||||
let key = lightcraft_catalog::query::folder_key;
|
||||
assert_eq!(actual.map(key), Some(key(expected)), "{why}: {actual:?} vs {expected}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_renamed_folder_stays_the_chosen_one_and_undo_follows_it_back() {
|
||||
let dir = Scratch::new("rename");
|
||||
|
|
@ -200,10 +206,10 @@ fn a_renamed_folder_stays_the_chosen_one_and_undo_follows_it_back() {
|
|||
s.execute("library.filter", &json!({"libraryFolder": trip})).unwrap();
|
||||
assert_eq!(s.visible().len(), 2);
|
||||
s.execute("folder.rename", &json!({"path": trip, "name": "holiday"})).unwrap();
|
||||
assert_eq!(s.filter.library_folder.as_deref(), Some(renamed.as_str()), "the choice follows the folder");
|
||||
same_folder(s.filter.library_folder.as_deref(), &renamed, "the choice follows the folder");
|
||||
assert_eq!(s.visible().len(), 2, "and still shows its photos");
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert_eq!(s.filter.library_folder.as_deref(), Some(trip.as_str()));
|
||||
same_folder(s.filter.library_folder.as_deref(), &trip, "the choice is the same folder");
|
||||
assert_eq!(s.visible().len(), 2);
|
||||
}
|
||||
|
||||
|
|
@ -217,10 +223,10 @@ fn a_moved_folder_keeps_a_chosen_subfolder_chosen() {
|
|||
add(&mut s, &format!("{day1}/b.jpg"));
|
||||
s.execute("library.filter", &json!({"libraryFolder": day1})).unwrap();
|
||||
s.execute("folder.move", &json!({"path": dir.path("trip"), "into": dir.path("archive")})).unwrap();
|
||||
assert_eq!(s.filter.library_folder.as_deref(), Some(moved.as_str()), "the subfolder follows its parent");
|
||||
same_folder(s.filter.library_folder.as_deref(), &moved, "the subfolder follows its parent");
|
||||
assert_eq!(s.visible().len(), 1);
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert_eq!(s.filter.library_folder.as_deref(), Some(day1.as_str()));
|
||||
same_folder(s.filter.library_folder.as_deref(), &day1, "the choice is the same folder");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -229,12 +235,21 @@ fn following_a_folder_reads_both_spellings_the_same_way() {
|
|||
let mut s = Session::new();
|
||||
s.filter.library_folder = Some("/a/x/../b/sub".into());
|
||||
crate::cmd::browse::follow_folder(&mut s, "/a/b", "/a/c");
|
||||
assert_eq!(s.filter.library_folder.as_deref(), Some("/a/c/sub"), "a subfolder stays a subfolder, never widens to its parent");
|
||||
same_folder(s.filter.library_folder.as_deref(), "/a/c/sub", "a subfolder stays a subfolder, never widens to its parent");
|
||||
s.filter.library_folder = Some("/elsewhere/b".into());
|
||||
crate::cmd::browse::follow_folder(&mut s, "/a/b", "/a/c");
|
||||
assert_eq!(s.filter.library_folder.as_deref(), Some("/elsewhere/b"), "other folders are left alone");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn following_a_folder_keeps_the_case_of_the_subfolder_names() {
|
||||
// Windows compares folders in lower case; the name the user sees must not change with it
|
||||
let mut s = Session::new();
|
||||
s.filter.library_folder = Some("/A/Trip/Day1".into());
|
||||
crate::cmd::browse::follow_folder(&mut s, "/A/Trip", "/B");
|
||||
assert_eq!(s.filter.library_folder.as_deref(), Some(std::path::Path::new("/B").join("Day1").to_str().unwrap()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn choosing_a_folder_replaces_the_source_the_way_an_album_does() {
|
||||
let mut s = session();
|
||||
|
|
@ -296,10 +311,10 @@ fn a_renamed_folder_stays_the_one_shown_and_undo_follows_it_back() {
|
|||
add(&mut s, &format!("{day1}/b.jpg"));
|
||||
source_folder(&mut s, &day1);
|
||||
s.execute("folder.rename", &json!({"path": trip, "name": "holiday"})).unwrap();
|
||||
assert_eq!(s.library_folder.as_deref(), Some(format!("{renamed}/day1").as_str()), "the subfolder follows its parent");
|
||||
same_folder(s.library_folder.as_deref(), &format!("{renamed}/day1"), "the subfolder follows its parent");
|
||||
assert_eq!(s.visible().len(), 1);
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert_eq!(s.library_folder.as_deref(), Some(day1.as_str()));
|
||||
same_folder(s.library_folder.as_deref(), &day1, "the choice is the same folder");
|
||||
assert_eq!(s.visible().len(), 1);
|
||||
}
|
||||
|
||||
|
|
@ -308,7 +323,7 @@ fn the_shown_folder_follows_reading_both_spellings_the_same_way() {
|
|||
let mut s = Session::new();
|
||||
s.library_folder = Some("/a/x/../b/sub".into());
|
||||
crate::cmd::browse::follow_folder(&mut s, "/a/b", "/a/c");
|
||||
assert_eq!(s.library_folder.as_deref(), Some("/a/c/sub"));
|
||||
same_folder(s.library_folder.as_deref(), "/a/c/sub", "the shown folder reads both spellings alike");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -1007,3 +1007,58 @@ fn trashed_photo_ids(tag: &str) -> (Session, std::path::PathBuf, [u64; 1], u64)
|
|||
let (s, src, id) = trashed_photo(tag);
|
||||
(s, src, [id], 0)
|
||||
}
|
||||
|
||||
/// Issue #374: launch arguments never turn into a walk of a whole drive or home folder. An empty
|
||||
/// argument is ignored; `.`, `..` and (on Windows) `/` are resolved against the working folder a
|
||||
/// launcher picked, which is often a drive root; a root is replaced by its memory card's DCIM
|
||||
/// folder or refused, and so is the home folder.
|
||||
#[test]
|
||||
fn launch_paths_never_become_a_drive_root_or_the_home_folder() {
|
||||
use crate::import::{LaunchPath, launch_path};
|
||||
let root = if cfg!(windows) { Path::new("C:\\") } else { Path::new("/") };
|
||||
let home = if cfg!(windows) { Path::new("C:\\Users\\me") } else { Path::new("/home/me") };
|
||||
let none = |_: &Path| None;
|
||||
let refused = |r: Option<LaunchPath>| matches!(r, Some(LaunchPath::Refused { .. }));
|
||||
for empty in ["", " "] {
|
||||
assert_eq!(launch_path(empty, Some(root), Some(home), none), None, "{empty:?}");
|
||||
}
|
||||
// the working folder is the drive root: `.` (and `..`) name the whole drive
|
||||
for arg in [".", "..", "./", ".\\..", "/"] {
|
||||
let arg = if cfg!(windows) { arg.to_string() } else { arg.replace('\\', "/") };
|
||||
assert!(refused(launch_path(&arg, Some(root), Some(home), none)), "{arg:?}");
|
||||
}
|
||||
assert!(refused(launch_path("..", Some(&home.join("Pictures")), Some(home), none)), "up to the home folder");
|
||||
assert!(refused(launch_path(&home.to_string_lossy(), None, Some(home), none)));
|
||||
assert!(refused(launch_path(".", None, Some(home), none)), "relative, no working folder");
|
||||
// a memory card's root: its camera folder
|
||||
let dcim = |r: &Path| Some(r.join("DCIM"));
|
||||
assert_eq!(launch_path(".", Some(root), Some(home), dcim), Some(LaunchPath::Import(root.join("DCIM").to_string_lossy().to_string())));
|
||||
// ordinary folders and files resolve to absolute paths
|
||||
let pics = home.join("Pictures");
|
||||
assert_eq!(launch_path("Pictures", Some(home), Some(home), none), Some(LaunchPath::Import(pics.to_string_lossy().to_string())));
|
||||
assert_eq!(
|
||||
launch_path(&pics.join("a.jpg").to_string_lossy(), None, Some(home), none),
|
||||
Some(LaunchPath::Import(pics.join("a.jpg").to_string_lossy().to_string()))
|
||||
);
|
||||
}
|
||||
|
||||
/// Issue #374: a folder import is bounded — a tree deeper or larger than the limits is not walked
|
||||
/// to the end (it stops and says so), and what it found is still returned.
|
||||
#[test]
|
||||
fn expand_stops_at_the_walk_limits() {
|
||||
let d = temp_dir("bounded");
|
||||
let mut deep = d.clone();
|
||||
for i in 0..8 {
|
||||
deep = deep.join(format!("d{i}"));
|
||||
std::fs::create_dir_all(&deep).unwrap();
|
||||
std::fs::write(deep.join(format!("p{i}.jpg")), b"x").unwrap();
|
||||
}
|
||||
let top = [d.to_string_lossy().to_string()];
|
||||
let (all, cut) = crate::import::expand_within(&top, None, crate::walk::Limits::default());
|
||||
assert_eq!((all.len(), cut), (8, false));
|
||||
let (some, cut) = crate::import::expand_within(&top, None, crate::walk::Limits { max_depth: 3, max_entries: 1000 });
|
||||
assert_eq!((some.len(), cut), (2, true), "{some:?}");
|
||||
let (few, cut) = crate::import::expand_within(&top, None, crate::walk::Limits { max_depth: 32, max_entries: 5 });
|
||||
assert!(cut && few.len() < 8, "{few:?}");
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,183 @@
|
|||
//! Bounded folder walks (issue #374): every recursive listing of the disk — import, Local's
|
||||
//! Include Subfolders, Find Missing Photos, preset and LUT folders, MCP — goes through
|
||||
//! [`files_in`], which stops at a depth and a number of directory entries instead of walking a
|
||||
//! whole drive, follows a symbolic link or junction to a folder at most once (no loops), and
|
||||
//! decides what is a folder from the directory listing itself (no extra open per entry, which
|
||||
//! is what makes a large walk slow on Windows).
|
||||
//!
|
||||
//! It never chooses where to walk: callers pass a folder the user named. Nothing in the app walks
|
||||
//! at startup or on the UI thread.
|
||||
|
||||
use std::collections::HashSet;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
/// How far a walk may go.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub struct Limits {
|
||||
/// Folder levels below the starting folder that are listed (its own entries are level 1).
|
||||
pub max_depth: usize,
|
||||
/// Directory entries looked at in all (files and folders, hidden ones included).
|
||||
pub max_entries: usize,
|
||||
}
|
||||
|
||||
impl Default for Limits {
|
||||
fn default() -> Self {
|
||||
Limits { max_depth: MAX_DEPTH, max_entries: MAX_ENTRIES }
|
||||
}
|
||||
}
|
||||
|
||||
/// Folder levels a walk descends at most; photo trees are a handful of levels deep.
|
||||
pub const MAX_DEPTH: usize = 32;
|
||||
/// Directory entries a walk looks at at most: far more than any photo folder holds, far fewer
|
||||
/// than a whole system drive.
|
||||
pub const MAX_ENTRIES: usize = 1_000_000;
|
||||
|
||||
/// What a walk found.
|
||||
#[derive(Debug, Default, PartialEq)]
|
||||
pub struct Walked {
|
||||
/// The files `keep` accepted, in path order, folder by folder (depth first).
|
||||
pub files: Vec<PathBuf>,
|
||||
/// The walk stopped at a [`Limits`] bound: there may be more files.
|
||||
pub truncated: bool,
|
||||
/// Directory entries looked at.
|
||||
pub entries: usize,
|
||||
}
|
||||
|
||||
fn hidden(p: &Path) -> bool {
|
||||
p.file_name().is_some_and(|n| n.to_string_lossy().starts_with('.'))
|
||||
}
|
||||
|
||||
/// The files below folder `root` that `keep` accepts. Hidden entries (names starting with `.`)
|
||||
/// are left out, the folder `skip` (e.g. the library's own) is never entered, and the walk stops
|
||||
/// at `limits` (see [`Walked::truncated`]). Unreadable folders are skipped.
|
||||
pub fn files_in(root: &Path, skip: Option<&Path>, limits: Limits, mut keep: impl FnMut(&Path) -> bool) -> Walked {
|
||||
let mut out = Walked::default();
|
||||
// canonical folders reached through a link, so a link loop is walked at most once
|
||||
let mut linked: HashSet<PathBuf> = HashSet::new();
|
||||
if let Ok(c) = std::fs::canonicalize(root) {
|
||||
linked.insert(c);
|
||||
}
|
||||
// (path, depth of the folder it is in, is a folder); popped in path order
|
||||
let mut stack: Vec<(PathBuf, usize, bool)> = vec![(root.to_path_buf(), 0, true)];
|
||||
while let Some((path, depth, is_dir)) = stack.pop() {
|
||||
if !is_dir {
|
||||
if keep(&path) {
|
||||
out.files.push(path);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if skip.is_some_and(|s| path == s) {
|
||||
continue;
|
||||
}
|
||||
if depth >= limits.max_depth {
|
||||
out.truncated = true;
|
||||
continue;
|
||||
}
|
||||
let Ok(rd) = std::fs::read_dir(&path) else { continue };
|
||||
let mut children: Vec<(PathBuf, bool)> = Vec::new();
|
||||
for e in rd.flatten() {
|
||||
if out.entries >= limits.max_entries {
|
||||
out.truncated = true;
|
||||
break;
|
||||
}
|
||||
out.entries = out.entries.saturating_add(1);
|
||||
let p = e.path();
|
||||
if hidden(&p) {
|
||||
continue;
|
||||
}
|
||||
let Ok(ft) = e.file_type() else { continue };
|
||||
let dir = if ft.is_symlink() {
|
||||
// a link (or a Windows junction): a folder is entered once, a file is a file
|
||||
match std::fs::metadata(&p) {
|
||||
Ok(m) if m.is_dir() => {
|
||||
let Ok(c) = std::fs::canonicalize(&p) else { continue };
|
||||
if !linked.insert(c) {
|
||||
continue;
|
||||
}
|
||||
true
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
} else {
|
||||
ft.is_dir()
|
||||
};
|
||||
children.push((p, dir));
|
||||
}
|
||||
children.sort();
|
||||
let below = depth.saturating_add(1);
|
||||
stack.extend(children.into_iter().rev().map(|(p, d)| (p, below, d)));
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn tmp(tag: &str) -> PathBuf {
|
||||
let d = std::env::temp_dir().join(format!("lc-walk-{tag}-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
std::fs::create_dir_all(&d).unwrap();
|
||||
d
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn files_come_in_path_order_without_hidden_ones_or_the_skipped_folder() {
|
||||
let d = tmp("order");
|
||||
for f in ["b/2.jpg", "a/1.jpg", "a/x/0.jpg", "c.jpg", ".hidden/h.jpg", "a/.h.jpg", "lib/l.jpg", "a/n.txt"] {
|
||||
let p = d.join(f);
|
||||
std::fs::create_dir_all(p.parent().unwrap()).unwrap();
|
||||
std::fs::write(p, b"x").unwrap();
|
||||
}
|
||||
let lib = d.join("lib");
|
||||
let w = files_in(&d, Some(&lib), Limits::default(), |p| p.extension().is_some_and(|e| e == "jpg"));
|
||||
let rel: Vec<String> = w.files.iter().map(|p| p.strip_prefix(&d).unwrap().to_string_lossy().replace('\\', "/")).collect();
|
||||
assert_eq!(rel, ["a/1.jpg", "a/x/0.jpg", "b/2.jpg", "c.jpg"]);
|
||||
assert!(!w.truncated);
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
/// Issue #374: a walk never runs on without end — a tree deeper than the limit, or with more
|
||||
/// entries than the limit, stops and says so.
|
||||
#[test]
|
||||
fn deep_and_wide_trees_stop_at_the_limits() {
|
||||
let d = tmp("bounds");
|
||||
let mut deep = d.join("deep");
|
||||
for i in 0..12 {
|
||||
deep = deep.join(format!("l{i}"));
|
||||
}
|
||||
std::fs::create_dir_all(&deep).unwrap();
|
||||
std::fs::write(deep.join("bottom.jpg"), b"x").unwrap();
|
||||
let wide = d.join("wide");
|
||||
std::fs::create_dir_all(&wide).unwrap();
|
||||
for i in 0..50 {
|
||||
std::fs::write(wide.join(format!("{i:02}.jpg")), b"x").unwrap();
|
||||
}
|
||||
let all = |_: &Path| true;
|
||||
let w = files_in(&d.join("deep"), None, Limits { max_depth: 5, max_entries: 1000 }, all);
|
||||
assert!(w.files.is_empty() && w.truncated, "{w:?}");
|
||||
let w = files_in(&d.join("deep"), None, Limits { max_depth: 13, max_entries: 1000 }, all);
|
||||
assert_eq!((w.files.len(), w.truncated), (1, false), "{w:?}");
|
||||
let w = files_in(&wide, None, Limits { max_depth: 5, max_entries: 20 }, all);
|
||||
assert_eq!((w.files.len(), w.entries, w.truncated), (20, 20, true));
|
||||
let w = files_in(&wide, None, Limits::default(), all);
|
||||
assert_eq!((w.files.len(), w.truncated), (50, false));
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
/// A link back up the tree (a loop) is followed at most once: the walk ends.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn link_loops_end() {
|
||||
let d = tmp("loop");
|
||||
std::fs::create_dir_all(d.join("a/b")).unwrap();
|
||||
std::fs::write(d.join("a/b/p.jpg"), b"x").unwrap();
|
||||
std::os::unix::fs::symlink(&d, d.join("a/b/up")).unwrap();
|
||||
std::os::unix::fs::symlink(d.join("a"), d.join("alias")).unwrap();
|
||||
let w = files_in(&d, None, Limits::default(), |_| true);
|
||||
assert!(!w.truncated, "{w:?}");
|
||||
assert!(w.files.len() <= 2, "{w:?}");
|
||||
assert!(w.files.iter().any(|p| p.ends_with("a/b/p.jpg")), "{w:?}");
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
}
|
||||
|
|
@ -139,31 +139,20 @@ impl Backend for Headless {
|
|||
}
|
||||
}
|
||||
|
||||
/// Expand folders (recursively, sorted) into photo files and make paths absolute.
|
||||
/// Expand folders (recursively, sorted; bounded, see `lightcraft_engine::walk`) into photo files
|
||||
/// and make paths absolute.
|
||||
pub fn expand_paths(paths: &[String]) -> Vec<String> {
|
||||
fn walk(p: &Path, out: &mut Vec<String>) {
|
||||
if p.is_dir() {
|
||||
if let Ok(rd) = std::fs::read_dir(p) {
|
||||
let mut v: Vec<_> = rd.flatten().map(|e| e.path()).collect();
|
||||
v.sort();
|
||||
for c in v {
|
||||
if c.file_name().is_some_and(|n| !n.to_string_lossy().starts_with('.')) {
|
||||
walk(&c, out);
|
||||
}
|
||||
}
|
||||
}
|
||||
} else if p.extension().is_some_and(|e| PHOTO_EXTENSIONS.contains(&e.to_string_lossy().to_lowercase().as_str())) {
|
||||
out.push(std::path::absolute(p).unwrap_or_else(|_| p.to_path_buf()).to_string_lossy().to_string());
|
||||
}
|
||||
}
|
||||
let absolute = |p: &Path| std::path::absolute(p).unwrap_or_else(|_| p.to_path_buf()).to_string_lossy().to_string();
|
||||
let photo = |p: &Path| p.extension().is_some_and(|e| PHOTO_EXTENSIONS.contains(&e.to_string_lossy().to_lowercase().as_str()));
|
||||
let mut out = Vec::new();
|
||||
for p in paths {
|
||||
let path = Path::new(p);
|
||||
if path.is_dir() {
|
||||
walk(path, &mut out);
|
||||
let w = lightcraft_engine::walk::files_in(path, None, lightcraft_engine::walk::Limits::default(), photo);
|
||||
out.extend(w.files.iter().map(|f| absolute(f)));
|
||||
} else {
|
||||
// Explicit files are kept even with unknown extensions (the probe decides).
|
||||
out.push(std::path::absolute(path).unwrap_or_else(|_| path.to_path_buf()).to_string_lossy().to_string());
|
||||
out.push(absolute(path));
|
||||
}
|
||||
}
|
||||
out
|
||||
|
|
|
|||
|
|
@ -56,8 +56,11 @@ fn open(app: &mut LightcraftApp, path: Option<String>) {
|
|||
Ok(serde_json::Value::Null) => {} // folder dialog cancelled
|
||||
Ok(_) => {
|
||||
let files = app.library_problem.take().map(|p| p.pending_import).unwrap_or_default();
|
||||
if !files.is_empty() {
|
||||
let _ = app.run("library.import", json!({"paths": files}));
|
||||
// in the background, like files dropped on the window (issue #374)
|
||||
if !files.is_empty()
|
||||
&& let Err(e) = crate::import::start_paths(app, files)
|
||||
{
|
||||
log::warn!("import: {e}");
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
|
|
@ -144,8 +147,11 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
|
|||
p.dismissed = true;
|
||||
}
|
||||
let files = app.library_problem.as_mut().map(|p| std::mem::take(&mut p.pending_import)).unwrap_or_default();
|
||||
if !files.is_empty() {
|
||||
let _ = app.run("library.import", json!({"paths": files}));
|
||||
// in the background, like files dropped on the window (issue #374)
|
||||
if !files.is_empty()
|
||||
&& let Err(e) = crate::import::start_paths(app, files)
|
||||
{
|
||||
log::warn!("import: {e}");
|
||||
}
|
||||
}
|
||||
Some(Choice::Quit) => app.ui.quit = true,
|
||||
|
|
|
|||
|
|
@ -75,6 +75,8 @@ fn unopenable_library_asks_instead_of_running_a_demo() {
|
|||
assert!(!has(&h, "button:libraryRetry"));
|
||||
assert!(has(&h, "indicator:temporarySession"));
|
||||
assert!(h.app.session.library.is_none());
|
||||
// imported on the import worker (issue #374: never synchronously on the UI thread)
|
||||
h.step_until(T, |h| h.app.import.is_none() && h.app.session.catalog.len() == 1);
|
||||
assert_eq!(h.app.session.catalog.len(), 1, "the command-line photo is imported into the temporary session");
|
||||
if let Some(p) = std::env::var_os("LIGHTCRAFT_TEST_SHOTS") {
|
||||
let r = h.request("ui.screenshot", json!({"path": std::path::Path::new(&p).join("banner.png").to_string_lossy(), "headless": true}), T);
|
||||
|
|
|
|||
|
|
@ -397,7 +397,8 @@ fn a_renamed_folder_keeps_its_place_in_the_tree() {
|
|||
let _cleanup = Cleanup(base.clone());
|
||||
std::fs::create_dir_all(base.join("pics/trip")).unwrap();
|
||||
std::fs::create_dir_all(base.join("pics/home")).unwrap();
|
||||
let b = base.to_string_lossy().to_string();
|
||||
// the folder tree writes paths with forward slashes, and its widget ids carry them
|
||||
let b = base.to_string_lossy().replace('\\', "/");
|
||||
let mut h = folders_app(&[&format!("{b}/pics/trip/a.jpg"), &format!("{b}/pics/home/b.jpg")]);
|
||||
h.app.session.source = lightcraft_engine::LibrarySource::LibraryFolder;
|
||||
h.app.session.library_folder = Some(format!("{b}/pics/trip"));
|
||||
|
|
|
|||
|
|
@ -56,7 +56,7 @@ The additional supplied folder contains 748 RAFs. Profiles were fitted separatel
|
|||
|
||||
For validation, every fifth usable photo of each model in sorted path order was withheld. Profiles trained on the remaining 160 X-H2S and 436 X-T4 photos were tested on 41 and 109 withheld photos. Mean CIE76 ΔE against the camera JPEG was 9.86 → 1.70 with per-file fitting → 1.62 with the pooled X-H2S profile, and 13.64 → 2.43 → 2.34 for X-T4. The profiles improved 108 of 150 files; the largest regression against per-file fitting was 0.60 ΔE, with no previously accepted fit rejected. These are withheld files from the same limited scene collection, not independent coverage of all subjects or illuminants.
|
||||
|
||||
The three original dark X-H2S wrench files have too little colour variation to learn a matrix, but now use the known profile to fit tone: ΔE 8.74 → 2.25, all accepted. The four original X-T4 files and two public X-T4 files remain accepted with the final bundled profile; they regress slightly against their own per-file fits (six-file mean 4.72 → 4.94), while remaining much closer than the neutral fallback (13.47). One public X-T20 reference still fails the gates and keeps the fallback.
|
||||
The three original dark X-H2S wrench files have too little colour variation to learn a matrix, but now use the known profile to fit tone: ΔE 8.74 → 2.25, all accepted. The four original X-T4 files and two public X-T4 files remain accepted with the final bundled profile; they regress slightly against their own per-file fits (six-file mean 4.72 → 4.94), while remaining much closer than the neutral fallback (13.47). The public X-T20 reference fails the gates on all pixels (held-out RMS 0.114) and passes on the second attempt away from edges (0.056, from 0.185 uncorrected), so it is fitted too.
|
||||
|
||||
RAF inputs also work with `lightcraft-cli calibrate`. Tests cover real X-Trans/16-bit Bayer fits, profile use on dull scenes, per-file fitting after profile rejection, monochrome rejection, resolution independence, header/decoded relative WB, old-catalog WB transitions and invalid RAF calibration inputs. A full-size 6240×4160 X-T4 JPEG export also succeeded.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue