From 3f520a3420ec075ccc45047f31cfadf7c9065188 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 03:03:33 +0200 Subject: [PATCH] File: Save a Copy asks where to save a bare name instead of writing it to the working directory (fixes #1212) --- crates/ui-egui/src/dialogs.rs | 12 ++++++++- crates/ui-egui/tests/file_menu.rs | 43 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..9b42a70ff 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1170,7 +1170,17 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { d.values.insert("path".into(), json!(path.clone())); ok_cancel(ui, &mut confirm, &mut open); if confirm { - let target = if path.is_empty() { app.services.pick_save.as_ref().and_then(|f| f(&format!("Copy.{fmt}"))) } else { Some(path) }; + // A bare name has no folder: written as typed it would land in the + // process's working directory (`/` from the Finder, the install folder on + // Windows), never where the user looks. The picker chooses the folder. + let pick = app.services.pick_save.as_ref(); + let target = if path.is_empty() { + pick.and_then(|f| f(&format!("Copy.{fmt}"))) + } else if let Some(f) = pick.filter(|_| std::path::Path::new(&path).is_relative()) { + f(&if path.ends_with(&format!(".{fmt}")) { path } else { format!("{path}.{fmt}") }) + } else { + Some(path) + }; if let Some(t) = target { let id = match fmt.as_str() { "csv" => "file.exportCsv", diff --git a/crates/ui-egui/tests/file_menu.rs b/crates/ui-egui/tests/file_menu.rs index 691eef367..c1b531e96 100644 --- a/crates/ui-egui/tests/file_menu.rs +++ b/crates/ui-egui/tests/file_menu.rs @@ -217,3 +217,46 @@ fn a_cancelled_export_pdf_picker_writes_nothing() { assert!(h.state().message.is_none()); assert!(h.state_mut().session.take_ui_requests().is_empty()); } + +/// Share › Export as Web Page (and the native File menu's Save as Web Page) asks for a name in its +/// own dialog. A bare name has no folder: written as typed it lands in the process's working +/// directory, where nobody looks for it (#1212). The save picker chooses the folder instead. +#[test] +fn save_a_copy_with_a_bare_name_asks_where_to_save_it() { + let dir = std::env::temp_dir().join(format!("gridcraft-save-copy-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let asked = Arc::new(Mutex::new(Vec::::new())); + let (capture, target) = (asked.clone(), dir.clone()); + let mut h = harness(Services { + pick_save: Some(Box::new(move |suggested| { + capture.lock().unwrap().push(suggested.to_string()); + Some(target.join("Chosen.html").to_string_lossy().into_owned()) + })), + ..Default::default() + }); + h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "42"})).unwrap(); + let save_copy = |h: &mut egui_kittest::Harness<'static, SheetApp>, path: &str| { + h.state_mut().open_dialog("saveCopy", serde_json::json!({"format": "html"})); + h.run_steps(3); + h.state_mut().dialog.as_mut().unwrap().values.insert("path".into(), serde_json::json!(path)); + h.run_steps(2); + h.get_by_label(" OK ").click(); + h.run_steps(3); + }; + let name = format!("gridcraft-save-copy-{}", std::process::id()); + save_copy(&mut h, &name); + let stray = std::path::PathBuf::from(format!("{name}.html")); + let written_in_cwd = stray.exists(); + let _ = std::fs::remove_file(&stray); + assert!(!written_in_cwd, "a bare name is not written to the working directory"); + assert_eq!(*asked.lock().unwrap(), vec![format!("{name}.html")], "the picker is offered the typed name"); + assert!(std::fs::read_to_string(dir.join("Chosen.html")).unwrap().contains("42"), "the page is written where the picker says"); + assert!(h.state().message.is_none()); + // A full path skips the picker; a write that fails is reported, not swallowed. + save_copy(&mut h, &dir.join("Named.html").to_string_lossy()); + assert!(dir.join("Named.html").exists()); + save_copy(&mut h, &dir.join("missing").join("Lost.html").to_string_lossy()); + assert!(h.state().message.is_some(), "a failed write is reported"); + assert_eq!(asked.lock().unwrap().len(), 1); + let _ = std::fs::remove_dir_all(&dir); +}