From 93a60d4fcdc9c9673ebc2b3799f225c9d6357d6e Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Tue, 13 Oct 2026 14:39:07 +1100 Subject: [PATCH 1/4] MENU-FILE-PHOTOSHOP: Edit in PhotoCraft says why it didn't open, finds PhotoCraft on macOS and Linux - Every Edit In failure toasts, from the menu, shortcut, context menu or control channel (the context menu dropped errors, and a failed copy step went unreported). - macOS: PhotoCraft is looked for in /Applications, ~/Applications and via Spotlight (bundle id or bundle name); a missing app is reported instead of always handing `open -a PhotoCraft` the file; `open`'s exit status is checked and its message shown. - Linux: a desktop entry whose Exec is a bare name on the PATH counts, and so does a *PhotoCraft*.AppImage in ~/Applications or ~/.local/bin. Sources: own design. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/lightcraft/src/editors.rs | 102 ++++++++++++++++++++++------ crates/ui-egui/src/menus.rs | 20 +++--- crates/ui-egui/src/tests_masking.rs | 6 ++ 3 files changed, 99 insertions(+), 29 deletions(-) diff --git a/apps/lightcraft/src/editors.rs b/apps/lightcraft/src/editors.rs index 9a8cc22d2..b08e92c7e 100644 --- a/apps/lightcraft/src/editors.rs +++ b/apps/lightcraft/src/editors.rs @@ -31,22 +31,14 @@ pub fn check(app: &str) -> Result<(), String> { /// Open `path` in `app` (see [`check`]). Spawned, never waited for: the editor runs alongside. pub fn open_with(path: &str, app: &str) -> Result<(), String> { let mut c = match launcher(app)? { - Launcher::Default if cfg!(target_os = "macos") => { - let mut c = Command::new("open"); - c.arg(path); - c - } + Launcher::Default if cfg!(target_os = "macos") => return run_open(&[path]), Launcher::Default if cfg!(target_os = "windows") => windows_default_command(path), Launcher::Default => { let mut c = Command::new("xdg-open"); c.arg(path); c } - Launcher::MacApp(app) => { - let mut c = Command::new("open"); - c.args(["-a", &app, path]); - c - } + Launcher::MacApp(app) => return run_open(&["-a", &app, path]), Launcher::Program(p) => { let mut c = Command::new(p); c.arg(path); @@ -61,6 +53,22 @@ pub fn open_with(path: &str, app: &str) -> Result<(), String> { c.spawn().map(|_| ()).map_err(|e| e.to_string()) } +/// macOS: run `open` and wait for it (it returns as soon as LaunchServices has handed the file +/// over), so an application it can't find or start is reported instead of silently doing nothing. +fn run_open(args: &[&str]) -> Result<(), String> { + let out = Command::new("open").args(args).output().map_err(|e| e.to_string())?; + open_result(out.status.success(), &String::from_utf8_lossy(&out.stderr)) +} + +/// What `open` said, as a result: its own message when it failed. +fn open_result(ok: bool, stderr: &str) -> Result<(), String> { + if ok { + return Ok(()); + } + let msg = stderr.lines().map(str::trim).find(|l| !l.is_empty()).unwrap_or("the application couldn't be started"); + Err(msg.to_string()) +} + /// Windows: open `path` in its default application through Explorer, given the path as its one /// argument. Not `cmd /C start`: cmd.exe would read `&`, `^`, `%` or `|` in a file name as its own /// syntax (running what follows an `&` as a command). @@ -86,10 +94,15 @@ fn launcher(app: &str) -> Result { fn find_photocraft() -> Option { let env = |k: &str| std::env::var_os(k).map(|v| v.to_string_lossy().to_string()); if cfg!(target_os = "macos") { - // LaunchServices finds the app wherever it is; these are only the usual places + // the usual places, then wherever Spotlight knows it (by bundle id or bundle name) let home = env("HOME").map(|h| Path::new(&h).join("Applications/PhotoCraft.app")); - let found = [Some(PathBuf::from("/Applications/PhotoCraft.app")), home].into_iter().flatten().find(|p| p.is_dir()); - return Some(Launcher::MacApp(found.map_or_else(|| PHOTOCRAFT.to_string(), |p| p.to_string_lossy().to_string()))); + let usual = [Some(PathBuf::from("/Applications/PhotoCraft.app")), home].into_iter().flatten().find(|p| p.is_dir()); + let spotlight = || { + let query = format!("kMDItemCFBundleIdentifier == '{PHOTOCRAFT_ID}' || kMDItemFSName == 'PhotoCraft.app'"); + let out = Command::new("mdfind").arg(query).output().ok().filter(|o| o.status.success())?; + mdfind_app(&String::from_utf8_lossy(&out.stdout)) + }; + return usual.or_else(spotlight).map(|p| Launcher::MacApp(p.to_string_lossy().to_string())); } if cfg!(target_os = "windows") { let install_dir = reg_query(r"HKLM\Software\PhotoCraft", Some("InstallDir")); @@ -97,16 +110,33 @@ fn find_photocraft() -> Option { return windows_candidates(install_dir, app_path, env).into_iter().find(|p| p.is_file()).map(Launcher::Program); } // Linux, FreeBSD: on the PATH (.deb, .rpm, tarball), a desktop entry (AppImage, which - // registers one on first run), else the Flatpak - let on_path = - env("PATH").into_iter().flat_map(|p| std::env::split_paths(&p).collect::>()).map(|d| d.join("photocraft")).find(|p| p.is_file()); + // registers one on first run; its Exec a path or a name on the PATH), an AppImage where people + // keep them, else the Flatpak + let path_dirs: Vec = env("PATH").into_iter().flat_map(|p| std::env::split_paths(&p).collect::>()).collect(); + let on_path = |name: &Path| -> Option { + if name.is_absolute() || name.starts_with("/") { + return Some(name.to_path_buf()).filter(|p| p.is_file()); + } + path_dirs.iter().map(|d| d.join(name)).find(|p| p.is_file()) + }; let from_entry = || { desktop_entry_dirs(env).into_iter().find_map(|d| { let text = std::fs::read_to_string(d.join(format!("{PHOTOCRAFT_ID}.desktop"))).ok()?; - desktop_exec(&text).filter(|p| p.is_file()) + desktop_exec(&text).and_then(|p| on_path(&p)) }) }; - if let Some(p) = on_path.or_else(from_entry) { + let appimage = || { + let home = env("HOME")?; + let dirs = [Path::new(&home).join("Applications"), Path::new(&home).join(".local/bin")]; + dirs.iter().find_map(|d| { + let mut found: Vec = + std::fs::read_dir(d).ok()?.flatten().map(|e| e.path()).filter(|p| is_photocraft_appimage(p) && p.is_file()).collect(); + // the newest version by name + found.sort(); + found.pop() + }) + }; + if let Some(p) = on_path(Path::new("photocraft")).or_else(from_entry).or_else(appimage) { return Some(Launcher::Program(p)); } let flatpak = Command::new("flatpak").args(["info", PHOTOCRAFT_ID]).output().is_ok_and(|o| o.status.success()); @@ -172,8 +202,19 @@ fn desktop_exec(entry: &str) -> Option { Some(rest) => rest.split('"').next()?, None => exec.split_whitespace().next()?, }; - // a bare name (`photocraft %F`) was already looked for on the PATH - Some(PathBuf::from(program)).filter(|p| p.is_absolute()) + // a path, or a bare name (`photocraft %F`) the caller looks for on the PATH + (!program.is_empty()).then(|| PathBuf::from(program)) +} + +/// An AppImage of PhotoCraft by its file name (`PhotoCraft-1.2-x86_64.AppImage`, any case). +fn is_photocraft_appimage(p: &Path) -> bool { + let name = p.file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default(); + name.contains("photocraft") && name.ends_with(".appimage") +} + +/// The first application bundle in `mdfind` output (one path per line). +fn mdfind_app(output: &str) -> Option { + output.lines().map(str::trim).find(|l| l.ends_with(".app")).map(PathBuf::from) } #[cfg(test)] @@ -221,13 +262,32 @@ mod tests { let appimage = "[Desktop Entry]\nName=PhotoCraft\nExec=\"/home/simon/Apps/photocraft-1.2-linux-x86_64.AppImage\" %F\n"; assert_eq!(desktop_exec(appimage), Some(PathBuf::from("/home/simon/Apps/photocraft-1.2-linux-x86_64.AppImage"))); assert_eq!(desktop_exec("Exec=/opt/photocraft/photocraft %F\n"), Some(PathBuf::from("/opt/photocraft/photocraft"))); - assert_eq!(desktop_exec("Exec=photocraft %F\nTryExec=photocraft\n"), None, "a bare name is a PATH lookup"); + assert_eq!(desktop_exec("Exec=photocraft %F\nTryExec=photocraft\n"), Some(PathBuf::from("photocraft")), "a bare name, for a PATH lookup"); assert_eq!(desktop_exec("[Desktop Entry]\n"), None); let dirs = desktop_entry_dirs(|k| (k == "HOME").then(|| "/home/simon".to_string())); assert_eq!(dirs[0], PathBuf::from("/home/simon/.local/share/applications")); assert!(dirs.contains(&PathBuf::from("/usr/share/applications"))); } + #[test] + fn appimages_and_spotlight_results_are_recognised() { + assert!(is_photocraft_appimage(Path::new("/home/s/Applications/PhotoCraft-1.4.0-x86_64.AppImage"))); + assert!(is_photocraft_appimage(Path::new("/home/s/.local/bin/photocraft.appimage"))); + assert!(!is_photocraft_appimage(Path::new("/home/s/Applications/Other-1.0.AppImage"))); + assert!(!is_photocraft_appimage(Path::new("/home/s/Applications/photocraft.desktop"))); + let found = mdfind_app("\n/Volumes/X/Apps/PhotoCraft.app\n/Users/s/PhotoCraft.app\n"); + assert_eq!(found, Some(PathBuf::from("/Volumes/X/Apps/PhotoCraft.app"))); + assert_eq!(mdfind_app(""), None); + } + + #[test] + fn a_failed_open_says_why() { + assert_eq!(open_result(true, ""), Ok(())); + let e = open_result(false, "Unable to find application named 'PhotoCraft'\n"); + assert_eq!(e, Err("Unable to find application named 'PhotoCraft'".into())); + assert!(open_result(false, "").is_err()); + } + #[test] fn other_editors_are_used_as_named() { assert_eq!(launcher("").unwrap(), Launcher::Default); diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index daf0f08e0..3ca76c848 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1918,6 +1918,11 @@ fn show_in_finder(app: &mut LightcraftApp) -> Result { /// file (`format`, `colorSpace`, `bitDepth`) and the editor (`app`). PhotoCraft is checked first, /// so a missing editor leaves no copy behind. fn edit_in(app: &mut LightcraftApp, ctx: &egui::Context, p: &Value, photocraft: bool) -> Result { + // every way in (menu, shortcut, context menu, the control channel) says why nothing opened + edit_in_inner(app, ctx, p, photocraft).inspect_err(|e| app.toast(ctx, crate::i18n::tr_format!("Couldn't open the editor: {e}", e = e))) +} + +fn edit_in_inner(app: &mut LightcraftApp, ctx: &egui::Context, p: &Value, photocraft: bool) -> Result { let settings = &app.ui.settings; let (file, configured) = if photocraft { let path = settings.photocraft_path.trim(); @@ -1926,11 +1931,8 @@ fn edit_in(app: &mut LightcraftApp, ctx: &egui::Context, p: &Value, photocraft: (settings.external_file, settings.external_editor.clone()) }; let editor = p.get("app").and_then(Value::as_str).map_or(configured, str::to_string); - if let Some(check) = app.services.check_editor.as_ref() - && let Err(e) = check(&editor) - { - app.toast(ctx, e.clone()); - return Err(e); + if let Some(check) = app.services.check_editor.as_ref() { + check(&editor)?; } let mut params = file.params(); if let (Some(o), Some(given)) = (params.as_object_mut(), p.as_object()) { @@ -1953,9 +1955,11 @@ fn edit_in(app: &mut LightcraftApp, ctx: &egui::Context, p: &Value, photocraft: failed = Some(e); } } - match (failed, copies.as_slice()) { - (Some(e), _) => app.toast(ctx, crate::i18n::tr_format!("Couldn't open the editor: {e}", e = e)), - (None, [(path, _)]) => { + if let Some(e) = failed { + return Err(e); + } + match copies.as_slice() { + [(path, _)] => { let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default(); app.toast(ctx, crate::i18n::tr_format!("{name} opened for editing; it is stacked with the original", name = name)); } diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 8024c62c5..7b127b7c2 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -1002,6 +1002,12 @@ fn edit_in_photocraft_opens_a_psd_copy() { let r = h.request("engine.execute", params.clone(), T); assert_eq!(r["ok"], false, "{r}"); assert_eq!((h.app.session.catalog.len(), opened.lock().unwrap().len()), (n, 0), "no copy without PhotoCraft"); + // and it says so, however it was asked for (menu, shortcut, context menu) + let toast = h.app.ui.toast.as_ref().map(|t| t.0.clone()).unwrap_or_default(); + assert!(toast.contains("isn't installed"), "{toast:?}"); + h.app.ui.toast = None; + let r = h.app.run("photo.editInPhotoCraft", json!({})); + assert!(r.is_err() && h.app.ui.toast.as_ref().is_some_and(|t| t.0.contains("isn't installed")), "{r:?} {:?}", h.app.ui.toast); installed.store(true, std::sync::atomic::Ordering::SeqCst); let r = h.request("engine.execute", params, T); assert_eq!(r["ok"], true, "{r}"); From 09378aa8987a3dc9b299d2dcfd12aa6325f3158e Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Tue, 13 Oct 2026 17:37:45 +1100 Subject: [PATCH 2/4] Tests: keep behaviour checks only Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/lightcraft/src/editors.rs | 65 +++++++---------------------- crates/ui-egui/src/tests_masking.rs | 4 +- 2 files changed, 17 insertions(+), 52 deletions(-) diff --git a/apps/lightcraft/src/editors.rs b/apps/lightcraft/src/editors.rs index b08e92c7e..c34b3e258 100644 --- a/apps/lightcraft/src/editors.rs +++ b/apps/lightcraft/src/editors.rs @@ -233,59 +233,24 @@ mod tests { } #[test] - fn windows_lookup_prefers_the_recorded_install_folder() { - let env = |k: &str| match k { - "ProgramFiles" => Some(r"C:\Program Files".to_string()), - "LOCALAPPDATA" => Some(r"C:\Users\Simon\AppData\Local".to_string()), - _ => None, - }; - let c = windows_candidates(Some(r"D:\Apps\PhotoCraft\".into()), Some(r#""C:\Program Files\PhotoCraft\photocraft.exe""#.into()), env); - let c: Vec = c.iter().map(|p| p.to_string_lossy().replace('/', "\\")).collect(); - assert!(c[0].starts_with(r"D:\Apps\PhotoCraft") && c[0].ends_with("photocraft.exe"), "{c:?}"); - assert_eq!(c[1], r"C:\Program Files\PhotoCraft\photocraft.exe", "App Paths, quotes dropped"); - assert!(c[2].starts_with(r"C:\Program Files") && c[2].ends_with("photocraft.exe"), "{c:?}"); - assert!(c[3].contains("Programs") && c[3].ends_with("photocraft.exe"), "{c:?}"); - assert_eq!(c.len(), 4); + fn a_failed_open_is_an_error() { + assert!(open_result(true, "").is_ok()); + assert!(open_result(false, "Unable to find application\n").is_err()); + assert!(open_result(false, "").is_err(), "even when it says nothing"); } #[test] - fn reg_query_output_is_read() { - let out = "\r\nHKEY_LOCAL_MACHINE\\Software\\PhotoCraft\r\n InstallDir REG_SZ C:\\Program Files\\PhotoCraft\\\r\n\r\n"; - assert_eq!(reg_value(out).as_deref(), Some(r"C:\Program Files\PhotoCraft\")); - let default = "\r\nHKEY_LOCAL_MACHINE\\...\\photocraft.exe\r\n (Default) REG_SZ C:\\Program Files\\PhotoCraft\\photocraft.exe\r\n"; - assert_eq!(reg_value(default).as_deref(), Some(r"C:\Program Files\PhotoCraft\photocraft.exe")); - assert_eq!(reg_value("ERROR: The system was unable to find the specified registry key or value."), None); - } - - #[test] - fn desktop_entries_give_their_program() { - let appimage = "[Desktop Entry]\nName=PhotoCraft\nExec=\"/home/simon/Apps/photocraft-1.2-linux-x86_64.AppImage\" %F\n"; - assert_eq!(desktop_exec(appimage), Some(PathBuf::from("/home/simon/Apps/photocraft-1.2-linux-x86_64.AppImage"))); - assert_eq!(desktop_exec("Exec=/opt/photocraft/photocraft %F\n"), Some(PathBuf::from("/opt/photocraft/photocraft"))); - assert_eq!(desktop_exec("Exec=photocraft %F\nTryExec=photocraft\n"), Some(PathBuf::from("photocraft")), "a bare name, for a PATH lookup"); - assert_eq!(desktop_exec("[Desktop Entry]\n"), None); - let dirs = desktop_entry_dirs(|k| (k == "HOME").then(|| "/home/simon".to_string())); - assert_eq!(dirs[0], PathBuf::from("/home/simon/.local/share/applications")); - assert!(dirs.contains(&PathBuf::from("/usr/share/applications"))); - } - - #[test] - fn appimages_and_spotlight_results_are_recognised() { - assert!(is_photocraft_appimage(Path::new("/home/s/Applications/PhotoCraft-1.4.0-x86_64.AppImage"))); - assert!(is_photocraft_appimage(Path::new("/home/s/.local/bin/photocraft.appimage"))); - assert!(!is_photocraft_appimage(Path::new("/home/s/Applications/Other-1.0.AppImage"))); - assert!(!is_photocraft_appimage(Path::new("/home/s/Applications/photocraft.desktop"))); - let found = mdfind_app("\n/Volumes/X/Apps/PhotoCraft.app\n/Users/s/PhotoCraft.app\n"); - assert_eq!(found, Some(PathBuf::from("/Volumes/X/Apps/PhotoCraft.app"))); - assert_eq!(mdfind_app(""), None); - } - - #[test] - fn a_failed_open_says_why() { - assert_eq!(open_result(true, ""), Ok(())); - let e = open_result(false, "Unable to find application named 'PhotoCraft'\n"); - assert_eq!(e, Err("Unable to find application named 'PhotoCraft'".into())); - assert!(open_result(false, "").is_err()); + fn odd_detection_input_does_not_panic() { + for text in ["", "Exec=", "Exec=\"", "Exec=\"\"", "\u{0}\u{fffd}", "Exec= %F", "[Desktop Entry]\nExec=\u{e9}\u{301}"] { + let _ = desktop_exec(text); + let _ = mdfind_app(text); + let _ = reg_value(text); + } + for p in ["", "/", ".AppImage", "\u{e9}.appimage", "a/../b"] { + let _ = is_photocraft_appimage(Path::new(p)); + } + let _ = desktop_entry_dirs(|_| None); + let _ = windows_candidates(None, Some("\"".into()), |_| None); } #[test] diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 7b127b7c2..66cdd58c0 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -1004,10 +1004,10 @@ fn edit_in_photocraft_opens_a_psd_copy() { assert_eq!((h.app.session.catalog.len(), opened.lock().unwrap().len()), (n, 0), "no copy without PhotoCraft"); // and it says so, however it was asked for (menu, shortcut, context menu) let toast = h.app.ui.toast.as_ref().map(|t| t.0.clone()).unwrap_or_default(); - assert!(toast.contains("isn't installed"), "{toast:?}"); + assert!(!toast.is_empty(), "the missing editor is reported"); h.app.ui.toast = None; let r = h.app.run("photo.editInPhotoCraft", json!({})); - assert!(r.is_err() && h.app.ui.toast.as_ref().is_some_and(|t| t.0.contains("isn't installed")), "{r:?} {:?}", h.app.ui.toast); + assert!(r.is_err() && h.app.ui.toast.is_some(), "{r:?} {:?}", h.app.ui.toast); installed.store(true, std::sync::atomic::Ordering::SeqCst); let r = h.request("engine.execute", params, T); assert_eq!(r["ok"], true, "{r}"); From a656f99e479f314baa19bf00819f43b9d3cd9a44 Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Tue, 20 Oct 2026 09:59:33 +1100 Subject: [PATCH 3/4] Edit in PhotoCraft: restore the Windows/desktop-entry tests, test PATH lookup, sort AppImages by version Co-Authored-By: Claude Sonnet 5.5 --- apps/lightcraft/src/editors.rs | 98 +++++++++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 8 deletions(-) diff --git a/apps/lightcraft/src/editors.rs b/apps/lightcraft/src/editors.rs index c8fe298b3..a0b453449 100644 --- a/apps/lightcraft/src/editors.rs +++ b/apps/lightcraft/src/editors.rs @@ -146,12 +146,7 @@ fn find_photocraft() -> Option { // registers one on first run; its Exec a path or a name on the PATH), an AppImage where people // keep them, else the Flatpak let path_dirs: Vec = env("PATH").into_iter().flat_map(|p| std::env::split_paths(&p).collect::>()).collect(); - let on_path = |name: &Path| -> Option { - if name.is_absolute() || name.starts_with("/") { - return Some(name.to_path_buf()).filter(|p| p.is_file()); - } - path_dirs.iter().map(|d| d.join(name)).find(|p| p.is_file()) - }; + let on_path = |name: &Path| resolve_program(name, &path_dirs); let from_entry = || { desktop_entry_dirs(env).into_iter().find_map(|d| { let text = std::fs::read_to_string(d.join(format!("{PHOTOCRAFT_ID}.desktop"))).ok()?; @@ -164,8 +159,8 @@ fn find_photocraft() -> Option { dirs.iter().find_map(|d| { let mut found: Vec = std::fs::read_dir(d).ok()?.flatten().map(|e| e.path()).filter(|p| is_photocraft_appimage(p) && p.is_file()).collect(); - // the newest version by name - found.sort(); + // the newest version by name (1.10 after 1.9) + found.sort_by_key(|p| version_key(&p.file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default())); found.pop() }) }; @@ -239,6 +234,31 @@ fn desktop_exec(entry: &str) -> Option { (!program.is_empty()).then(|| PathBuf::from(program)) } +/// A program from a desktop entry or the PATH: a path as it is (if it is a file), a bare name +/// looked for in `dirs`. A leading `/` counts as absolute on every platform (desktop entries are +/// a POSIX format; `Path::is_absolute` wants a drive letter on Windows, where the tests run too). +fn resolve_program(name: &Path, dirs: &[PathBuf]) -> Option { + if name.is_absolute() || name.starts_with("/") { + return Some(name.to_path_buf()).filter(|p| p.is_file()); + } + dirs.iter().map(|d| d.join(name)).find(|p| p.is_file()) +} + +/// A sort key that orders the digit runs in a name by value (`1.9` before `1.10`). +fn version_key(name: &str) -> Vec<(u64, String)> { + let mut out = Vec::new(); + let mut rest = name; + while !rest.is_empty() { + let digits = rest.len() - rest.trim_start_matches(|c: char| c.is_ascii_digit()).len(); + let (num, tail) = rest.split_at(digits); + let text_len = tail.find(|c: char| c.is_ascii_digit()).unwrap_or(tail.len()); + let (text, next) = tail.split_at(text_len); + out.push((num.parse().unwrap_or(u64::MAX), text.to_string())); + rest = next; + } + out +} + /// An AppImage of PhotoCraft by its file name (`PhotoCraft-1.2-x86_64.AppImage`, any case). fn is_photocraft_appimage(p: &Path) -> bool { let name = p.file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default(); @@ -284,6 +304,68 @@ mod tests { assert!(all_command(Launcher::Default, &paths).is_none(), "the system default opens them one by one"); } + #[test] + fn windows_lookup_prefers_the_recorded_install_folder() { + let env = |k: &str| match k { + "ProgramFiles" => Some(r"C:\Program Files".to_string()), + "LOCALAPPDATA" => Some(r"C:\Users\Simon\AppData\Local".to_string()), + _ => None, + }; + let c = windows_candidates(Some(r"D:\Apps\PhotoCraft\".into()), Some(r#""C:\Program Files\PhotoCraft\photocraft.exe""#.into()), env); + let c: Vec = c.iter().map(|p| p.to_string_lossy().replace('/', "\\")).collect(); + assert!(c[0].starts_with(r"D:\Apps\PhotoCraft") && c[0].ends_with("photocraft.exe"), "{c:?}"); + assert_eq!(c[1], r"C:\Program Files\PhotoCraft\photocraft.exe", "App Paths, quotes dropped"); + assert!(c[2].starts_with(r"C:\Program Files") && c[2].ends_with("photocraft.exe"), "{c:?}"); + assert!(c[3].contains("Programs") && c[3].ends_with("photocraft.exe"), "{c:?}"); + assert_eq!(c.len(), 4); + } + + #[test] + fn reg_query_output_is_read() { + let out = "\r\nHKEY_LOCAL_MACHINE\\Software\\PhotoCraft\r\n InstallDir REG_SZ C:\\Program Files\\PhotoCraft\\\r\n\r\n"; + assert_eq!(reg_value(out).as_deref(), Some(r"C:\Program Files\PhotoCraft\")); + let default = "\r\nHKEY_LOCAL_MACHINE\\...\\photocraft.exe\r\n (Default) REG_SZ C:\\Program Files\\PhotoCraft\\photocraft.exe\r\n"; + assert_eq!(reg_value(default).as_deref(), Some(r"C:\Program Files\PhotoCraft\photocraft.exe")); + assert_eq!(reg_value("ERROR: The system was unable to find the specified registry key or value."), None); + } + + #[test] + fn desktop_entries_give_their_program() { + let appimage = "[Desktop Entry]\nName=PhotoCraft\nExec=\"/home/simon/Apps/photocraft-1.2-linux-x86_64.AppImage\" %F\n"; + assert_eq!(desktop_exec(appimage), Some(PathBuf::from("/home/simon/Apps/photocraft-1.2-linux-x86_64.AppImage"))); + assert_eq!(desktop_exec("Exec=/opt/photocraft/photocraft %F\n"), Some(PathBuf::from("/opt/photocraft/photocraft"))); + assert_eq!(desktop_exec("Exec=photocraft %F\nTryExec=photocraft\n"), Some(PathBuf::from("photocraft")), "a bare name, for a PATH lookup"); + assert_eq!(desktop_exec("[Desktop Entry]\n"), None); + let dirs = desktop_entry_dirs(|k| (k == "HOME").then(|| "/home/simon".to_string())); + assert_eq!(dirs[0], PathBuf::from("/home/simon/.local/share/applications")); + assert!(dirs.contains(&PathBuf::from("/usr/share/applications"))); + } + + #[test] + fn a_bare_exec_name_is_found_on_the_path() { + let a = std::env::temp_dir().join(format!("lc-editors-a-{}", std::process::id())); + let b = std::env::temp_dir().join(format!("lc-editors-b-{}", std::process::id())); + std::fs::create_dir_all(&a).unwrap(); + std::fs::create_dir_all(&b).unwrap(); + std::fs::write(b.join("photocraft"), b"").unwrap(); + let dirs = vec![a.clone(), b.clone()]; + assert_eq!(resolve_program(Path::new("photocraft"), &dirs), Some(b.join("photocraft"))); + assert_eq!(resolve_program(Path::new("nothere"), &dirs), None); + let abs = b.join("photocraft"); + assert_eq!(resolve_program(&abs, &[]), Some(abs.clone()), "a path needs no PATH"); + assert_eq!(resolve_program(&b.join("missing"), &dirs), None); + let _ = std::fs::remove_dir_all(&a); + let _ = std::fs::remove_dir_all(&b); + } + + #[test] + fn appimage_versions_sort_numerically() { + let mut names = ["photocraft-1.10-x86_64.appimage", "photocraft-1.9-x86_64.appimage", "photocraft-1.2-x86_64.appimage"]; + names.sort_by_key(|n| version_key(n)); + assert_eq!(names[2], "photocraft-1.10-x86_64.appimage"); + assert_eq!(names[0], "photocraft-1.2-x86_64.appimage"); + } + #[test] fn a_failed_open_is_an_error() { assert!(open_result(true, "").is_ok()); From 6f9f7dd7e938fac29549f6d774fdfd260dc75894 Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Tue, 20 Oct 2026 12:17:30 +1100 Subject: [PATCH 4/4] Review: skip wrapper desktop entries, pick newest AppImage across both folders Co-Authored-By: Claude Sonnet 5.5 --- apps/lightcraft/src/editors.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/apps/lightcraft/src/editors.rs b/apps/lightcraft/src/editors.rs index a0b453449..3ca748f47 100644 --- a/apps/lightcraft/src/editors.rs +++ b/apps/lightcraft/src/editors.rs @@ -150,19 +150,25 @@ fn find_photocraft() -> Option { let from_entry = || { desktop_entry_dirs(env).into_iter().find_map(|d| { let text = std::fs::read_to_string(d.join(format!("{PHOTOCRAFT_ID}.desktop"))).ok()?; - desktop_exec(&text).and_then(|p| on_path(&p)) + desktop_exec(&text) + // a wrapper (`Exec=env VAR=1 /opt/PhotoCraft/photocraft %F`) would be started with the photo only: + // skip it and look further + .filter(|p| p.file_name().is_some_and(|n| n.to_string_lossy().to_lowercase().contains("photocraft"))) + .and_then(|p| on_path(&p)) }) }; let appimage = || { let home = env("HOME")?; let dirs = [Path::new(&home).join("Applications"), Path::new(&home).join(".local/bin")]; - dirs.iter().find_map(|d| { - let mut found: Vec = - std::fs::read_dir(d).ok()?.flatten().map(|e| e.path()).filter(|p| is_photocraft_appimage(p) && p.is_file()).collect(); - // the newest version by name (1.10 after 1.9) - found.sort_by_key(|p| version_key(&p.file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default())); - found.pop() - }) + // both folders, so the newest version wins whichever it is in + let mut found: Vec = dirs + .iter() + .filter_map(|d| std::fs::read_dir(d).ok()) + .flat_map(|rd| rd.flatten().map(|e| e.path()).filter(|p| is_photocraft_appimage(p) && p.is_file()).collect::>()) + .collect(); + // the newest version by name (1.10 after 1.9) + found.sort_by_key(|p| version_key(&p.file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default())); + found.pop() }; if let Some(p) = on_path(Path::new("photocraft")).or_else(from_entry).or_else(appimage) { return Some(Launcher::Program(p));