diff --git a/crates/engine/src/variables_cmds.rs b/crates/engine/src/variables_cmds.rs index cbc3737dee..3180e8e96a 100644 --- a/crates/engine/src/variables_cmds.rs +++ b/crates/engine/src/variables_cmds.rs @@ -363,7 +363,6 @@ fn split_csv(line: &str, delim: u8) -> Vec { fn export_as_files(s: &mut Session, p: &Value) -> Result { let dir = p.get("dir").and_then(Value::as_str).ok_or_else(|| bad("file.export.dataSetsAsFiles", "need `dir`"))?; let format = p.get("format").and_then(Value::as_str).unwrap_or("png"); - std::fs::create_dir_all(dir).map_err(|e| EngineError::Other(format!("mkdir `{dir}`: {e}")))?; let st = s.active().ok_or(EngineError::NoDocument)?; let original = st.doc.clone(); let vars = original.variables.clone(); @@ -375,17 +374,29 @@ fn export_as_files(s: &mut Session, p: &Value) -> Result { // Filename template: `{name}` (sanitised data-set name), `{index}` (1-based). Default `{name}`. let template = p.get("naming").and_then(Value::as_str).unwrap_or("{name}"); let doc_stem = original.name.rsplit_once('.').map_or(original.name.as_str(), |(a, _)| a).to_string(); + // Plan the whole batch before writing: sanitised names or a constant template may collide. + // Match the case-insensitive comparison used by the other batch exporters. + let mut claimed = std::collections::HashMap::new(); let mut files = Vec::new(); for (n, set) in sets.iter().enumerate() { - let mut doc = (*original).clone(); - apply_to_doc(&mut doc, &vars, set)?; let safe: String = set.name.chars().map(|c| if c.is_alphanumeric() || c == '-' || c == '_' { c } else { '_' }).collect(); let stem = template.replace("{name}", &safe).replace("{index}", &format!("{:03}", n + 1)).replace("{document}", &doc_stem); let path = format!("{dir}/{stem}.{format}"); + if let Some(first) = claimed.insert(path.to_lowercase(), &set.name) { + return Err(bad( + "file.export.dataSetsAsFiles", + format!("data sets `{first}` and `{}` resolve to the same output `{path}`; include {{index}} in `naming` to use distinct filenames", set.name), + )); + } + files.push(path); + } + std::fs::create_dir_all(dir).map_err(|e| EngineError::Other(format!("mkdir `{dir}`: {e}")))?; + for (set, path) in sets.iter().zip(&files) { + let mut doc = (*original).clone(); + apply_to_doc(&mut doc, &vars, set)?; let opts = photocraft_io::ExportOptions::default(); let bytes = photocraft_io::export(&doc, format, &opts).map(|r| r.bytes).map_err(|e| EngineError::Other(format!("export `{}`: {e}", set.name)))?; - crate::file_cmds::write_file(&path, &bytes)?; - files.push(path); + crate::file_cmds::write_file(path, &bytes)?; } Ok(json!({"files": files, "count": files.len()})) } @@ -446,7 +457,7 @@ pub fn specs() -> Vec { "file.export.dataSetsAsFiles", "Data Sets as Files…", &["File", "Export"], - "{dir, format?:png, dataSets?[names], naming?:\"{name}|{index}|{document}\"} → {files,count}: apply each data set and export the flattened document", + "{dir, format?:png, dataSets?[names], naming?:\"{name}|{index}|{document}\"} → {files,count}: apply each data set and export the flattened document; colliding filenames are rejected before writing (include {index} to disambiguate)", |s, p| export_as_files(s, p) ), CommandSpec { diff --git a/crates/engine/src/variables_cmds/tests.rs b/crates/engine/src/variables_cmds/tests.rs index ed14f8acfc..37fbad93fc 100644 --- a/crates/engine/src/variables_cmds/tests.rs +++ b/crates/engine/src/variables_cmds/tests.rs @@ -321,6 +321,88 @@ fn export_data_sets_as_files() { assert_eq!(text_of(&s, title), "Old"); } +fn visibility_export_session(names: &[&str]) -> Session { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8, "background": "white", "name": "card"})).unwrap(); + let layer = doc(&s).layers[0].id; + s.execute("image.variables.define", json!({"defs": [{"name": "show", "layer": layer.0, "type": "visibility"}]})).unwrap(); + let sets: Vec<_> = + names.iter().enumerate().map(|(i, name)| json!({"name": name, "values": [{"variable": "show", "kind": "visibility", "value": i == 0}]})).collect(); + s.execute("image.variables.dataSets", json!({"dataSets": sets})).unwrap(); + s +} + +#[test] +fn export_data_sets_rejects_collisions_before_writing() { + for (label, names, naming, collision) in [ + ("sanitized", ["unique", "a b", "a_b"], "{name}", "a_b.png"), + ("constant", ["unique", "a b", "a_b"], "{document}", "card.png"), + ("case", ["unique", "Card", "card"], "{name}", "card.png"), + ] { + let mut s = visibility_export_session(&names); + let before = s.active().unwrap().doc.clone(); + let revision = s.active().unwrap().revision; + let history = s.active().unwrap().history.entries(); + let past = s.active().unwrap().history.past_len(); + let redo = s.active().unwrap().history.can_redo(); + let dir = tmp(&format!("collision-{label}")); + let first = format!("{dir}/unique.png"); + let collided = format!("{dir}/{collision}"); + std::fs::write(&first, b"keep earlier export").unwrap(); + std::fs::write(&collided, b"keep colliding export").unwrap(); + let err = s.execute("file.export.dataSetsAsFiles", json!({"dir": dir, "naming": naming})).unwrap_err(); + assert!(matches!(err, EngineError::BadParams { .. }), "{err}"); + let message = err.to_string(); + assert!(message.contains(collision) && message.contains("{index}"), "{message}"); + assert_eq!(std::fs::read(&first).unwrap(), b"keep earlier export"); + assert_eq!(std::fs::read(&collided).unwrap(), b"keep colliding export"); + assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 2); + let absent = format!("{dir}/not-created"); + assert!(s.execute("file.export.dataSetsAsFiles", json!({"dir": absent, "naming": naming})).is_err()); + assert!(!std::path::Path::new(&absent).exists()); + let after = s.active().unwrap(); + assert!(Arc::ptr_eq(&before, &after.doc)); + assert_eq!(after.revision, revision); + assert_eq!(after.history.entries(), history); + assert_eq!(after.history.past_len(), past); + assert_eq!(after.history.can_redo(), redo); + std::fs::remove_dir_all(dir).unwrap(); + } +} + +#[test] +fn export_data_sets_unique_paths_preserve_variations_and_document() { + for (label, names, naming, selected, expected) in [ + ("indexed", ["a b", "a_b"], "card-{index}", None, vec!["card-001.png", "card-002.png"]), + ("distinct", ["a-b", "a_b"], "{name}", None, vec!["a-b.png", "a_b.png"]), + ("subset", ["a b", "a_b"], "{name}", Some(vec!["a b"]), vec!["a_b.png"]), + ] { + let mut s = visibility_export_session(&names); + let before = s.active().unwrap().doc.clone(); + let revision = s.active().unwrap().revision; + let history = s.active().unwrap().history.entries(); + let past = s.active().unwrap().history.past_len(); + let redo = s.active().unwrap().history.can_redo(); + let dir = tmp(&format!("unique-{label}")); + let result = s.execute("file.export.dataSetsAsFiles", json!({"dir": dir, "naming": naming, "dataSets": selected})).unwrap(); + let paths: Vec<_> = expected.iter().map(|name| format!("{dir}/{name}")).collect(); + assert_eq!(result, json!({"files": paths, "count": paths.len()})); + assert_eq!(std::fs::read_dir(&dir).unwrap().count(), paths.len()); + for (i, path) in paths.iter().enumerate() { + let exported = photocraft_io::import(path, &std::fs::read(path).unwrap()).unwrap().document; + let pixel = photocraft_compose::render(&exported, Rect::new(0, 0, 1, 1)).px[0]; + assert_eq!(pixel, if i == 0 { [1.0; 4] } else { [0.0; 4] }); + } + let after = s.active().unwrap(); + assert!(Arc::ptr_eq(&before, &after.doc)); + assert_eq!(after.revision, revision); + assert_eq!(after.history.entries(), history); + assert_eq!(after.history.past_len(), past); + assert_eq!(after.history.can_redo(), redo); + std::fs::remove_dir_all(dir).unwrap(); + } +} + #[test] fn pixel_replacement_changes_the_layer() { let (mut s, photo, _b, _t) = session();