Merge pull request #6780 from dotCooCoo/fix/action-batch-param-types

Actions: Batch refuses options of the wrong type before it writes

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

@@ -11,7 +11,7 @@ use serde_json::json;
 
 use super::super::{Result, Session, bad};
 #[cfg(not(target_arch = "wasm32"))]
-use super::super::{bool_or, find_command, str_param};
+use super::super::{check_switches, find_command, switch_or, text_param};
 
 #[cfg(target_arch = "wasm32")]
 pub(super) fn action_batch(_: &mut Session, _: &Value) -> Result<Value> {
@@ -68,12 +68,24 @@ pub(super) fn action_batch(s: &mut Session, p: &Value) -> Result<Value> {
             ));
         }
     }
-    let source = str_param(p, "source").unwrap_or("folder");
-    let destination = str_param(p, "destination").unwrap_or("none");
+    // Every option is checked before the destination folder is made.
+    let source = text_param(p, "source", C)?.unwrap_or("folder");
+    if !matches!(source, "folder" | "datasets") {
+        return Err(bad(C, "`source` must be `folder` or `datasets`"));
+    }
+    let destination = text_param(p, "destination", C)?.unwrap_or("none");
     if !matches!(destination, "none" | "saveAndClose" | "folder") {
         return Err(bad(C, "`destination` must be `none`, `saveAndClose` or `folder`"));
     }
-    let dest_folder = str_param(p, "destinationFolder").map(absolute_path);
+    let dest_folder = text_param(p, "destinationFolder", C)?.map(absolute_path);
+    text_param(p, "folder", C)?;
+    let label = text_param(p, "label", C)?.unwrap_or("Batch").to_string();
+    let dataset_file_naming = text_param(p, "datasetFileNaming", C)?.unwrap_or("fileDataSetName");
+    let override_open = switch_or(p, "overrideOpen", true, C)?;
+    let stop_on_error = switch_or(p, "stopOnError", true, C)?;
+    let override_save = switch_or(p, "overrideSave", true, C)?;
+    let override_export = switch_or(p, "overrideExport", true, C)?;
+    check_switches(p, &["includeSubdirs"], C)?;
     if destination == "folder" {
         let Some(ref folder) = dest_folder else {
             return Err(bad(C, "`destinationFolder` is required when destination is `folder`"));
@@ -81,17 +93,15 @@ pub(super) fn action_batch(s: &mut Session, p: &Value) -> Result<Value> {
         may_write(folder)?;
         std::fs::create_dir_all(folder).map_err(|e| bad(C, format!("{folder}: {e}")))?;
     }
-    let label = str_param(p, "label").unwrap_or("Batch").to_string();
-    let override_open = bool_or(p, "overrideOpen", true);
     let run = Run {
         label: &label,
         steps: cmds,
-        stop_on_error: bool_or(p, "stopOnError", true),
-        override_save: bool_or(p, "overrideSave", true),
-        override_export: bool_or(p, "overrideExport", true),
+        stop_on_error,
+        override_save,
+        override_export,
         destination,
         dest_folder: dest_folder.as_deref(),
-        dataset_file_naming: p.get("datasetFileNaming").map_or("fileDataSetName", |v| v.as_str().unwrap_or("")),
+        dataset_file_naming,
     };
     match source {
         "folder" => batch_folder(s, C, p, override_open, &run),
@@ -186,9 +196,9 @@ fn retargeting_save(c: &Value, redirected: bool) -> bool {
 fn batch_folder(s: &mut Session, cmd: &str, p: &Value, override_open: bool, run: &Run<'_>) -> Result<Value> {
     use super::{absolute_path, may_read};
 
-    let folder = str_param(p, "folder").map(absolute_path).ok_or_else(|| bad(cmd, "missing `folder`"))?;
+    let folder = text_param(p, "folder", cmd)?.map(absolute_path).ok_or_else(|| bad(cmd, "missing `folder`"))?;
     may_read(&folder)?;
-    let include_subdirs = bool_or(p, "includeSubdirs", false);
+    let include_subdirs = switch_or(p, "includeSubdirs", false, cmd)?;
     let files = collect_files(&folder, include_subdirs, 0)?;
     if files.len() > MAX_BATCH_FILES {
         return Err(bad(cmd, format!("more than {MAX_BATCH_FILES} files")));
@@ -810,6 +820,46 @@ mod tests {
         assert!(dest.join("b.svg").is_file(), "b.svg written");
     }
 
+    #[test]
+    fn batch_refuses_options_of_the_wrong_type_before_it_writes() {
+        let src = tmp("types-src");
+        let dest = tmp("types-dest");
+        write_svg(&src, "a.svg", "#f00");
+        let mut s = Session::new();
+        let cases = [
+            ("stopOnError", json!("false"), "`stopOnError` must be true or false"),
+            ("overrideOpen", json!(0), "`overrideOpen` must be true or false"),
+            ("overrideSave", json!("true"), "`overrideSave` must be true or false"),
+            ("overrideExport", json!([]), "`overrideExport` must be true or false"),
+            ("includeSubdirs", json!(1), "`includeSubdirs` must be true or false"),
+            ("source", json!(1), "`source` must be a string"),
+            ("source", json!("files"), "`source` must be `folder` or `datasets`"),
+            ("destination", json!(true), "`destination` must be a string"),
+            ("folder", json!(5), "`folder` must be a string"),
+            ("label", json!(5), "`label` must be a string"),
+            ("datasetFileNaming", json!(3), "`datasetFileNaming` must be a string"),
+        ];
+        for (key, value, message) in cases {
+            let out = dest.join(key);
+            let mut p = json!({
+                "folder": src.to_string_lossy(),
+                "destination": "folder",
+                "destinationFolder": out.to_string_lossy(),
+                "commands": [{"command": "object.unlockAll", "params": {}}],
+            });
+            p[key] = value;
+            let e = s.execute("action.batch", &p).unwrap_err().to_string();
+            assert!(e.contains(message), "{key}: {e}");
+            assert!(!out.exists(), "{key}: the destination folder was created");
+        }
+        let e = s
+            .execute("action.batch", &json!({"folder": src.to_string_lossy(), "destination": "folder", "destinationFolder": 7, "commands": [{"command": "object.unlockAll"}]}))
+            .unwrap_err()
+            .to_string();
+        assert!(e.contains("`destinationFolder` must be a string"), "{e}");
+        assert!(s.active().is_none(), "no file was opened");
+    }
+
     #[cfg(unix)]
     #[test]
     fn batch_folder_keeps_backslash_in_source_file_name() {

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

@@ -117,7 +117,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Batch…",
             [],
             None,
-            "{commands: [{command, params}] (the action's steps), source?: folder (default) | datasets, folder? (required for folder: the source folder), includeSubdirs?: false, destination?: none (default: leave each file open) | saveAndClose (save over the source and close; for source datasets, save the open document over its own file after each data set, and refuse a document that was never saved, was converted from an older version or was restored by Data Recovery, unless a recorded Save or Save As step writes a file first; a Save or Save As step that overrideSave points into destinationFolder makes each data set's file the document's own) | folder (save or export into destinationFolder and close), destinationFolder? (required when destination is folder; also the folder Override Save/Export write into), datasetFileNaming?: fileNumber | fileDataSetName (default) | dataSetName (used only for source datasets; ignored for folder), overrideOpen?: true (skip document.open / file.open steps: the batch opens each source file), overrideSave?: true (point document.save / file.save / file.saveAs at destinationFolder/{stem}.{ext}), overrideExport?: true (point document.export / document.exportSelection the same way, and give document.exportForScreens / assets.export the folder destinationFolder/{stem}, one for each source file or data set, in which the export names its files as it does outside Batch), stopOnError?: true, label?} play the commands on every readable file in folder (and its subfolders when includeSubdirs; {stem} is the file's name without its extension and without trailing periods and spaces, with `-2`, `-3`… added to a name an earlier file used or whose file or folder has the name of one an earlier file wrote or of another source file or a folder of source files in destinationFolder), or on every data set of the open document. Data set names use the document stem plus a three-digit ordinal, the document stem and an underscore plus the data set name, or the data set name alone. Invalid file name characters become `-` and trailing periods and spaces are dropped; a name already used in the batch or whose output file or folder already exists in a destination folder the batch may read and write gets the next free `-2`, `-3` suffix before the format-derived extension. Caps: 500 files or data sets, 8 folder levels, 256 steps. Batches cannot nest. → {processed: [{path|dataset, result|error}…], count, failed, stopped, warning?} (warning: for source datasets, when Save and Close without such a step, or a Save, Save a Copy, Save as Template or export step (document.export, exportSelection, exportForScreens, exportPdf, exportEps, exportDxf, exportForWeb, exportForOffice) that writes a file and is not pointed into destinationFolder, saves every data set over the same file)",
+            "{commands: [{command, params}] (the action's steps), source?: folder (default) | datasets, folder? (required for folder: the source folder), includeSubdirs?: false, destination?: none (default: leave each file open) | saveAndClose (save over the source and close; for source datasets, save the open document over its own file after each data set, and refuse a document that was never saved, was converted from an older version or was restored by Data Recovery, unless a recorded Save or Save As step writes a file first; a Save or Save As step that overrideSave points into destinationFolder makes each data set's file the document's own) | folder (save or export into destinationFolder and close), destinationFolder? (required when destination is folder; also the folder Override Save/Export write into), datasetFileNaming?: fileNumber | fileDataSetName (default) | dataSetName (used only for source datasets; ignored for folder), overrideOpen?: true (skip document.open / file.open steps: the batch opens each source file), overrideSave?: true (point document.save / file.save / file.saveAs at destinationFolder/{stem}.{ext}), overrideExport?: true (point document.export / document.exportSelection the same way, and give document.exportForScreens / assets.export the folder destinationFolder/{stem}, one for each source file or data set, in which the export names its files as it does outside Batch), stopOnError?: true, label?} play the commands on every readable file in folder (and its subfolders when includeSubdirs; {stem} is the file's name without its extension and without trailing periods and spaces, with `-2`, `-3`… added to a name an earlier file used or whose file or folder has the name of one an earlier file wrote or of another source file or a folder of source files in destinationFolder), or on every data set of the open document. Data set names use the document stem plus a three-digit ordinal, the document stem and an underscore plus the data set name, or the data set name alone. Invalid file name characters become `-` and trailing periods and spaces are dropped; a name already used in the batch or whose output file or folder already exists in a destination folder the batch may read and write gets the next free `-2`, `-3` suffix before the format-derived extension. Caps: 500 files or data sets, 8 folder levels, 256 steps. Batches cannot nest. A switch (includeSubdirs, overrideOpen, overrideSave, overrideExport, stopOnError) that is not true or false, or a source, folder, destination, destinationFolder, datasetFileNaming or label that is not a string, is an error before any folder is made or file opened; null reads as left out. → {processed: [{path|dataset, result|error}…], count, failed, stopped, warning?} (warning: for source datasets, when Save and Close without such a step, or a Save, Save a Copy, Save as Template or export step (document.export, exportSelection, exportForScreens, exportPdf, exportEps, exportDxf, exportForWeb, exportForOffice) that writes a file and is not pointed into destinationFolder, saves every data set over the same file)",
             always,
             action_batch::action_batch
         ),

docs/mcp.md

@@ -4648,7 +4648,12 @@ template, AI or AIT format counts artboard file names when the document has any
 counts them when the document has several artboards.
 Under Data Set Name, VectorCraft makes Windows device names safe (`CON` becomes `CON-`). This is
 VectorCraft's naming behavior. Caps: 500
-files or data sets, 8 folder levels, 256 steps. An action recorded in the Actions panel leaves the artboard out,
+files or data sets, 8 folder levels, 256 steps. `action.batch` refuses `includeSubdirs`, `overrideOpen`,
+`overrideSave`, `overrideExport` or `stopOnError` given as anything but true or false, such as `"false"`, with
+`` `stopOnError` must be true or false ``, and `source`, `folder`, `destination`, `destinationFolder`,
+`datasetFileNaming` or `label` given as anything but a string with `` `label` must be a string ``, naming the key.
+It checks them, and that `source` is `folder` or `datasets`, before it makes the destination folder or opens a file.
+A null is read as a key left out. An action recorded in the Actions panel leaves the artboard out,
 and its steps of these commands get the artboard that is active when the action plays. A press with the Selection,
 Direct Selection or Group Selection tool inside an artboard makes that artboard the active one; with the Selection
 tool, a Shift-click on an empty spot inside an artboard makes it active and keeps the selection. Choosing the Artboard