From d2af4c0b345277258bcc9eb61d79e33ead54cb8c Mon Sep 17 00:00:00 2001 From: Kevin Griffin <117586586+KevinGriffin-new@users.noreply.github.com> Date: Tue, 14 Jul 2026 13:38:16 -0700 Subject: [PATCH] fix(plot): never fail silently on PDF export; add EXPORTPDF (#369) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Export to PDF could end in complete silence: the flow hands off to an async rfd save dialog, and when that future resolves to None the app mapped it straight to Task::none(). None does not only mean the user cancelled — on Linux the XDG portal SaveFile call can fail outright (rfd then tries a zenity fallback, which may not be installed) and the future still just resolves to None. rfd reports those failures only through the `log` crate, and logging is opt-in (--log / RUST_LOG), so the reporter saw exactly what #369 describes: no dialog, no error, no console output, no file. - EXPORT / EXPORTPDF now accept an optional path argument that exports straight to the file with no dialog — both the workaround for systems where the native save dialog cannot open and a headless-testable entry point. A relative path lands next to the drawing; a missing extension gets .pdf. - The dialog-None arms (PlotExportPath / PlotWindowExportPath) now push a command-line message pointing at EXPORTPDF instead of doing nothing. - The success message prints the full output path, not just the file name, so users can find the PDF. Verified end to end on the real binary via --serve: LINE + CIRCLE then EXPORTPDF writes a valid PDF with the expected geometry. Both new tests fail on the previous code. Fixes #369 Co-Authored-By: Claude Fable 5 --- COMMANDS.md | 4 +-- src/app/automation.rs | 50 +++++++++++++++++++++++++++++++++++++ src/app/commands/display.rs | 36 ++++++++++++++++++++++++-- src/app/update/file.rs | 15 +++++++---- src/app/update/mod.rs | 22 ++++++++++++++-- 5 files changed, 116 insertions(+), 11 deletions(-) diff --git a/COMMANDS.md b/COMMANDS.md index c234d1e8a..ea6709db7 100644 --- a/COMMANDS.md +++ b/COMMANDS.md @@ -303,8 +303,8 @@ Status of every standard CAD command in Open CAD Studio: | `QSAVE` | — | Quick save | ✅ | | `SAVEALL` | — | Save all open drawings | ✅ | | `PLOT` | — | Print / plot | ✅ | -| `EXPORT` | — | Export | ✅ | -| `EXPORTPDF` | — | Export to PDF | ✅ | +| `EXPORT [path]` | — | Export to PDF (save dialog, or straight to `path`) | ✅ | +| `EXPORTPDF [path]` | — | Export to PDF (save dialog, or straight to `path`) | ✅ | | `PAGESETUP` | — | Page setup | ✅ | | `PLOTSTYLE` | — | Plot style | ✅ | | `PURGE` | PU | Purge unused items | ✅ | diff --git a/src/app/automation.rs b/src/app/automation.rs index b1dcaefbe..423b3d8e8 100644 --- a/src/app/automation.rs +++ b/src/app/automation.rs @@ -541,6 +541,56 @@ 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 save_then_open_round_trips() { let mut app = OpenCADStudio::new_for_test(); diff --git a/src/app/commands/display.rs b/src/app/commands/display.rs index fbc6429f1..d31cb2368 100644 --- a/src/app/commands/display.rs +++ b/src/app/commands/display.rs @@ -365,8 +365,40 @@ impl OpenCADStudio { "PLOT" | "PRINT" => { return Some(Task::done(Message::PlotDialogOpen)); } - "EXPORT" | "EXPORTPDF" => { - return Some(Task::done(Message::PlotExport)); + // 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)); } // PLOTSTYLE — load or clear CTB/STB plot style table cmd if cmd == "PLOTSTYLE" || cmd.starts_with("PLOTSTYLE ") => { diff --git a/src/app/update/file.rs b/src/app/update/file.rs index 0bba88e4a..9c354b16e 100644 --- a/src/app/update/file.rs +++ b/src/app/update/file.rs @@ -1119,7 +1119,10 @@ pub(super) fn on_open_file(&mut self) -> Task { } } - pub(super) fn on_plot_export_path_some(&mut self, path: std::path::PathBuf) -> Task { + pub(in crate::app) fn on_plot_export_path_some( + &mut self, + path: std::path::PathBuf, + ) -> Task { let i = self.active_tab; let scene = &self.tabs[i].scene; let wires = scene.entity_wires(); @@ -1222,10 +1225,12 @@ pub(super) fn on_open_file(&mut self) -> Task { &path, self.active_plot_style.as_ref(), ) { - Ok(()) => self.command_line.push_info(&format!( - "Exported: {}", - path.file_name().unwrap_or_default().to_string_lossy() - )), + // Full path, not just the file name — when the export was + // driven by EXPORTPDF the user needs to see where + // the file actually landed. (#369) + Ok(()) => self + .command_line + .push_info(&format!("Exported: {}", path.display())), Err(e) => self.command_line.push_error(&format!("Export failed: {e}")), } Task::none() diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 101388ea0..03cd8e000 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -3535,7 +3535,18 @@ impl OpenCADStudio { Message::PlotExportPath, ) } - Message::PlotExportPath(None) => Task::none(), + // 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() + } Message::PlotExportPath(Some(path)) => self.on_plot_export_path_some(path), Message::PlotFormat(f) => { @@ -3559,7 +3570,14 @@ impl OpenCADStudio { Message::PlotWindowExportPath, ) } - Message::PlotWindowExportPath(None) => Task::none(), + // 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() + } Message::PlotWindowExportPath(Some(path)) => self.on_plot_window_export_path_some(path), // ── Print to system printer ───────────────────────────────────────