Merge pull request #6751 from dotCooCoo/feat/eps-save-use-artboards

EPS: Save As shows Use Artboards, off by default, and the box fits all visible art

crates/engine/src/cmd/fileio.rs

@@ -71,7 +71,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;
@@ -82,6 +81,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};

crates/engine/src/cmd/fileio/load.rs

@@ -1019,13 +1019,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<String>)> {
+    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<String>),
+    targets: impl IntoIterator<Item = &'a str>,
+    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

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<String>,
+    /// 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<String>)>,
 }
 
 impl SavePlan {
@@ -313,7 +317,8 @@ pub fn save_plan(s: &Session, mode: SaveMode, p: &Value) -> Result<SavePlan> {
         .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.
@@ -910,6 +915,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())?;
+        }
         if let Some(stopped) = stop(&files.iter().map(|(p, _)| p.as_str()).collect::<Vec<_>>()) {
             return Ok(stopped);
         }

crates/engine/src/cmd/recovery.rs

@@ -1163,6 +1163,7 @@ pub fn jobs(s: &mut Session, store: &Arc<dyn RecoveryStore>) -> Result<(Vec<Reco
             folder: None,
             modified: None,
             notes: Vec::new(),
+            lossy_source: None,
         };
         let save = fileio::job_for(s, st, plan)?;
         let meta = Meta {

crates/engine/src/tests/save.rs

@@ -569,6 +569,30 @@ fn eps_save_with_use_artboards_and_a_named_artboard_reports_it_as_the_range() {
     std::fs::remove_dir_all(d).unwrap();
 }
 
+#[test]
+fn eps_save_with_use_artboards_refuses_to_replace_the_file_read_from_before_writing_any_file() {
+    let d = dir("eps-artboards-lossy-sibling");
+    let eps_path = path(&d, "Art.eps");
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 200, "height": 100, "artboards": 2})).unwrap();
+    rect(&mut s);
+    let options = json!({"useArtboards": true, "previewFormat": "none", "thumbnails": false});
+    let r = s.execute("document.save", &json!({"path": eps_path, "options": options})).unwrap();
+    let sibling = r["files"][1].as_str().unwrap().to_string();
+    let original = std::fs::read(&sibling).unwrap();
+    std::fs::remove_file(&eps_path).unwrap();
+    let st = s.doc_mut().unwrap();
+    st.imported_from = Some(sibling.clone());
+    st.import_losses.push("test import omitted artwork".into());
+    let err = s.execute("document.save", &json!({"path": eps_path, "options": options})).unwrap_err().to_string();
+    assert!(err.contains("acknowledgeLoss") && err.contains(&sibling), "{err}");
+    assert_eq!(std::fs::read(&sibling).unwrap(), original);
+    assert!(!std::path::Path::new(&eps_path).exists(), "no file of the set is written when one is refused");
+    s.execute("document.save", &json!({"path": eps_path, "options": options, "acknowledgeLoss": true})).unwrap();
+    assert!(std::path::Path::new(&eps_path).exists());
+    std::fs::remove_dir_all(d).unwrap();
+}
+
 #[test]
 fn eps_save_with_use_artboards_writes_the_central_file_and_one_per_artboard() {
     let d = dir("eps-artboards");

crates/ui-egui/src/io/save.rs

@@ -300,7 +300,9 @@ pub(super) fn save_suggesting(
         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)), None);
+        // 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)), None);
     }
     if is_web(app) && ask_options {
         return Ok(open_options(app, mode.command(), first.format, &first.name, true));
@@ -337,6 +339,7 @@ pub(super) fn save_suggesting(
         crate::dialogs::opened_by_panel(app);
         return r;
     }
+    let chosen = plan(app, mode, &q)?;
     write_plan(app, chosen, (save_command(mode), q), None)
 }
 

crates/ui-egui/src/tests/save.rs

@@ -298,6 +298,101 @@ fn replacing_a_file_that_opening_left_things_out_of_asks_first() {
     let _ = std::fs::remove_dir_all(d);
 }
 
+#[test]
+fn a_save_does_not_replace_the_file_read_from_through_a_file_written_beside_the_path() {
+    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 siblings = artboard_files(&d);
+    assert_eq!(siblings.len(), 2, "{siblings:?}");
+    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();
+    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);
+    // 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<String> {
+    let mut files: Vec<String> = 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");