From 95d90511e89163b70d27f9ec835211698040be21 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 06:26:44 -0800 Subject: [PATCH] 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");