From 6e2ae5f82b2190cf59cb2eb52197d7fa3cd0d609 Mon Sep 17 00:00:00 2001 From: yoriswind Date: Sun, 1 Nov 2026 19:03:44 -0800 Subject: [PATCH] Honor recorded Batch Stop For Errors policy --- crates/engine/src/cmd/atn_cmds/steps_batch.rs | 7 ++- crates/engine/src/cmd/atn_cmds/tests.rs | 11 ++++- crates/engine/src/cmd/file_cmds.rs | 2 +- crates/engine/src/cmd/file_cmds/batch_run.rs | 27 ++++++++--- crates/engine/src/cmd/file_cmds/tests.rs | 46 ++++++++++++++++++- docs/atn.md | 5 ++ 6 files changed, 88 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/atn_cmds/steps_batch.rs b/crates/engine/src/cmd/atn_cmds/steps_batch.rs index 159cb0ca7b..7e30b9e598 100644 --- a/crates/engine/src/cmd/atn_cmds/steps_batch.rs +++ b/crates/engine/src/cmd/atn_cmds/steps_batch.rs @@ -118,7 +118,7 @@ fn read(d: &Descriptor) -> Option { return None; } } - let mut p = json!({"input":path_text(d.get("null")?)?,"output":path_text(d.get("T ")?)?,"flatOutput":true}); + let mut p = json!({"input":path_text(d.get("null")?)?,"output":path_text(d.get("T ")?)?,"flatOutput":true,"stopOnError":true}); let D::Reference(items) = d.get("Usng")? else { return None }; match items.as_slice() { [ReferenceItem::Name { class: action, name }, ReferenceItem::Name { class: set, name: set_name }] @@ -166,6 +166,7 @@ pub(super) fn batch_to(_: &str, p: &Value) -> Option { "flatOutput", "includeSubfolders", "overrideSaveAs", + "stopOnError", "namingComponents", "startingSerial", "compatibility", @@ -175,6 +176,10 @@ pub(super) fn batch_to(_: &str, p: &Value) -> Option { }) { return None; } + // Without Log, the recorded dialog stops at the first failing file. Native defaults continue. + if p.get("stopOnError") != Some(&Value::Bool(true)) { + return None; + } if let Some(original) = p.get(ORIGINAL) { let d = Descriptor::from_bytes(&photocraft_paint::tile::b64_decode(original.as_str()?)?).ok()?; let expected = read(&d)?; diff --git a/crates/engine/src/cmd/atn_cmds/tests.rs b/crates/engine/src/cmd/atn_cmds/tests.rs index 3ccbe9b791..9f6b258843 100644 --- a/crates/engine/src/cmd/atn_cmds/tests.rs +++ b/crates/engine/src/cmd/atn_cmds/tests.rs @@ -107,7 +107,7 @@ fn batch_descriptor(component: &str) -> Descriptor { /// Preserve the recorded template in the generic registry round-trip witness. fn batch_case() -> Value { - json!({"input":"/input","output":"/output","action":"Child","set":"S","flatOutput":true,"includeSubfolders":false,"overrideSaveAs":false, + json!({"input":"/input","output":"/output","action":"Child","set":"S","flatOutput":true,"includeSubfolders":false,"overrideSaveAs":false,"stopOnError":true, "namingComponents":[{"component":"serial3"}],"startingSerial":7,"compatibility":{"macintosh":true,"windows":false,"unix":true}, "__atnBatch":b64(&batch_descriptor("threeDigit").to_bytes())}) } @@ -123,6 +123,7 @@ fn batch_all_documented_components_and_compatibility_flags_round_trip() { let p = &imported.actions[0].steps[0].1; assert_eq!(p["namingComponents"], json!([{"component":native}])); assert_eq!(p["startingSerial"], 7); + assert_eq!(p["stopOnError"], true); assert_eq!(p["compatibility"], json!({"macintosh":true,"windows":false,"unix":true})); let (out, skipped) = export("S", &[&imported.actions[0]], &[], false).unwrap(); assert!(skipped.is_empty()); @@ -150,6 +151,14 @@ fn batch_unknown_components_wrong_start_placement_and_unsupported_modes_stay_who let mut p = batch_case(); p["target"] = json!("mask"); assert!(super::steps_batch::batch_to("file.automate.batch", &p).is_none()); + for value in [Value::Null, json!(false), json!("yes")] { + let mut p = batch_case(); + p["stopOnError"] = value; + assert!(super::steps_batch::batch_to("file.automate.batch", &p).is_none()); + } + let mut p = batch_case(); + p.as_object_mut().unwrap().remove("stopOnError"); + assert!(super::steps_batch::batch_to("file.automate.batch", &p).is_none()); } /// A set with a mapped filter, an adjustment layer, selection steps, a layer by name, a Stop, diff --git a/crates/engine/src/cmd/file_cmds.rs b/crates/engine/src/cmd/file_cmds.rs index d478c5585a..ae1cc28705 100644 --- a/crates/engine/src/cmd/file_cmds.rs +++ b/crates/engine/src/cmd/file_cmds.rs @@ -308,7 +308,7 @@ pub fn specs() -> Vec { "Batch…", &["File", "Automate"], None, - r##"{"action":name|index? (instead of steps),"set":str?,"steps":[[commandId,params]|{"command":id,"params":{}}…] (a recorded action; a Save, Save As or Save a Copy step marks the document written to output, in this command's format and options: as the save found it if it was closed or another document is active at the end, else as the action leaves it; with no such step, the active document at the end),"input":folder|[paths],"includeSubfolders":bool=false (a folder input also takes the images in its subfolders, up to 32 levels and 100000 files; each result keeps its subfolder under output),"output":folder,"format":"same|png|jpg|psd|tiff|…"="same","quality":0..12?,"jpegFormat":"baseline|optimized|progressive"?,"namingComponents":[{"component":choice}|{"text":str}]? (up to six; the exact filename, with no automatic extension),"compatibility":{"macintosh":bool,"windows":bool,"unix":bool}?,"flatOutput":bool=false (structured naming only),"overrideSaveAs":bool? (recorded routing; requires a directly represented Save configuration),"naming":string? (blank keeps source names; {name}, {serial} padded to 3 digits, {date} as YYYY-MM-DD UTC; extension added automatically),"startingSerial":1..4294967295=1 (input position, including failed inputs),"tiffLayers":bool=false,"tiffCompression":"none|lzw|zip"?,"tgaBits":24|32?,"dpxBits":8|10|12|16? (DPX: bits per sample; unset, 8 for an 8-bit document, else 16),"ddsCompression":"none|bc1|bc2|bc3"="none" (DDS: uncompressed, or block compression; BC1 keeps alpha on or off),"ddsMipmaps":bool=false (DDS: also write the full mip chain),"calledActions":[{"name","steps"}…]? (the actions Play Action steps call; default: the session's)} → {files, errors} (an input whose output name was already written in the run goes to errors)"##, + r##"{"action":name|index? (instead of steps),"set":str?,"steps":[[commandId,params]|{"command":id,"params":{}}…] (a recorded action; a Save, Save As or Save a Copy step marks the document written to output, in this command's format and options: as the save found it if it was closed or another document is active at the end, else as the action leaves it; with no such step, the active document at the end),"input":folder|[paths],"includeSubfolders":bool=false (a folder input also takes the images in its subfolders, up to 32 levels and 100000 files; each result keeps its subfolder under output),"output":folder,"format":"same|png|jpg|psd|tiff|…"="same","quality":0..12?,"jpegFormat":"baseline|optimized|progressive"?,"namingComponents":[{"component":choice}|{"text":str}]? (up to six; the exact filename, with no automatic extension),"compatibility":{"macintosh":bool,"windows":bool,"unix":bool}?,"flatOutput":bool=false (structured naming only),"overrideSaveAs":bool? (recorded routing; requires a directly represented Save configuration),"stopOnError":bool=false (report the first failed input and stop; earlier outputs remain saved),"naming":string? (blank keeps source names; {name}, {serial} padded to 3 digits, {date} as YYYY-MM-DD UTC; extension added automatically),"startingSerial":1..4294967295=1 (input position, including failed inputs),"tiffLayers":bool=false,"tiffCompression":"none|lzw|zip"?,"tgaBits":24|32?,"dpxBits":8|10|12|16? (DPX: bits per sample; unset, 8 for an 8-bit document, else 16),"ddsCompression":"none|bc1|bc2|bc3"="none" (DDS: uncompressed, or block compression; BC1 keeps alpha on or off),"ddsMipmaps":bool=false (DDS: also write the full mip chain),"calledActions":[{"name","steps"}…]? (the actions Play Action steps call; default: the session's)} → {files, errors} (an input whose output name was already written in the run goes to errors)"##, native, batch ), diff --git a/crates/engine/src/cmd/file_cmds/batch_run.rs b/crates/engine/src/cmd/file_cmds/batch_run.rs index c9a0092c3c..43c45c90f4 100644 --- a/crates/engine/src/cmd/file_cmds/batch_run.rs +++ b/crates/engine/src/cmd/file_cmds/batch_run.rs @@ -129,11 +129,15 @@ pub(crate) fn process_tree( suffix: &str, f: &dyn Fn(&mut Session, &str) -> Result, ) -> Value { - let outputs = inputs.iter().map(|BatchInput { path, rel_dir }| { + process_outputs(s, default_outputs(inputs, output, format, suffix), save, None, false, f) +} + +/// Stream legacy destination names without collecting another path list for large batches. +fn default_outputs<'a>(inputs: &'a [BatchInput], output: &'a str, format: &'a str, suffix: &'a str) -> impl Iterator { + inputs.iter().map(move |BatchInput { path, rel_dir }| { let ext = if format == "same" { same_format(path) } else { format.trim_start_matches('.').to_ascii_lowercase() }; (path.as_str(), join(&join(output, rel_dir), &format!("{}{suffix}.{ext}", stem(path)))) - }); - process_outputs(s, outputs, save, None, f) + }) } /// The extension a batch output takes with Format "Same": the input's own, except for formats @@ -183,6 +187,7 @@ fn process_outputs<'a>( outputs: impl IntoIterator, save: SaveOpts, content_format: Option<&str>, + stop_on_error: bool, f: &dyn Fn(&mut Session, &str) -> Result, ) -> Value { // The scratch sessions have default preferences; a raw opens at the user's Raw Defaults. @@ -209,7 +214,12 @@ fn process_outputs<'a>( written.record(&out, path); files.push(out); } - Err(e) => errors.push(json!({"file": path, "error": e.to_string()})), + Err(e) => { + errors.push(json!({"file": path, "error": e.to_string()})); + if stop_on_error { + break; + } + } } } json!({"files": files, "errors": errors}) @@ -275,6 +285,11 @@ pub(crate) fn batch_action(list: &[crate::actions_cmds::Action], p: &Value) -> R pub(super) fn batch(s: &mut Session, p: &Value) -> Result { let cmd = "file.automate.batch"; + let stop_on_error = match p.get("stopOnError") { + None | Some(Value::Null) => false, + Some(Value::Bool(value)) => *value, + _ => return Err(EngineError::BadParams { cmd: cmd.into(), msg: "stopOnError must be a boolean".into() }), + }; // Steps that call another action play it from the caller's actions; the playback stack comes // along so an action that batches itself is caught as a recursive call. let list = match p.get("calledActions") { @@ -389,8 +404,8 @@ pub(super) fn batch(s: &mut Session, p: &Value) -> Result { result }; let r = match names { - Some(names) => process_outputs(s, inputs.iter().map(|i| i.path.as_str()).zip(names), save, structured.then_some(format.as_str()), &run), - None => process_tree(s, &inputs, &output, &format, save, "", &run), + Some(names) => process_outputs(s, inputs.iter().map(|i| i.path.as_str()).zip(names), save, structured.then_some(format.as_str()), stop_on_error, &run), + None => process_outputs(s, default_outputs(&inputs, &output, &format, ""), save, None, stop_on_error, &run), }; if s.actions.playing > 0 { s.actions.playback_steps = carried.get(); diff --git a/crates/engine/src/cmd/file_cmds/tests.rs b/crates/engine/src/cmd/file_cmds/tests.rs index 455401016c..176e1c16e4 100644 --- a/crates/engine/src/cmd/file_cmds/tests.rs +++ b/crates/engine/src/cmd/file_cmds/tests.rs @@ -69,7 +69,7 @@ fn batch_structured_names_keep_image_format_and_refuse_unsupported_save_routing( child.set = Some("S".into()); s.actions.list.push(child); let params = json!({"input":dir,"output":output,"action":"Child","set":"S","includeSubfolders":false,"flatOutput":true, - "overrideSaveAs":false,"namingComponents":[{"text":"from-atn"}]}); + "overrideSaveAs":false,"stopOnError":true,"namingComponents":[{"text":"from-atn"}]}); let driver = crate::actions_cmds::Action::new("Driver", vec![("file.automate.batch".into(), params)]); let (bytes, skipped) = crate::atn_cmds::export("S", &[&driver], &[], false).unwrap(); assert!(skipped.is_empty()); @@ -80,6 +80,50 @@ fn batch_structured_names_keep_image_format_and_refuse_unsupported_save_routing( assert!(std::fs::read(join(&output, "from-atn")).unwrap().starts_with(b"\x89PNG\r\n\x1a\n")); } +/// Recorded Stop For Errors keeps prior writes and leaves later inputs untouched; native defaults continue. +#[test] +fn batch_recorded_stop_for_errors_retains_earlier_outputs_and_stops_before_later_inputs() { + let dir = tmp("batch-stop-for-errors"); + let first = png(&dir, "a.png", 3, 2, "#ff0000"); + let bad = join(&dir, "b.png"); + std::fs::write(&bad, b"not a PNG").unwrap(); + let last = png(&dir, "c.png", 3, 2, "#0000ff"); + let mut s = Session::new(); + for (mode, stop) in [("default", None), ("continue", Some(false)), ("stop", Some(true))] { + let output = join(&dir, mode); + let mut p = json!({"input":[first,bad,last],"output":output,"steps":[]}); + if let Some(stop) = stop { + p["stopOnError"] = json!(stop); + } + let result = s.execute("file.automate.batch", p).unwrap(); + assert_eq!(result["files"].as_array().unwrap().len(), if stop == Some(true) { 1 } else { 2 }, "{result}"); + assert_eq!(result["errors"].as_array().unwrap().len(), 1, "{result}"); + assert_eq!(result["errors"][0]["file"], bad); + assert!(std::path::Path::new(&join(&output, "a.png")).is_file()); + assert_eq!(std::path::Path::new(&join(&output, "c.png")).exists(), stop != Some(true)); + } + let mut child = crate::actions_cmds::Action::new("Child", vec![]); + child.set = Some("S".into()); + s.actions.list.push(child); + let output = join(&dir, "recorded"); + let p = json!({"input":dir,"output":output,"action":"Child","set":"S","stopOnError":true, + "includeSubfolders":false,"overrideSaveAs":false,"flatOutput":true, + "namingComponents":[{"component":"name"},{"component":"extension"}]}); + let driver = crate::actions_cmds::Action::new("Driver", vec![("file.automate.batch".into(), p)]); + let (bytes, skipped) = crate::atn_cmds::export("S", &[&driver], &[], false).unwrap(); + assert!(skipped.is_empty()); + s.actions.list.extend(crate::atn_cmds::import(&bytes).unwrap().actions); + let result = s.execute("actions.play", json!({"action":"Driver"})).unwrap(); + assert_eq!(result["ran"], 1, "{result}"); + assert!(result.get("failed").is_none(), "{result}"); + assert!(std::path::Path::new(&join(&output, "a.png")).is_file()); + assert!(!std::path::Path::new(&join(&output, "c.png")).exists()); + let invalid_output = join(&dir, "invalid"); + let err = s.execute("file.automate.batch", json!({"input":[first],"output":invalid_output,"steps":[],"stopOnError":"true"})).unwrap_err(); + assert!(err.to_string().contains("stopOnError"), "{err}"); + assert!(!std::path::Path::new(&invalid_output).exists()); +} + fn tmp(name: &str) -> String { let d = std::env::temp_dir().join(format!("photocraft-file-cmds-{}-{name}", std::process::id())); let _ = std::fs::remove_dir_all(&d); diff --git a/docs/atn.md b/docs/atn.md index 0748e37934..5189883fe7 100644 --- a/docs/atn.md +++ b/docs/atn.md @@ -254,6 +254,11 @@ The original native template format and subfolder layout remain the defaults; recorded folder steps use flat structured output. Duplicate output protection remains in place rather than silently overwriting an earlier result. +Recorded folder steps without an error log set `stopOnError`: the first failed +input is reported, earlier outputs stay saved and later inputs are not processed. +The native command still continues by default. That continuing mode cannot be +exported as a no-log Batch step, which would change its error policy. + This maps supported text-path folder forms. Alias bytes, open-document sources, Save and Close, error logs and enabled Open/dialog overrides remain preserved steps. For an admitted recorded Save As configuration, native preflight refuses