From 6b0274120b641ee066a4f66b586edd2d8737b398 Mon Sep 17 00:00:00 2001 From: DWAA1660 Date: Wed, 7 Oct 2026 16:08:01 -0600 Subject: [PATCH 1/2] Export: say so when there's no export folder instead of failing with Access denied On Windows HOME is usually unset, so the default export folder came out empty and photos were written by bare filename into the working directory, failing with 'Access is denied. (os error 5)'. - Default export folder uses %USERPROFILE%\Pictures\LightCraft Exports on Windows (matching the default library location). - A blank Folder in the Export dialog, or no folder resolvable at all, now fails with 'You need to specify an export location!' before any write. - The Export dialog stays open on an error (like Import) and shows it; the menu and keyboard export paths show it too instead of doing nothing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PNY4WojPYorWa6rQLxSYHa --- crates/ui-egui/src/control.rs | 32 ++++++++++++++++++++++------ crates/ui-egui/src/menubar.rs | 15 ++++++++++++- crates/ui-egui/src/panels/dialogs.rs | 5 +++-- crates/ui-egui/src/shortcuts.rs | 14 +++++++++--- 4 files changed, 54 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 1b9d22bb..5eb30041 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -334,9 +334,16 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest } } -/// Default export folder: `~/Pictures/LightCraft Exports` (falls back to the working directory). +/// Shown when an export has nowhere to go (no folder typed or chosen, and no home folder to default to). +pub const NO_EXPORT_FOLDER: &str = "You need to specify an export location!"; + +/// Default export folder: `~/Pictures/LightCraft Exports` (`%USERPROFILE%\Pictures\LightCraft Exports` +/// on Windows, where `HOME` usually isn't set). Empty when no home folder is known. pub fn default_export_dir() -> String { - std::env::var("HOME").map(|h| format!("{h}/Pictures/LightCraft Exports")).unwrap_or_default() + let home = if cfg!(windows) { std::env::var_os("USERPROFILE").or_else(|| std::env::var_os("HOME")) } else { std::env::var_os("HOME") }; + home.filter(|h| !h.is_empty()) + .map(|h| std::path::PathBuf::from(h).join("Pictures").join("LightCraft Exports").to_string_lossy().into_owned()) + .unwrap_or_default() } /// Export the selected photos (UI command `app.export`). Params: see @@ -365,10 +372,23 @@ pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result d.trim().to_string(), + None => { + let last_dir = + app.session.last_export.as_ref().and_then(|l| l.get("dir")).and_then(Value::as_str).map(str::trim).filter(|d| !d.is_empty()); + last_dir.map(str::to_string).unwrap_or_else(default_export_dir) + } + }; + if dir.is_empty() && exact.is_none() { + return Err(NO_EXPORT_FOLDER.into()); + } + let to = Destination { dir: dir.clone(), exact }; let background = p.get("background").and_then(Value::as_bool).unwrap_or(false) && app.services.write_shared.is_some(); let out = if background { let items = lightcraft_engine::export::prepare_batch(&mut app.session, &ids, &opts)?; diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index 7e8054f2..0329415f 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -735,7 +735,13 @@ pub fn show_in_window(app: &mut LightcraftApp, ui: &mut egui::Ui, max_width: f32 }); } if let Some((id, params)) = clicked { - let _ = run_item(app, &id, params); + let r = run_item(app, &id, params); + // an export that can't start (e.g. no folder) says why instead of doing nothing + if let Err(e) = r + && matches!(id.as_str(), "app.export" | "app.exportPrevious") + { + app.toast(ui.ctx(), e); + } } ui.cursor().left() - start } @@ -945,6 +951,13 @@ mod tests { // remembered (expanded) for Export with Previous, folder included let last = app.session.last_export.clone().unwrap(); assert_eq!((last["format"].as_str(), last["width"].as_u64(), last.get("preset")), (Some("png"), Some(40), None)); + // a blank folder (the Export dialog's Folder field cleared) is refused with a clear message + // instead of writing into the working directory + let n = w.len(); + drop(w); + let r = run_item(&mut app, "app.export", json!({"preset": "Tiny PNG", "dir": " "})); + assert_eq!(r.unwrap_err(), crate::control::NO_EXPORT_FOLDER); + assert_eq!(written.lock().unwrap().len(), n, "nothing written without a folder"); } #[test] diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index bc23383b..c2c3d267 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -824,8 +824,9 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { } if confirm { match confirm_dialog(app, &dlg) { - // the import review stays open on an error (e.g. an unusable folder template) - Err(e) if matches!(dlg, Dialog::Import { .. }) => app.toast(ctx, e), + // the import review stays open on an error (e.g. an unusable folder template), and so + // does Export (e.g. no folder chosen) so the choices aren't lost + Err(e) if matches!(dlg, Dialog::Import { .. } | Dialog::Export { .. }) => app.toast(ctx, e), // the SAM 3 dialog stays open to show the download (or why it can't start) Err(e) if matches!(dlg, Dialog::SamModel { .. }) => { if let Dialog::SamModel { error, .. } = &mut dlg { diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index c5d2ce16..f76757e0 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -158,8 +158,11 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { o.remove("advance"); } cull(app, id, params, advance); - } else { - let _ = app.run(id, params); + } else if let Err(e) = app.run(id, params) + && matches!(id, "app.export" | "app.exportPrevious") + { + // an export that can't start (e.g. no folder) says why instead of doing nothing + app.toast(ctx, e); } } for f in fire { @@ -255,7 +258,12 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { continue; } } - let _ = app.run(&f, json!({})); + // an export that can't start (e.g. no folder) says why instead of doing nothing + if let Err(e) = app.run(&f, json!({})) + && matches!(f.as_str(), "app.export" | "app.exportPrevious") + { + app.toast(ctx, e); + } match f.as_str() { "photo.pick" => app.toast(ctx, "Flagged as Pick"), "photo.reject" => app.toast(ctx, "Flagged as Reject"), From ce165dfa4a5a80e52b107b6cb7b7990288b469f7 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Wed, 7 Oct 2026 19:56:00 -0400 Subject: [PATCH 2/2] Export: calmer wording for the missing-folder error Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/control.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 5eb30041..1a6abdde 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -335,7 +335,7 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest } /// Shown when an export has nowhere to go (no folder typed or chosen, and no home folder to default to). -pub const NO_EXPORT_FOLDER: &str = "You need to specify an export location!"; +pub const NO_EXPORT_FOLDER: &str = "Choose an export folder first."; /// Default export folder: `~/Pictures/LightCraft Exports` (`%USERPROFILE%\Pictures\LightCraft Exports` /// on Windows, where `HOME` usually isn't set). Empty when no home folder is known.