Merge pull request #3488 from KaboomFox/export-dialog-guard

Export As: a held Enter doesn't confirm the options; exporters get their file

crates/ui-egui/src/dialogs/export_as.rs

@@ -142,6 +142,23 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result<Value, String> {
     app.ui.dialog = None;
     let path = io::target_path(app, None, f.extensions[0])?;
     let range = if all { "all".to_string() } else { range };
+    let r = options_or_write(app, f, plugin, path, use_artboards, all, range);
+    // The options follow the save panel: the Enter that closed it isn't theirs (as in Save As).
+    super::opened_by_panel(app);
+    r
+}
+
+/// After the save panel picked `path`: the options dialog of format `f` (or of its exporter
+/// plug-in), else the file written.
+fn options_or_write(
+    app: &mut VectorcraftApp,
+    f: &'static Format,
+    plugin: Option<std::sync::Arc<vectorcraft_plugins::Plugin>>,
+    path: String,
+    use_artboards: bool,
+    all: bool,
+    range: String,
+) -> Result<Value, String> {
     match f.id {
         "svg" | "svgz" => {
             svg_options::open(app, svg_options::Mode::Export, Some(&path));

crates/ui-egui/src/dialogs/mod.rs

@@ -426,6 +426,14 @@ pub(crate) fn settle(app: &mut VectorcraftApp) {
 /// until Enter has been let go.
 pub(crate) const AFTER_PANEL: &str = "__afterPanel";
 
+/// Marks the open dialog, if any, as one a save panel opened ([`AFTER_PANEL`]): the Enter that
+/// closed the panel, still held, doesn't confirm it.
+pub(crate) fn opened_by_panel(app: &mut VectorcraftApp) {
+    if let Some(d) = app.ui.dialog.as_mut() {
+        d.fields.insert(AFTER_PANEL.into(), json!(true));
+    }
+}
+
 /// Draws the active dialog and handles shared initial focus, keyboard shortcuts and actions.
 /// Inline actions and footer buttons use the same confirmation and preview rollback paths.
 pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) {

crates/ui-egui/src/dialogs/tests_export.rs

@@ -16,10 +16,15 @@ pub(super) type Written = Rc<RefCell<Vec<(String, Vec<u8>)>>>;
 /// An app with `artboards` 60×40 pt artboards whose save dialog answers `/out/<suggested>` and
 /// whose writer records what it is given.
 pub(super) fn app(artboards: usize) -> (VectorcraftApp, Written) {
+    app_saving(artboards, |p| format!("/out/{}", p.name))
+}
+
+/// [`app`], with a save dialog that answers `pick`.
+pub(super) fn app_saving(artboards: usize, pick: fn(&crate::FilePick) -> String) -> (VectorcraftApp, Written) {
     let written = Written::default();
     let w = written.clone();
     let services = Services {
-        pick_save: Some(Box::new(|p: &crate::FilePick| Some(format!("/out/{}", p.name)))),
+        pick_save: Some(Box::new(move |p: &crate::FilePick| Some(pick(p)))),
         write: Some(Box::new(move |p: &str, b: &[u8]| {
             w.borrow_mut().push((p.to_string(), b.to_vec()));
             Ok(())
@@ -266,3 +271,42 @@ fn export_as_writes_only_the_chosen_artboard_in_every_format() {
         }
     }
 }
+
+/// The Enter that closed the save panel, still held as the format's options open, doesn't write
+/// the file; Enter pressed again after it is let go does. So for every options dialog Export As
+/// opens after its panel, an exporter plug-in's included (as Save As's options).
+#[test]
+fn the_enter_that_closed_the_save_panel_does_not_confirm_the_options() {
+    let params = r#"{"speed":{"type":"number","min":1,"max":100,"default":40}}"#;
+    let module = wat::parse_str(vectorcraft_plugins::wat::returning(
+        &vectorcraft_plugins::wat::exporter("test.ui.held", r#"["tuh"]"#, params),
+        r#"{"dataBase64":"SGk="}"#,
+    ))
+    .unwrap();
+    let enter = |pressed| egui::Event::Key { key: egui::Key::Enter, physical_key: None, pressed, repeat: false, modifiers: Default::default() };
+    let ctx = egui::Context::default();
+    theme::install_fonts(&ctx);
+    let frame = |app: &mut VectorcraftApp, events: Vec<egui::Event>| {
+        let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| show(app, ui.ctx()));
+        out.textures_delta.clear();
+    };
+    // Each format whose options follow the panel (PDF's with Use Artboards: Save PDF).
+    let formats = [("png", false), ("svg", false), ("pdf", true), ("txt", false), ("dxf", false), ("eps", false), ("plugin.test.ui.held", false)];
+    for (format, artboards) in formats {
+        let (mut app, written) = app(1);
+        app.run("plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&module)})).unwrap();
+        app.run("file.exportAs", json!({"format": format})).unwrap();
+        set(&mut app, "useArtboards", json!(artboards));
+        confirm(&mut app).unwrap();
+        let options = kind(&app).map(str::to_string);
+        assert!(options.as_deref().is_some_and(|k| k != "exportAs"), "{format}: its options follow the panel, not {options:?}");
+        frame(&mut app, vec![enter(true)]);
+        assert_eq!(kind(&app).map(str::to_string), options, "{format}: the Enter still held isn't the options'");
+        assert!(written.borrow().is_empty(), "{format}: nothing written");
+        frame(&mut app, vec![enter(false)]);
+        frame(&mut app, vec![enter(true)]);
+        assert!(app.ui.dialog.is_none(), "{format}: Enter pressed again confirms");
+        assert!(!written.borrow().is_empty(), "{format}: and writes");
+    }
+    vectorcraft_plugins::registry::remove("test.ui.held");
+}

crates/ui-egui/src/dialogs/tests_plugin_export.rs

@@ -1,11 +1,12 @@
 //! Exporter plug-ins in File › Export › Export As: listed after the built-in formats, written
 //! whole (Use Artboards is not theirs to apply), and their parameters asked for once the file is
-//! picked. Each test installs exporters of its own: plug-ins are process-wide.
+//! picked. Each test installs exporters of its own and removes them: plug-ins are process-wide, and
+//! a format left installed would show in other tests' Export As.
 
 use serde_json::{Value, json};
 use vectorcraft_plugins::wat::{self as tpl, Echo};
 
-use super::tests_export::{app, frame, kind, names, set};
+use super::tests_export::{app, app_saving, frame, kind, names, set};
 use super::*;
 
 fn install(app: &mut VectorcraftApp, src: &str) {
@@ -13,6 +14,8 @@ fn install(app: &mut VectorcraftApp, src: &str) {
     app.run("plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&wasm)})).unwrap();
 }
 
+/// Export As lists an exporter after the built-in formats and writes its file of the whole
+/// document (Use Artboards greyed out), with the exporter's warnings in the status bar.
 #[test]
 fn export_as_lists_exporters_and_writes_their_file_whole() {
     let (mut app, written) = app(2);
@@ -37,8 +40,10 @@ fn export_as_lists_exporters_and_writes_their_file_whole() {
     assert_eq!(names(&written), ["/out/Untitled-1.tuw"]);
     assert_eq!(written.borrow()[0].1, b"Hi");
     assert!(app.ui.status.contains("a note"), "{}", app.ui.status);
+    app.run("plugin.remove", json!({"id": "test.ui.whole"})).unwrap();
 }
 
+/// An exporter with parameters asks for them once the file is picked, and writes with them.
 #[test]
 fn an_exporter_with_parameters_asks_for_them_once_the_file_is_picked() {
     let (mut app, written) = app(1);
@@ -59,6 +64,7 @@ fn an_exporter_with_parameters_asks_for_them_once_the_file_is_picked() {
     assert_eq!(names(&written), ["/out/Untitled-1.tup"]);
     let sent: Value = serde_json::from_slice(&written.borrow()[0].1).unwrap();
     assert_eq!(sent, json!({"density": 7.0, "_context": {"mode": "export", "extension": "tup", "name": "Untitled-1"}}));
+    app.run("plugin.remove", json!({"id": "test.ui.params"})).unwrap();
 }
 
 /// `CRAFT_UI_PLUGIN_EXPORT_SHOT=<folder>` saves the pictures of the dialogs to that folder: Export As
@@ -113,4 +119,18 @@ fn a_declining_exporter_writes_nothing() {
     assert!(written.borrow().is_empty());
     let said = r.err().unwrap_or_default() + &app.ui.status;
     assert!(said.contains("Nothing to plot."), "{said}");
+    app.run("plugin.remove", json!({"id": "test.ui.declines"})).unwrap();
+}
+
+/// Export options that aren't an object are none: the exporter is still told the file it writes,
+/// by its name and extension (here the second it lists), not the document's name and its first.
+#[test]
+fn an_exporter_is_told_its_file_whatever_the_options_are() {
+    let (mut app, written) = app_saving(1, |_| "/out/plotter-job.tnb".into());
+    install(&mut app, &tpl::echo_exporter(&tpl::exporter("test.ui.nulloptions", r#"["tna","tnb"]"#, "{}"), Echo::Params));
+    crate::io::export(&mut app, Some("plugin.test.ui.nulloptions"), None, &Value::Null).unwrap();
+    assert_eq!(names(&written), ["/out/plotter-job.tnb"]);
+    let sent: Value = serde_json::from_slice(&written.borrow()[0].1).unwrap();
+    assert_eq!(sent, json!({"_context": {"mode": "export", "extension": "tnb", "name": "plotter-job"}}));
+    app.run("plugin.remove", json!({"id": "test.ui.nulloptions"})).unwrap();
 }

crates/ui-egui/src/io.rs

@@ -487,9 +487,7 @@ pub fn save(app: &mut VectorcraftApp, mode: SaveMode, p: &Value, ask_options: bo
     if ask && asks_options(mode, chosen.format) {
         let path = chosen.path.clone().unwrap_or_default();
         let r = ask_format_options(app, mode, chosen.format, &path);
-        if let Some(d) = app.ui.dialog.as_mut() {
-            d.fields.insert(crate::dialogs::AFTER_PANEL.into(), json!(true));
-        }
+        crate::dialogs::opened_by_panel(app);
         return r;
     }
     write_plan(app, chosen)
@@ -712,6 +710,9 @@ pub fn hide_exports(app: &mut VectorcraftApp) -> Value {
 /// document keeps its path. → `{path, warnings, files?}` (`files`: every file when there are
 /// several).
 pub fn export(app: &mut VectorcraftApp, format: Option<&str>, path: Option<String>, params: &Value) -> Result<Value, String> {
+    // Options given as anything but an object are none: the path and format still reach the
+    // encoder (an exporter plug-in is told the file it writes).
+    let params = &if params.is_object() { params.clone() } else { json!({}) };
     let f = fileio::writable_format(format, path.as_deref())?;
     let st = app.session.active().ok_or("no document")?;
     let doc = match fileio::export_source(st, params).map_err(|e| e.to_string())? {