From 582416f4f2671365ae7c28b74433afcdb67aa820 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 22:00:47 -0800 Subject: [PATCH 1/2] EPS Save: every file of a multi-artboard save is checked against the file read from A save with Use Artboards writes a file per artboard beside the path. Each of them is now checked against the file the document was read from, before the first file is written, so a sibling cannot replace that file without acknowledgeLoss. The interface checks the files beside the path the same way. --- crates/engine/src/cmd/fileio.rs | 2 +- crates/engine/src/cmd/fileio/load.rs | 29 ++++++++++++++++++++++++-- crates/engine/src/cmd/fileio/save.rs | 10 ++++++++- crates/engine/src/cmd/recovery.rs | 1 + crates/engine/src/tests/save.rs | 24 +++++++++++++++++++++ crates/ui-egui/src/io/save.rs | 6 ++++-- crates/ui-egui/src/tests/save.rs | 31 ++++++++++++++++++++++++++++ 7 files changed, 97 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/fileio.rs b/crates/engine/src/cmd/fileio.rs index 6478cc81b8..25718292ae 100644 --- a/crates/engine/src/cmd/fileio.rs +++ b/crates/engine/src/cmd/fileio.rs @@ -69,7 +69,6 @@ pub(crate) use encode::{anti_alias, background, with_single_artboard}; pub(crate) use export::with_artboard_fills; pub(crate) use export::{export_crop, isolated}; pub use export::{export_doc, export_source}; -pub(crate) use load::check_not_lossy_overwrite; use load::err; pub(crate) use load::import_svg; pub(crate) use load::is_placeholder; @@ -80,6 +79,7 @@ pub use load::{ Loaded, RasterImage, ReadFile, detect, file_name, load, load_with, open_bytes, open_bytes_with, open_read, open_template, raster_image, read_for_open, texture_image, }; +pub(crate) use load::{check_files_not_lossy_overwrite, check_not_lossy_overwrite, lossy_source}; pub use load::{losses_summary, overwrite_losses}; pub use mesh3d::{DEFAULT_FORMAT_3D, FORMATS_3D, format_3d_label}; pub(crate) use mesh3d::{mesh_file_names, mesh_files}; diff --git a/crates/engine/src/cmd/fileio/load.rs b/crates/engine/src/cmd/fileio/load.rs index fde1ed273f..cc3de2a529 100644 --- a/crates/engine/src/cmd/fileio/load.rs +++ b/crates/engine/src/cmd/fileio/load.rs @@ -993,13 +993,38 @@ fn is_loss(note: &str) -> bool { /// Another name keeps the original file, with all it has. pub(crate) fn check_not_lossy_overwrite(st: &crate::DocState, target: &str, p: &Value, cmd: &str) -> Result<()> { let Some(losses) = overwrite_losses(st, target, p) else { return Ok(()) }; + Err(lossy_overwrite(target, losses, cmd)) +} + +/// The refusal to write `target`, the file the document was read from, when reading it left out `losses`. +fn lossy_overwrite(target: &str, losses: &[String], cmd: &str) -> EngineError { let what = losses_summary(losses); - Err(bad( + bad( cmd, format!( "{target} is the file this document was read from, and reading it left things out ({what}): writing over it would lose them for good. Write another file instead, or pass acknowledgeLoss: true to replace it anyway" ), - )) + ) +} + +/// The file the document was read from and what reading it left out, when a write to that file +/// needs `acknowledgeLoss` and `p` does not have it ([`overwrite_losses`]). +pub(crate) fn lossy_source(st: &crate::DocState, p: &Value) -> Option<(String, Vec)> { + let source = st.imported_from.as_deref().filter(|_| !st.import_losses.is_empty())?; + (!bool_or(p, "acknowledgeLoss", false)).then(|| (source.to_string(), st.import_losses.clone())) +} + +/// None of the files `targets` (every file a save writes, before any is written) is the +/// `source` file that reading left `losses` out of ([`lossy_source`]). +pub(crate) fn check_files_not_lossy_overwrite<'a>( + source: &(String, Vec), + targets: impl IntoIterator, + cmd: &str, +) -> Result<()> { + match targets.into_iter().find(|t| same_file(&source.0, t)) { + Some(target) => Err(lossy_overwrite(target, &source.1, cmd)), + None => Ok(()), + } } /// What writing the file `target` for document `st` would lose for good: the notes of what reading diff --git a/crates/engine/src/cmd/fileio/save.rs b/crates/engine/src/cmd/fileio/save.rs index 301a7c79fe..a0cb4c75fa 100644 --- a/crates/engine/src/cmd/fileio/save.rs +++ b/crates/engine/src/cmd/fileio/save.rs @@ -99,6 +99,10 @@ pub struct SavePlan { /// `preview`, `precision`) given beside the path, and each key of `options`, that the format /// doesn't read, naming the file the save writes. pub notes: Vec, + /// The file the document was read from and what reading it left out, when the save needs + /// `acknowledgeLoss` to write over it and was not given it: every file the save writes is + /// checked against it before the first is written. + pub lossy_source: Option<(String, Vec)>, } impl SavePlan { @@ -308,7 +312,8 @@ pub fn save_plan(s: &Session, mode: SaveMode, p: &Value) -> Result { .chain(nested.filter(|k| !reads_option(format, k)).map(|k| format!("options.{k}"))) .map(|k| format!("`{k}` was not used: {} files don't read it", written.label)) .collect(); - Ok(SavePlan { mode, path, format, options, name, folder, modified, notes }) + let lossy_source = super::lossy_source(st, p); + Ok(SavePlan { mode, path, format, options, name, folder, modified, notes, lossy_source }) } /// The native save options, which a save also takes beside the path. @@ -899,6 +904,9 @@ impl SaveJob { return Ok(out); }; let files = enc.named(doc, path); + if let Some(source) = &plan.lossy_source { + super::check_files_not_lossy_overwrite(source, files.iter().map(|(p, _)| p.as_str()), plan.mode.command())?; + } for (p, bytes) in &files { write(p, bytes)?; } diff --git a/crates/engine/src/cmd/recovery.rs b/crates/engine/src/cmd/recovery.rs index b75066ec6a..d7561e4e10 100644 --- a/crates/engine/src/cmd/recovery.rs +++ b/crates/engine/src/cmd/recovery.rs @@ -905,6 +905,7 @@ pub fn jobs(s: &mut Session, store: &Arc) -> Result<(Vec = std::fs::read_dir(&d) + .unwrap() + .map(|e| e.unwrap().path()) + .filter(|p| p.file_name().is_some_and(|n| n.to_string_lossy().starts_with("Art_"))) + .collect(); + siblings.sort(); + assert_eq!(siblings.len(), 2, "{siblings:?}"); + let sibling = siblings[0].to_string_lossy().to_string(); + let saved = std::fs::read(&sibling).unwrap(); + std::fs::remove_file(&path).unwrap(); + let st = app.session.doc_mut().unwrap(); + st.imported_from = Some(sibling.clone()); + st.import_losses = vec!["hidden type".into()]; + rect(&mut app); + let _ = app.run("file.saveAs", json!({"path": path, "options": options})); + crate::background::wait_all(&mut app); + assert_eq!(std::fs::read(&sibling).unwrap(), saved); + assert!(!std::path::Path::new(&path).exists(), "no file of the set is written when one is refused"); + let _ = std::fs::remove_dir_all(d); +} + #[test] fn native_saves_stream_into_the_file() { let d = dir("stream"); From 95d90511e89163b70d27f9ec835211698040be21 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 06:26:44 -0800 Subject: [PATCH 2/2] Save: files written beside the path are checked against the file read from again The merge of main kept main's calls to write_plan, which plan the save with acknowledgeLoss set, so a save in the app no longer refused to replace the file the document was read from when that file was one written beside the path (an EPS artboard file). The save is planned again without it before it is written, as before the merge. The files a save writes beside the path still ask before replacing a file that is there, except those the document's last save wrote. A new test covers an EPS save with Use Artboards meeting artboard files from another document: it asks, Cancel writes nothing, OK writes every file, and the next Save does not ask. --- crates/ui-egui/src/io/save.rs | 5 +- crates/ui-egui/src/tests/save.rs | 78 +++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/io/save.rs b/crates/ui-egui/src/io/save.rs index d9d475e76f..def7cedf12 100644 --- a/crates/ui-egui/src/io/save.rs +++ b/crates/ui-egui/src/io/save.rs @@ -264,7 +264,9 @@ pub fn save(app: &mut VectorcraftApp, mode: SaveMode, p: &Value, ask_options: bo if ask && matches!(mode, SaveMode::SaveAs | SaveMode::Copy) && matches!(first.format.id, "svg" | "svgz") { return ask_format_options(app, mode, first.format, &path); } - return write_plan(app, first, (save_command(mode), with_path(p, &path))); + // Only `path` was asked about: the files written beside it are checked against the file + // the document was read from when the save writes them. + return write_plan(app, plan(app, mode, p)?, (save_command(mode), with_path(p, &path))); } if is_web(app) && ask_options { return Ok(open_options(app, mode.command(), first.format, &first.name, true)); @@ -291,6 +293,7 @@ pub fn save(app: &mut VectorcraftApp, mode: SaveMode, p: &Value, ask_options: bo crate::dialogs::opened_by_panel(app); return r; } + let chosen = plan(app, mode, &q)?; write_plan(app, chosen, (save_command(mode), q)) } diff --git a/crates/ui-egui/src/tests/save.rs b/crates/ui-egui/src/tests/save.rs index 7511bdb069..458511043e 100644 --- a/crates/ui-egui/src/tests/save.rs +++ b/crates/ui-egui/src/tests/save.rs @@ -303,19 +303,15 @@ fn a_save_does_not_replace_the_file_read_from_through_a_file_written_beside_the_ let d = dir("lossy-beside"); let path = d.join("Art.eps").to_string_lossy().to_string(); let mut app = app(); + checks_files_there(&mut app); app.run("file.new", json!({"width": 200, "height": 100, "artboards": 2})).unwrap(); rect(&mut app); let options = json!({"useArtboards": true, "previewFormat": "none", "thumbnails": false}); let _ = app.run("file.saveAs", json!({"path": path, "options": options})); crate::background::wait_all(&mut app); - let mut siblings: Vec<_> = std::fs::read_dir(&d) - .unwrap() - .map(|e| e.unwrap().path()) - .filter(|p| p.file_name().is_some_and(|n| n.to_string_lossy().starts_with("Art_"))) - .collect(); - siblings.sort(); + let siblings = artboard_files(&d); assert_eq!(siblings.len(), 2, "{siblings:?}"); - let sibling = siblings[0].to_string_lossy().to_string(); + let sibling = siblings[0].clone(); let saved = std::fs::read(&sibling).unwrap(); std::fs::remove_file(&path).unwrap(); let st = app.session.doc_mut().unwrap(); @@ -324,11 +320,79 @@ fn a_save_does_not_replace_the_file_read_from_through_a_file_written_beside_the_ rect(&mut app); let _ = app.run("file.saveAs", json!({"path": path, "options": options})); crate::background::wait_all(&mut app); + // The document's last save wrote the files beside the path, so the save does not ask whether + // to replace them: it is refused. + assert!(app.ui.dialog.as_ref().is_none_or(|d| d.kind != dialogs::confirm::REPLACE_FILES)); assert_eq!(std::fs::read(&sibling).unwrap(), saved); assert!(!std::path::Path::new(&path).exists(), "no file of the set is written when one is refused"); let _ = std::fs::remove_dir_all(d); } +/// The app checks for files that are there, as the desktop app does. +fn checks_files_there(app: &mut VectorcraftApp) { + app.services.exists = Some(std::sync::Arc::new(|p: &str| std::path::Path::new(p).exists())); +} + +/// The files in `d` that an EPS save of `Art.eps` with Use Artboards wrote beside it, sorted. +fn artboard_files(d: &std::path::Path) -> Vec { + let mut files: Vec = std::fs::read_dir(d) + .unwrap() + .map(|e| e.unwrap().path()) + .filter(|p| p.file_name().is_some_and(|n| n.to_string_lossy().starts_with("Art_"))) + .map(|p| p.to_string_lossy().to_string()) + .collect(); + files.sort(); + files +} + +#[test] +fn an_eps_save_with_use_artboards_asks_before_replacing_an_artboard_file_that_is_there() { + let d = dir("eps-artboards-there"); + let path = d.join("Art.eps").to_string_lossy().to_string(); + let options = json!({"useArtboards": true, "previewFormat": "none", "thumbnails": false}); + let save_as = |app: &mut VectorcraftApp| { + let _ = app.run("file.saveAs", json!({"path": path, "options": options})); + crate::background::wait_all(app); + }; + // Another document's save leaves files with the names of the artboard files. + let mut other = app(); + other.run("file.new", json!({"width": 200, "height": 100, "artboards": 2})).unwrap(); + save_as(&mut other); + let there = artboard_files(&d); + assert_eq!(there.len(), 2, "{there:?}"); + std::fs::remove_file(&path).unwrap(); + for f in &there { + std::fs::write(f, b"other").unwrap(); + } + let mut app = app(); + checks_files_there(&mut app); + app.run("file.new", json!({"width": 200, "height": 100, "artboards": 2})).unwrap(); + rect(&mut app); + save_as(&mut app); + let dialog = app.ui.dialog.as_ref().unwrap(); + assert_eq!(dialog.kind, dialogs::confirm::REPLACE_FILES); + assert_eq!(dialog.fields["files"], json!(there)); + assert!(!std::path::Path::new(&path).exists(), "nothing is written while it asks"); + assert!(there.iter().all(|f| std::fs::read(f).unwrap() == b"other")); + // Cancel writes nothing. + dialogs::cancel(&mut app); + assert!(!std::path::Path::new(&path).exists()); + assert!(there.iter().all(|f| std::fs::read(f).unwrap() == b"other")); + // OK writes every file. + save_as(&mut app); + dialogs::confirm(&mut app).unwrap(); + crate::background::wait_all(&mut app); + assert!(std::path::Path::new(&path).exists()); + assert!(there.iter().all(|f| std::fs::read(f).unwrap().starts_with(b"%!PS"))); + // The document's next save writes them again without asking. + rect(&mut app); + let _ = app.run("file.save", json!({})); + crate::background::wait_all(&mut app); + assert!(app.ui.dialog.is_none()); + assert!(!app.session.active().unwrap().is_dirty()); + let _ = std::fs::remove_dir_all(d); +} + #[test] fn native_saves_stream_into_the_file() { let d = dir("stream");