CLI: a --cmd can act on a handle an earlier --cmd returned ({last}) (#1871)

* cli: act on a handle an earlier --cmd returned ({last}) (#1663)

A run's --cmd steps were independent: mview answered {"viewports":["100"]}
but viewport.set could not use that handle, so it failed with "no viewport
given".

The batch runner now remembers the last object handle a --cmd returned (the
strings in its result that name an entity of the active drawing, so counts and
names are not mistaken for handles). A parameter value of exactly {last} in a
later --cmd, at any depth, is replaced with it; every other value is passed
through unchanged. With no handle yet the run fails with a clear error instead
of passing a bad handle to the command.

Fixes #1663

* cli: keep {last} to the drawing and handle fields that returned it

Read handles only from result handle fields, never arbitrary strings: a group named like another object's handle is a name, not a reference. Clear the remembered handle when new, open, document.switch (or a script running one) makes another drawing current, and keep a handle the new drawing's own command returns.

README.md

@@ -219,6 +219,10 @@ cadcraft-cli convert drawing.dxf drawing.svg
 cadcraft-cli convert drawing.dwg detail.png --window 0,0,20000,15000
 cadcraft-cli run drawing.dwg --cmd 'zoom {"mode":"window","p1":[0,0],"p2":[20000,15000]}' --export detail.png
 cadcraft-cli run drawing.dwg --cmd 'select {"window":[[0,0],[20000,15000]]}' --cmd 'zoom {"mode":"object"}' --export sel.png
+
+# Act on what an earlier --cmd made: {last} is the last handle it returned.
+cadcraft-cli run --metric --cmd 'layout.set {"name":"Layout1"}' \
+  --cmd 'mview {"p1":[20,20],"p2":[200,150]}' --cmd 'viewport.set {"handle":"{last}","scale":1}'
 ```
 
 How image exports (PNG 2400×1600, SVG, PDF) are framed: `convert` fits the model-space extents.

apps/cadcraft-cli/src/main.rs

@@ -28,7 +28,7 @@ macro_rules! outln {
 
 use std::process::ExitCode;
 
-use cadcraft_engine::doc::Space;
+use cadcraft_engine::doc::{Handle, Space};
 use cadcraft_engine::geom::{Bounds2, Vec2};
 use cadcraft_engine::{Session, View};
 use serde_json::{Value, json};
@@ -73,7 +73,15 @@ image framing (png 2400x1600, svg, pdf; always model space):
                            scaled to fit the image without distortion.
   --view extents           always fit the drawing extents.
   --view current           the current view (as last set by zoom/pan/view.set; a freshly
-                           opened drawing's view is its extents plus a 10% margin).";
+                           opened drawing's view is its extents plus a 10% margin).
+
+--cmd chaining (run):
+  A --cmd parameter value of exactly \"{last}\" is replaced with the last object handle an
+  earlier --cmd in the same run returned, so a command can act on what the previous one made:
+    --cmd 'mview {\"p1\":[20,20],\"p2\":[200,150]}'        prints {\"viewports\":[\"100\"]}
+    --cmd 'viewport.set {\"handle\":\"{last}\",\"scale\":1}' changes viewport 100
+  It works in any parameter, at any depth (e.g. \"handles\":[\"{last}\"]); every other string is
+  passed unchanged. With no earlier returned handle the run fails and says so.";
 
 /// What part of model space an image export shows.
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -314,6 +322,133 @@ fn skipped_message(d: &cadcraft_engine::doc::Drawing, path: &str) -> Option<Stri
     Some(format!("{path}: {n} {records} could not be read and are left out ({})", list.join(", ")))
 }
 
+/// A `--cmd` parameter value of exactly `{last}` is filled in with the last object handle an
+/// earlier `--cmd` in the same run returned: `mview` answers `{"viewports":["100"]}`, so
+/// `viewport.set {"handle":"{last}","scale":1}` changes viewport 100. Any other string is passed
+/// through unchanged, and `--script` text is never rewritten.
+const LAST_HANDLE: &str = "{last}";
+
+/// How deep [`result_handles`] walks a command result (results are the engine's own JSON).
+const RESULT_DEPTH: usize = 16;
+
+/// Result keys whose values are object references: the engine's result convention for handles.
+/// A handle can be a string (`handle`, `viewport`, `insert`…) or a list of them (`handles`,
+/// `viewports`, `members`…). Every other field may hold a user-defined name, a message or a
+/// count, and is never read as a handle — a group named `100` is a name, not object `100`.
+fn handle_field(key: &str) -> bool {
+    matches!(
+        key,
+        "handle"
+            | "handles"
+            | "viewport"
+            | "viewports"
+            | "boundary"
+            | "clip"
+            | "insert"
+            | "members"
+            | "source"
+            | "objects"
+            | "entities"
+            | "selection"
+            | "parts"
+            | "refs"
+            | "matches"
+            | "line"
+            | "arc"
+            | "blocks"
+    )
+}
+
+/// Collect the handles of `v`, a value under a handle field: a handle string, an array of them,
+/// or objects that carry handle fields of their own (`entities`, `parts`).
+fn collect_result_handles(s: &Session, v: &Value, depth: usize, out: &mut Vec<Handle>) {
+    if depth > RESULT_DEPTH {
+        return;
+    }
+    match v {
+        Value::String(t) => {
+            if let Some(h) = Handle::parse_hex(t)
+                && s.doc().is_ok_and(|d| d.entity(h).is_some())
+            {
+                out.push(h);
+            }
+        }
+        Value::Array(a) => a.iter().for_each(|x| collect_result_handles(s, x, depth + 1, out)),
+        Value::Object(m) => {
+            for (k, x) in m {
+                if handle_field(k) {
+                    collect_result_handles(s, x, depth + 1, out);
+                } else {
+                    find_result_handles(s, x, depth + 1, out);
+                }
+            }
+        }
+        _ => {}
+    }
+}
+
+/// Find handle fields nested in containers that are not handles themselves (`groups` in
+/// `groups.list`, whose members are handles): strings here are names, never references.
+fn find_result_handles(s: &Session, v: &Value, depth: usize, out: &mut Vec<Handle>) {
+    if depth > RESULT_DEPTH {
+        return;
+    }
+    match v {
+        Value::Array(a) => a.iter().for_each(|x| find_result_handles(s, x, depth + 1, out)),
+        Value::Object(m) => {
+            for (k, x) in m {
+                if handle_field(k) {
+                    collect_result_handles(s, x, depth + 1, out);
+                } else {
+                    find_result_handles(s, x, depth + 1, out);
+                }
+            }
+        }
+        _ => {}
+    }
+}
+
+/// The object handles a command result mentions, in the order it lists them. A handle-only result
+/// (a handle string or an array of them) counts as it is; otherwise only handle fields do.
+fn result_handles(s: &Session, v: &Value, depth: usize, out: &mut Vec<Handle>) {
+    match v {
+        Value::String(_) => collect_result_handles(s, v, depth, out),
+        Value::Array(a) if a.iter().all(Value::is_string) => collect_result_handles(s, v, depth, out),
+        Value::Array(_) | Value::Object(_) => find_result_handles(s, v, depth, out),
+        _ => {}
+    }
+}
+
+/// The last handle `r` returned: what a later `{last}` stands for.
+fn returned_handle(s: &Session, r: &Value) -> Option<Handle> {
+    let mut hs = Vec::new();
+    result_handles(s, r, 0, &mut hs);
+    hs.last().copied()
+}
+
+/// The active drawing's identity ([`cadcraft_engine::DocState::uid`]): a handle is only meaningful
+/// in the drawing it came from, and `new`, `open`, `document.switch` and `close` can make another
+/// drawing current.
+fn active_drawing(s: &Session) -> Option<u64> {
+    s.docs.get(s.active).map(|st| st.uid)
+}
+
+/// Every `{last}` in `p`, at any depth, replaced by `h`. With no handle to fill in it is an
+/// error, never a token reaching the command as a (bad) handle.
+fn fill_last(p: &Value, h: Option<Handle>) -> Result<Value, String> {
+    match p {
+        Value::String(t) if t.as_str() == LAST_HANDLE => match h {
+            Some(h) => Ok(Value::String(h.hex())),
+            None => Err(format!("`{LAST_HANDLE}` needs an earlier --cmd in this run to have returned a handle")),
+        },
+        Value::Array(a) => a.iter().map(|x| fill_last(x, h)).collect::<Result<Vec<_>, _>>().map(Value::Array),
+        Value::Object(m) => {
+            m.iter().map(|(k, x)| Ok((k.clone(), fill_last(x, h)?))).collect::<Result<serde_json::Map<String, Value>, _>>().map(Value::Object)
+        }
+        other => Ok(other.clone()),
+    }
+}
+
 fn run(args: &[String]) -> Result<(), String> {
     let mut s = Session::empty();
     let mut it = args.iter();
@@ -349,13 +484,31 @@ fn run(args: &[String]) -> Result<(), String> {
     }
     check_framed_output(framing, exp.as_deref())?;
     let before = model_views(&s);
+    let mut last_handle: Option<Handle> = None;
     for (kind, body) in steps {
+        // A handle belongs to the drawing that returned it: if this step makes another drawing
+        // current (`new`, `open`, `document.switch`, `close`, or a script running one of them),
+        // the remembered handle is stale. A handle the step itself returns is taken afterwards,
+        // so it is a handle of the drawing it made current.
+        let drawing = active_drawing(&s);
         match kind.as_str() {
-            "script" => s.script(&body).map_err(|e| e.to_string())?,
+            "script" => {
+                s.script(&body).map_err(|e| e.to_string())?;
+                if active_drawing(&s) != drawing {
+                    last_handle = None;
+                }
+            }
             _ => {
                 let (id, json) = body.split_once(' ').unwrap_or((&body, "{}"));
                 let params: Value = serde_json::from_str(json).map_err(|e| format!("--cmd {id}: {e}"))?;
+                let params = fill_last(&params, last_handle).map_err(|e| format!("--cmd {id}: {e}"))?;
                 let r = s.execute(id, &params).map_err(|e| e.to_string())?;
+                if active_drawing(&s) != drawing {
+                    last_handle = None;
+                }
+                if let Some(h) = returned_handle(&s, &r) {
+                    last_handle = Some(h);
+                }
                 if !r.is_null() {
                     outln!("{r}");
                 }
@@ -780,6 +933,40 @@ mod tests {
         assert!(check_framed_output(Framing::Auto, Some("a.dxf")).is_ok());
     }
 
+    #[test]
+    fn last_placeholder_takes_the_previous_handle() {
+        let mut s = sample_session();
+        let r = s.execute("line", &json!({"points": [[0, 0], [10, 0]]})).unwrap();
+        let h = returned_handle(&s, &r).expect("LINE returns its handle");
+        assert_eq!(fill_last(&json!({"handle": "{last}"}), Some(h)), Ok(json!({"handle": h.hex()})));
+        // Nested, as in a selection list.
+        assert_eq!(fill_last(&json!({"handles": ["{last}", "1"]}), Some(h)), Ok(json!({"handles": [h.hex(), "1"]})));
+        // Only the exact token: a name or a sentence that merely contains it stays as written.
+        for p in [json!("last"), json!("{last} "), json!({"name": "last"})] {
+            assert_eq!(fill_last(&p, Some(h)), Ok(p.clone()));
+        }
+        // Nothing returned yet: an error, not a token passed to the command as a bad handle.
+        assert!(fill_last(&json!({"handle": "{last}"}), None).unwrap_err().contains("{last}"));
+    }
+
+    /// Handles are read from a result by looking the strings up in the drawing: counts and names
+    /// are not mistaken for handles, and `mview`'s `viewports` works like `handle`/`handles`.
+    #[test]
+    fn returned_handle_reads_only_real_handles() {
+        let mut s = Session::empty();
+        s.new_drawing(false);
+        s.execute("layout.set", &json!({"name": "Layout1"})).unwrap();
+        let r = s.execute("mview", &json!({"p1": [20, 20], "p2": [200, 150]})).unwrap();
+        let h = returned_handle(&s, &r).expect("mview returns its viewport handle");
+        assert!(r["viewports"].as_array().is_some_and(|a| a.iter().any(|v| v.as_str() == Some(h.hex().as_str()))), "{r}");
+        assert_eq!(returned_handle(&s, &json!({"changed": 3, "name": "A1"})), None);
+        // The most recent of several.
+        let a = s.execute("line", &json!({"points": [[0, 0], [1, 0]]})).unwrap();
+        let b = s.execute("line", &json!({"points": [[0, 0], [2, 0]]})).unwrap();
+        let (ha, hb) = (returned_handle(&s, &a).unwrap(), returned_handle(&s, &b).unwrap());
+        assert_eq!(returned_handle(&s, &json!({"handles": [ha.hex(), hb.hex()]})), Some(hb));
+    }
+
     #[test]
     fn default_framing_is_extents() {
         // convert: nothing ran, always the extents.

apps/cadcraft-cli/tests/handle_chaining.rs

@@ -0,0 +1,225 @@
+//! A `--cmd` can act on a handle an earlier `--cmd` in the same run returned (#1663).
+//!
+//! `mview` answers `{"viewports":["100"]}`, but a following `viewport.set` could not use that
+//! handle: it had to be guessed, selected by point, or activated with `mspace`. A `--cmd` parameter
+//! value of exactly `{last}` is now filled in with the last handle an earlier `--cmd` returned.
+
+use std::process::{Command, Output};
+
+fn run(args: &[&str]) -> Output {
+    Command::new(env!("CARGO_BIN_EXE_cadcraft-cli"))
+        .args(args)
+        .stdout(std::process::Stdio::piped())
+        .stderr(std::process::Stdio::piped())
+        .output()
+        .expect("run cadcraft-cli")
+}
+
+/// The issue's sequence: `viewport.set` straight after `mview`, addressing the new viewport by the
+/// handle the runner remembers.
+#[test]
+fn viewport_set_after_mview_uses_the_returned_handle() {
+    let out = run(&[
+        "run",
+        "--metric",
+        "--cmd",
+        r#"layout.set {"name":"Layout1"}"#,
+        "--cmd",
+        r#"mview {"p1":[20,20],"p2":[200,150]}"#,
+        "--cmd",
+        r#"viewport.set {"handle":"{last}","scale":1}"#,
+    ]);
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    assert!(out.status.success(), "exit {:?}\nstdout: {stdout}\nstderr: {stderr}", out.status);
+    let mview = stdout.lines().find(|l| l.contains("\"viewports\"")).unwrap_or_else(|| panic!("no mview result in {stdout:?}"));
+    let created: serde_json::Value = serde_json::from_str(mview).expect("mview result is JSON");
+    let handle = created["viewports"].as_array().and_then(|a| a.first()).and_then(|h| h.as_str()).expect("a viewport handle");
+    assert!(handle.eq_ignore_ascii_case("100"), "{mview}");
+    let set = stdout.lines().find(|l| l.contains("\"viewHeight\"")).unwrap_or_else(|| panic!("no viewport.set result in {stdout:?}"));
+    let vp: serde_json::Value = serde_json::from_str(set).expect("viewport.set result is JSON");
+    assert_eq!(vp[0]["handle"].as_str(), Some(handle), "viewport.set changed the viewport `{handle}`: {set}");
+    assert_eq!(vp[0]["scale"].as_f64(), Some(1.0), "{set}");
+}
+
+/// `{last}` with no handle to fill in is a clear error, not the token reaching the command.
+#[test]
+fn last_without_an_earlier_handle_is_an_error() {
+    let out = run(&["run", "--metric", "--cmd", r#"viewport.set {"handle":"{last}","scale":1}"#]);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    assert_eq!(out.status.code(), Some(1), "stdout: {:?}\nstderr: {stderr}", String::from_utf8_lossy(&out.stdout));
+    assert!(stderr.contains("{last}"), "the error names the placeholder: {stderr}");
+    assert!(!stderr.contains("panicked"), "{stderr}");
+}
+
+/// The last JSON value a run printed: command results are one JSON value a line (`null` is not
+/// printed).
+fn last_json(stdout: &str) -> serde_json::Value {
+    let line = stdout
+        .lines()
+        .rev()
+        .find(|l| {
+            let t = l.trim_start();
+            t.starts_with('{') || t.starts_with('[')
+        })
+        .unwrap_or_else(|| panic!("no JSON result in {stdout:?}"));
+    serde_json::from_str(line).expect("command result is JSON")
+}
+
+/// `{last}` was not replaced: the error names the placeholder, and the run fails cleanly.
+fn assert_needs_an_earlier_handle(out: &Output) {
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    assert_eq!(out.status.code(), Some(1), "stdout: {:?}\nstderr: {stderr}", String::from_utf8_lossy(&out.stdout));
+    assert!(stderr.contains("{last}"), "the error names the placeholder: {stderr}");
+    assert!(!stderr.contains("panicked"), "{stderr}");
+}
+
+/// A user-defined name that happens to look like a handle is not a returned handle. `group`
+/// answers `{"name":"100","members":1}`: `100` is the first line's handle, but the name is not an
+/// object reference and must not replace the last handle the `line` command returned (`101`).
+#[test]
+fn a_group_name_that_looks_like_a_handle_is_not_last() {
+    let out = run(&[
+        "run",
+        "--metric",
+        "--cmd",
+        r#"line {"points":[[0,0],[10,0]]}"#,
+        "--cmd",
+        r#"line {"points":[[0,10],[10,10]]}"#,
+        "--cmd",
+        r#"group {"name":"100","handles":["101"]}"#,
+        "--cmd",
+        r#"erase {"handles":["{last}"]}"#,
+        "--cmd",
+        r#"entities {"type":"LINE"}"#,
+    ]);
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    assert!(out.status.success(), "exit {:?}\nstdout: {stdout}\nstderr: {stderr}", out.status);
+    let r = last_json(&stdout);
+    assert_eq!(r["count"].as_u64(), Some(1), "one line is left: {stdout}");
+    let left: Vec<&str> = r["entities"].as_array().into_iter().flatten().filter_map(|e| e["handle"].as_str()).collect();
+    assert_eq!(left, ["100"], "the group name 100 did not replace the handle 101 the line command returned: {stdout}");
+}
+
+/// `new` makes another drawing current: the handle an earlier `--cmd` returned in the old drawing
+/// is not a handle of the new one.
+#[test]
+fn last_is_cleared_when_new_switches_drawings() {
+    let out = run(&[
+        "run",
+        "--metric",
+        "--cmd",
+        r#"line {"points":[[0,0],[10,0]]}"#,
+        "--cmd",
+        r#"new {"metric":true}"#,
+        "--cmd",
+        r#"erase {"handles":["{last}"]}"#,
+    ]);
+    assert_needs_an_earlier_handle(&out);
+}
+
+/// `open` makes the opened file the current drawing: a handle from the drawing before it is stale.
+#[test]
+fn last_is_cleared_when_open_switches_drawings() {
+    let dir = std::env::temp_dir().join(format!("cc-cli-last-open-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).expect("temp dir");
+    let dxf = dir.join("opened.dxf");
+    let sample = run(&["sample", "bracket", dxf.to_str().expect("temp path")]);
+    assert!(sample.status.success(), "sample failed: {}", String::from_utf8_lossy(&sample.stderr));
+    let open = format!(r#"open {{"path":{}}}"#, serde_json::to_string(dxf.to_str().expect("temp path")).expect("path JSON"));
+    let out = run(&["run", "--metric", "--cmd", r#"line {"points":[[0,0],[10,0]]}"#, "--cmd", &open, "--cmd", r#"erase {"handles":["{last}"]}"#]);
+    assert_needs_an_earlier_handle(&out);
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+/// `document.switch` changes the current drawing without adding one: a handle remembered from the
+/// drawing left behind must not be used in the one switched to.
+#[test]
+fn last_is_cleared_when_document_switch_changes_drawings() {
+    let out = run(&[
+        "run",
+        "--metric",
+        "--cmd",
+        r#"line {"points":[[0,0],[10,0]]}"#,
+        "--cmd",
+        r#"new {"metric":true}"#,
+        "--cmd",
+        r#"line {"points":[[0,10],[10,10]]}"#,
+        "--cmd",
+        r#"document.switch {"index":0}"#,
+        "--cmd",
+        r#"erase {"handles":["{last}"]}"#,
+    ]);
+    assert_needs_an_earlier_handle(&out);
+}
+
+/// A handle a command returns after a switch belongs to the drawing now current, so it is kept.
+#[test]
+fn a_handle_returned_after_a_switch_is_last() {
+    let out = run(&[
+        "run",
+        "--metric",
+        "--cmd",
+        r#"line {"points":[[0,0],[10,0]]}"#,
+        "--cmd",
+        r#"new {"metric":true}"#,
+        "--cmd",
+        r#"document.switch {"index":0}"#,
+        "--cmd",
+        r#"line {"points":[[0,10],[10,10]]}"#,
+        "--cmd",
+        r#"erase {"handles":["{last}"]}"#,
+        "--cmd",
+        r#"entities {"type":"LINE"}"#,
+    ]);
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    assert!(out.status.success(), "exit {:?}\nstdout: {stdout}\nstderr: {stderr}", out.status);
+    let r = last_json(&stdout);
+    assert_eq!(r["count"].as_u64(), Some(1), "one line is left: {stdout}");
+    let left: Vec<&str> = r["entities"].as_array().into_iter().flatten().filter_map(|e| e["handle"].as_str()).collect();
+    assert_eq!(left, ["100"], "the line created after the switch was erased: {stdout}");
+}
+
+/// A script can change the current drawing too (`new {"metric":true}` at a script line), so the
+/// remembered handle is cleared after a script that changed it.
+#[test]
+fn last_is_cleared_by_a_script_that_changes_drawings() {
+    let out = run(&[
+        "run",
+        "--metric",
+        "--cmd",
+        r#"line {"points":[[0,0],[10,0]]}"#,
+        "--script",
+        r#"new {"metric":true}"#,
+        "--cmd",
+        r#"erase {"handles":["{last}"]}"#,
+    ]);
+    assert_needs_an_earlier_handle(&out);
+}
+
+/// Only the exact token is a placeholder: ordinary JSON strings are left alone.
+#[test]
+fn ordinary_values_are_not_placeholders() {
+    let dir = std::env::temp_dir().join(format!("cc-cli-last-placeholder-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).expect("temp dir");
+    let out_dxf = dir.join("text.dxf");
+    let out = run(&[
+        "run",
+        "--metric",
+        "--cmd",
+        r#"layout.set {"name":"Layout1"}"#,
+        "--cmd",
+        r#"mview {"p1":[20,20],"p2":[200,150]}"#,
+        "--cmd",
+        r#"text {"at":[0,0],"height":2,"text":"last and {last}"}"#,
+        "--save",
+        out_dxf.to_str().expect("temp path"),
+    ]);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    assert!(out.status.success(), "exit {:?}\nstderr: {stderr}", out.status);
+    let saved = std::fs::read_to_string(&out_dxf).expect("saved DXF");
+    assert!(saved.contains("last and {last}"), "the text is stored literally");
+    let _ = std::fs::remove_dir_all(&dir);
+}

docs/customization.md

@@ -44,6 +44,9 @@ Every command has a JSON form that runs without dialogs. `cadcraft-cli commands
 the catalog: each command's id and its parameters.
 
 - **Headless batch jobs:** `cadcraft-cli run FILE --cmd 'id {json}' --save OUT --export OUT.png`.
+  A `--cmd` parameter value of exactly `{last}` is replaced with the last object handle an earlier
+  `--cmd` in the same run returned, so `viewport.set {"handle":"{last}","scale":1}` can follow
+  `mview`. Every other value is passed unchanged.
 - **The running app:** start it with `--control PORT` and send JSON lines to `127.0.0.1:PORT`.
   See [control-protocol.md](control-protocol.md).