diff --git a/README.md b/README.md index cb5f7019..72fcdc7f 100644 --- a/README.md +++ b/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. diff --git a/apps/cadcraft-cli/src/main.rs b/apps/cadcraft-cli/src/main.rs index b94be57c..32c781ec 100644 --- a/apps/cadcraft-cli/src/main.rs +++ b/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 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) { + 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) { + 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) { + 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 { + 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 { + 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) -> Result { + 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::, _>>().map(Value::Array), + Value::Object(m) => { + m.iter().map(|(k, x)| Ok((k.clone(), fill_last(x, h)?))).collect::, _>>().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 = 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(¶ms, last_handle).map_err(|e| format!("--cmd {id}: {e}"))?; let r = s.execute(id, ¶ms).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. diff --git a/apps/cadcraft-cli/tests/handle_chaining.rs b/apps/cadcraft-cli/tests/handle_chaining.rs new file mode 100644 index 00000000..1dff30bf --- /dev/null +++ b/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); +} diff --git a/docs/customization.md b/docs/customization.md index 41c92ed3..33d1f8a2 100644 --- a/docs/customization.md +++ b/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).