diff --git a/crates/ui-egui/src/background.rs b/crates/ui-egui/src/background.rs index 16b87a6576..806434fa1f 100644 --- a/crates/ui-egui/src/background.rs +++ b/crates/ui-egui/src/background.rs @@ -190,6 +190,19 @@ pub fn run_with_progress( then(app, r) } +/// Run `f` on the UI thread once job `id` has finished, right after the job's own result is +/// applied, with that applied result (an error when the work failed). Nothing runs when no job +/// `id` is in progress. +pub fn after(app: &mut VectorcraftApp, id: u64, f: impl FnOnce(&mut VectorcraftApp, &Result) + 'static) { + let Some(job) = app.background.jobs.iter_mut().find(|j| j.id == id) else { return }; + let then = std::mem::replace(&mut job.then, Box::new(|_, r| r)); + job.then = Box::new(move |app, r| { + let r = then(app, r); + f(app, &r); + r + }); +} + /// Run `f` with every save and export it starts written at once, as with the preferences off, so /// its result is the written file or the error (a control client's `wait: true`). The jobs already /// running finish first, so none of them writes over its files afterwards. diff --git a/crates/ui-egui/src/dialogs/package.rs b/crates/ui-egui/src/dialogs/package.rs index 40a8db9cff..2ff2097975 100644 --- a/crates/ui-egui/src/dialogs/package.rs +++ b/crates/ui-egui/src/dialogs/package.rs @@ -1,14 +1,14 @@ //! File → Package…: where the package folder goes, its name, and what it collects (the linked //! files, in a folder of their own or not, relinked or not; the fonts; the report). A document -//! never saved is saved first (Save As); unsaved changes are saved before packaging. The web -//! downloads the package as a zip. +//! never saved is saved first (Save As); unsaved changes are saved before packaging, and when that +//! save fails no package is written. The web downloads the package as a zip. //! //! Fields: `folder` (the location), `name` (the package folder), `copyLinks`, `linksFolder`, //! `relink`, `copyFonts`, `report`. OK runs `file.package` with them, then offers to show the //! package folder (`file.showPackage`). On the desktop with Background Export on, the package is //! collected and written on the background thread ([`crate::background`]) after the dialog closes. -use std::cell::Cell; +use std::cell::{Cell, RefCell}; use std::rc::Rc; use serde_json::{Value, json}; @@ -108,21 +108,35 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result { } else if d.str("folder").trim().is_empty() { return Err("choose a location for the package".into()); } - // The package holds the document as saved. + // The package holds the document as saved: when the save fails, or asks a question and writes + // nothing yet, no package is written. + let mut saving = None; if app.session.active().is_some_and(|st| st.is_dirty()) { let r = io::save(app, vectorcraft_engine::cmd::fileio::SaveMode::Save, &json!({}), false)?; // The save asks first (it would replace a file that loses what opening it left out). if r.get("pending").is_some() { return Ok(r); } + // Background Save: the save is written on the worker, and its result decides the package. + if r["background"] == true { + saving = app.background.jobs.last().map(background::Job::id); + } } // Background Export (Preferences › File Handling): the linked files and fonts are collected // and the package is written from the document as it is now, while editing goes on. The jobs - // started before it (a save started above, an earlier package to the same folder) are written - // first: the worker runs its jobs in order, and a package made at once waits for them. + // started before it (an earlier package to the same folder) are written first: the worker + // runs its jobs in order, and a package made at once waits for them. let enabled = app.session.prefs.background_export; if !enabled { + let outcome: Rc>>> = Rc::default(); + if let Some(id) = saving { + let applied = outcome.clone(); + background::after(app, id, move |_, r| *applied.borrow_mut() = Some(r.clone())); + } background::wait_all(app); + if let Some(r) = outcome.take().filter(|r| !saved(r)) { + return r; + } } let package = app.session.prepare_package(¶ms).map_err(|e| e.to_string()).inspect_err(|e| app.ui.status = e.clone())?; let label = format!("Packaging {}", d.str("name")); @@ -134,7 +148,23 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result { |path: &str, bytes: &[u8]| write(path, bytes).map_err(|e| EngineError::Other(if e.contains(path) { e } else { format!("{path}: {e}") })); package.write(&mut write).map_err(|e| e.to_string()) }; - let r = background::run(app, enabled, label, None, work, move |app, r| packaged(app, r, web, in_background.get()))?; + let start = + move |app: &mut VectorcraftApp| background::run(app, enabled, label, None, work, move |app, r| packaged(app, r, web, in_background.get())); + // Both in the background: the package, collected from the document as it is now, starts once + // the save is written. + if let Some(id) = saving.filter(|_| enabled) { + later.set(true); + app.ui.dialog = None; + background::after(app, id, move |app, r| { + if saved(r) + && let Err(e) = start(app) + { + app.status(e); + } + }); + return Ok(json!({ "background": true })); + } + let r = start(app)?; if r["background"] == true { later.set(true); app.ui.dialog = None; @@ -142,6 +172,12 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result { Ok(r) } +/// Did a save whose applied result is `r` write the document (no error, and no question asked +/// before writing)? +fn saved(r: &Result) -> bool { + r.as_ref().is_ok_and(|r| r.get("pending").is_none()) +} + /// Report the package `r` (`file.package`'s reply): the status bar, the web's download, and the /// offer to show the package folder. `later`: it was written in the background, after the dialog /// closed, and a dialog open now is another one, which stays (the status bar names the folder). diff --git a/crates/ui-egui/src/dialogs/tests_package.rs b/crates/ui-egui/src/dialogs/tests_package.rs index 41253029a2..5c9e899354 100644 --- a/crates/ui-egui/src/dialogs/tests_package.rs +++ b/crates/ui-egui/src/dialogs/tests_package.rs @@ -181,7 +181,7 @@ fn a_dirty_documents_save_is_written_before_its_package() { let (mut app, files) = saved_with_worker(&dir, &gate); let (doc, out) = (dir.path("card.vectorcraft"), dir.path("out")); // With Background Export on, the save and the package both run in the background, and the - // worker writes the save first. + // package starts once the save is written. app.run("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); package_dialog(&mut app, &out); // A thread opens the gate after 10 s at the latest, so a confirm that waits for the save ends. @@ -191,7 +191,7 @@ fn a_dirty_documents_save_is_written_before_its_package() { opener.open(); }); assert_eq!(confirm(&mut app).unwrap()["background"], true); - assert_eq!(app.background.jobs.len(), 2, "the save and the package"); + assert_eq!(app.background.jobs.len(), 1, "the save; the package waits for it"); gate.open(); background::wait_all(&mut app); let paths = written(&files); @@ -216,6 +216,38 @@ fn a_dirty_documents_save_is_written_before_its_package() { assert!(app.ui.status.starts_with("Packaged 3 files in "), "the package's status came last: {}", app.ui.status); } +/// A save before packaging that fails writes no package, with Background Export on or off. +#[test] +fn a_failed_save_before_packaging_writes_no_package() { + let dir = Folder::new("failed-save"); + let (doc, out) = (dir.path("card.vectorcraft"), dir.path("out")); + let root = std::path::Path::new(&out).join("card Folder"); + for background_export in [true, false] { + let mut app = saved(&dir); + let (files, refused) = (Files::default(), doc.clone()); + let recorded = files.clone(); + app.services.write_shared = Some(Arc::new(move |p: &str, b: &[u8]| { + if p == refused { + return Err("Access is denied. (os error 5)".into()); + } + recorded.lock().unwrap().push((p.to_string(), b.to_vec())); + Ok(()) + })); + app.session.execute("prefs.set", &json!({"values": {"backgroundExport": background_export}})).unwrap(); + app.run("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + package_dialog(&mut app, &out); + let r = confirm(&mut app); + background::wait_all(&mut app); + assert!(written(&files).is_empty(), "Background Export {background_export}: {:?}", written(&files)); + assert!(!root.exists(), "Background Export {background_export}: no package folder"); + assert!(app.session.active().unwrap().is_dirty(), "the save failed"); + match r { + Ok(r) => assert!(background_export && r["background"] == true, "{r}"), + Err(e) => assert!(!background_export && e.contains("Access is denied"), "{e}"), + } + } +} + #[test] fn a_package_made_at_once_waits_for_one_in_the_background() { let dir = Folder::new("twice"); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 19f245175d..802ffb4a94 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -762,8 +762,8 @@ the `PATH` elsewhere), and any other file, or a raster image without PhotoCraft Package: File → Package… (`ui.packageDialog`) opens the `package` dialog for a saved document (a document never saved gets a `confirm` dialog whose OK runs `file.saveAs`). Fields: `folder` (the location), `name` (the package folder), -`copyLinks`, `linksFolder`, `relink`, `copyFonts`, `report`. `ui.dialog.confirm` saves unsaved changes, runs -`file.package` and then opens a `confirm` dialog whose OK runs `file.showPackage {folder}`; the web downloads the zip +`copyLinks`, `linksFolder`, `relink`, `copyFonts`, `report`. `ui.dialog.confirm` saves unsaved changes (when that save +fails, no package is written, also with Background Save and Background Export on), runs `file.package` and then opens a `confirm` dialog whose OK runs `file.showPackage {folder}`; the web downloads the zip instead. Document Info's flyout picks a category and Save… runs `docInfo.save {path?, selectionOnly?, category?}` with the category the panel shows (none for All Categories, which writes every category).