From 99dbc8254df2fccb4a286b295adc79cd8a7186d5 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Tue, 28 Jul 2026 19:23:29 +0300 Subject: [PATCH] fix(plot): parent PDF save dialog Remove the temporary EXPORTPDF fallback now that native save requests are tied to the main window.\n\nCloses #537 --- src/app/automation.rs | 50 ----------------------------- src/app/commands/display.rs | 36 ++------------------- src/app/update/mod.rs | 64 +++++++++++++++++++++---------------- src/io/pdf_export.rs | 19 ++++++++--- src/sys.rs | 14 ++++++++ 5 files changed, 66 insertions(+), 117 deletions(-) diff --git a/src/app/automation.rs b/src/app/automation.rs index ffe9a0b1b..b16905cd1 100644 --- a/src/app/automation.rs +++ b/src/app/automation.rs @@ -821,56 +821,6 @@ mod tests { } } - #[test] - fn exportpdf_with_explicit_path_writes_pdf_without_dialog() { - // EXPORTPDF must export straight to the given file — no save - // dialog — and confirm on the command line. This is the dialog-free - // export path added for #369 (Export to PDF silently did nothing when - // the native save dialog could not open). - let mut app = OpenCADStudio::new_for_test(); - app.automation_op(r#"{"op":"new"}"#); - app.automation_op(r#"{"op":"run","cmd":"LINE 0,0 100,50"}"#); - - let path = std::env::temp_dir().join(format!("ocs_test_369_{}.pdf", std::process::id())); - let _ = std::fs::remove_file(&path); - let start = app.command_line.history.len(); - let _ = app.run_command_line(&format!("EXPORTPDF {}", path.display())); - let out: String = app.command_line.history[start..] - .iter() - .map(|e| e.text.as_str()) - .collect::>() - .join("\n"); - assert!(out.contains("Exported"), "no export confirmation: {out:?}"); - - let bytes = std::fs::read(&path).expect("EXPORTPDF should write the file"); - assert!(bytes.starts_with(b"%PDF"), "output is not a PDF"); - assert!(bytes.len() > 200, "suspiciously small PDF: {}", bytes.len()); - let _ = std::fs::remove_file(&path); - } - - #[test] - fn cancelled_pdf_save_dialog_reports_on_command_line() { - // The save dialog resolving to None (cancelled, or it never opened — - // e.g. a broken XDG portal) must leave a visible message, not silence. - // Regression for #369. - use crate::app::Message; - let mut app = OpenCADStudio::new_for_test(); - app.automation_op(r#"{"op":"new"}"#); - for msg in [Message::PlotExportPath(None), Message::PlotWindowExportPath(None)] { - let start = app.command_line.history.len(); - let _ = app.update(msg); - let out: String = app.command_line.history[start..] - .iter() - .map(|e| e.text.as_str()) - .collect::>() - .join("\n"); - assert!( - out.contains("EXPORTPDF"), - "dialog-None must mention the EXPORTPDF fallback: {out:?}" - ); - } - } - #[test] fn handing_over_an_already_open_drawing_switches_to_its_tab() { // Double-clicking a drawing that is already open should land on the tab diff --git a/src/app/commands/display.rs b/src/app/commands/display.rs index caf11f247..ad9762610 100644 --- a/src/app/commands/display.rs +++ b/src/app/commands/display.rs @@ -448,40 +448,8 @@ impl OpenCADStudio { "PLOT" | "PRINT" => { return Some(Task::done(Message::PlotDialogOpen)); } - // With no argument these open the save dialog; with a path they - // export straight to it. The direct form is also the workaround for - // systems where the native save dialog cannot open at all (broken - // XDG portal / missing zenity on Linux) — there the dialog future - // resolves to None and no export would happen. (#369) - cmd if cmd == "EXPORT" - || cmd == "EXPORTPDF" - || cmd.starts_with("EXPORT ") - || cmd.starts_with("EXPORTPDF ") => - { - // Strip the keyword actually typed — EXPORTPDF first, since - // EXPORT is its prefix (see #295 for the failure mode). - let arg = cmd - .strip_prefix("EXPORTPDF") - .or_else(|| cmd.strip_prefix("EXPORT")) - .unwrap_or("") - .trim() - .trim_matches('"'); - if arg.is_empty() { - return Some(Task::done(Message::PlotExport)); - } - let mut path = std::path::PathBuf::from(arg); - if path.extension().is_none() { - path.set_extension("pdf"); - } - // A bare filename lands next to the drawing when it has a home. - if path.is_relative() { - if let Some(dir) = - self.tabs[i].current_path.as_deref().and_then(|p| p.parent()) - { - path = dir.join(path); - } - } - return Some(self.on_plot_export_path_some(path)); + "EXPORT" | "EXPORTPDF" => { + return Some(Task::done(Message::PlotExport)); } // PLOTSTYLE — load or clear CTB/STB plot style table cmd if cmd == "PLOTSTYLE" || cmd.starts_with("PLOTSTYLE ") => { diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 80e3397de..20eac761b 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -4356,23 +4356,25 @@ impl OpenCADStudio { .and_then(|p: &std::path::Path| p.file_stem()) .map(|s: &std::ffi::OsStr| s.to_string_lossy().into_owned()) .unwrap_or_else(|| "drawing".into()); - Task::perform( - crate::io::pdf_export::pick_pdf_path_owned(stem), - Message::PlotExportPath, - ) - } - // None means the save dialog was cancelled — or never opened at - // all (a broken XDG portal / missing zenity on Linux resolves to - // None too, and rfd only reports that through `log`, which is - // opt-in). Either way say something instead of silently doing - // nothing. (#369) - Message::PlotExportPath(None) => { - self.command_line.push_info( - "PDF export canceled — no file chosen. \ - If no dialog appeared, use EXPORTPDF .", - ); - Task::none() + #[cfg(not(target_arch = "wasm32"))] + { + let Some(window_id) = self.main_window else { + return Task::done(Message::PlotExportPath(None)); + }; + iced::window::run(window_id, move |parent| { + crate::io::pdf_export::pick_pdf_path_owned(stem, parent) + }) + .map(Message::PlotExportPath) + } + #[cfg(target_arch = "wasm32")] + { + Task::perform( + crate::io::pdf_export::pick_pdf_path_owned(stem), + Message::PlotExportPath, + ) + } } + Message::PlotExportPath(None) => Task::none(), Message::PlotExportPath(Some(path)) => self.on_plot_export_path_some(path), Message::PlotFormat(f) => { @@ -4391,19 +4393,25 @@ impl OpenCADStudio { .and_then(|p: &std::path::Path| p.file_stem()) .map(|s: &std::ffi::OsStr| s.to_string_lossy().into_owned()) .unwrap_or_else(|| "drawing".into()); - Task::perform( - crate::io::pdf_export::pick_pdf_path_owned(stem), - Message::PlotWindowExportPath, - ) - } - // Same silent-None trap as PlotExportPath above. (#369) - Message::PlotWindowExportPath(None) => { - self.command_line.push_info( - "PDF export canceled — no file chosen. \ - If no dialog appeared, use EXPORTPDF .", - ); - Task::none() + #[cfg(not(target_arch = "wasm32"))] + { + let Some(window_id) = self.main_window else { + return Task::done(Message::PlotWindowExportPath(None)); + }; + iced::window::run(window_id, move |parent| { + crate::io::pdf_export::pick_pdf_path_owned(stem, parent) + }) + .map(Message::PlotWindowExportPath) + } + #[cfg(target_arch = "wasm32")] + { + Task::perform( + crate::io::pdf_export::pick_pdf_path_owned(stem), + Message::PlotWindowExportPath, + ) + } } + Message::PlotWindowExportPath(None) => Task::none(), Message::PlotWindowExportPath(Some(path)) => self.on_plot_window_export_path_some(path), Message::BackgroundIoFinished(result, reopen_plot) => { diff --git a/src/io/pdf_export.rs b/src/io/pdf_export.rs index a12ecd03c..87d080cc8 100644 --- a/src/io/pdf_export.rs +++ b/src/io/pdf_export.rs @@ -99,17 +99,26 @@ pub fn export_pdf( file.write_all(&bytes).map_err(|e| e.to_string()) } -/// Show a PDF save-file dialog and return the chosen path (or None if cancelled). +/// Show a parented PDF save-file dialog and return the chosen path. +/// +/// The parent comes from `iced::window::run`, keeping the portal request tied +/// to the visible app window on Wayland instead of silently resolving to +/// `None` on desktops that reject a parentless save dialog (#537). #[cfg(not(target_arch = "wasm32"))] -pub async fn pick_pdf_path_owned(stem: String) -> Option { - crate::sys::file_dialog() +pub fn pick_pdf_path_owned( + stem: String, + parent: &dyn iced::window::Window, +) -> Option { + let path = crate::sys::blocking_file_dialog() + .set_parent(parent) .set_title("Export as PDF") .set_file_name(&format!("{stem}.pdf")) .add_filter("PDF Files", &["pdf"]) .add_filter("All Files", &["*"]) .save_file() - .await - .map(|h| crate::sys::handle_path(&h)) + ?; + crate::config::remember_dialog_dir(&path); + Some(path) } // ── PDF builder ─────────────────────────────────────────────────────────── diff --git a/src/sys.rs b/src/sys.rs index 358f0c695..2fba7d603 100644 --- a/src/sys.rs +++ b/src/sys.rs @@ -131,6 +131,20 @@ pub fn file_dialog() -> rfd::AsyncFileDialog { } } +/// Blocking native file dialog seeded with the last-used directory. +/// +/// Use this through `iced::window::run` when the picker needs the main +/// window's raw handle. Portal-based desktops can otherwise reject or lose a +/// parentless save request without ever showing a dialog (#537). +#[cfg(not(target_arch = "wasm32"))] +pub fn blocking_file_dialog() -> rfd::FileDialog { + let dlg = rfd::FileDialog::new(); + match crate::config::last_dialog_dir() { + Some(dir) => dlg.set_directory(dir), + None => dlg, + } +} + /// Web: no filesystem paths, nothing to remember. #[cfg(target_arch = "wasm32")] pub fn file_dialog() -> rfd::AsyncFileDialog {