From 96356dc544892203b5d059969e8a38ed7297ecd5 Mon Sep 17 00:00:00 2001 From: Mateusz Sych <151755914+SychPL@users.noreply.github.com> Date: Thu, 15 Oct 2026 08:13:14 +0200 Subject: [PATCH] CLI snapshot takes --in; an unknown command suggests close ids (#1301) snapshot starts from --in (a design or STEP file) with the script on top, as run does, and refuses options it doesn't take; an unknown command id names the closest registered ids. --- apps/solidcraft-cli/src/main.rs | 74 +++++++++++++++++------- apps/solidcraft-cli/tests/run_results.rs | 38 ++++++++++++ crates/engine/src/cmd/mod.rs | 42 ++++++++++++++ crates/engine/src/lib.rs | 2 +- crates/engine/src/tests.rs | 14 +++++ docs/book/src/dev/commands.md | 3 +- 6 files changed, 149 insertions(+), 24 deletions(-) diff --git a/apps/solidcraft-cli/src/main.rs b/apps/solidcraft-cli/src/main.rs index 80cfce92..cf4e5c6d 100644 --- a/apps/solidcraft-cli/src/main.rs +++ b/apps/solidcraft-cli/src/main.rs @@ -6,7 +6,7 @@ //! solidcraft-cli script [--out script.json] the design's history as a command script //! solidcraft-cli bench-open [--parse-only] [--display LEVEL [--edit NAME EXPR]] loading timings as JSON lines //! solidcraft-cli bench-agent [--scale F] [--save F] [--script-out F] a ~1,300-body design built command by command, timed -//! solidcraft-cli snapshot --out shot.png [--width W] [--height H] [--view iso|front|top|…] +//! solidcraft-cli snapshot [--in design|part.step] --out shot.png [--width W] [--height H] [--view iso|front|top|…] //! solidcraft-cli exec [json-params] run one command on an empty design //! solidcraft-cli commands the command registry as JSON //! solidcraft-cli recipe translate an oracle recipe to a script @@ -47,7 +47,9 @@ const USAGE: &str = "usage: (the design's history as a command script that eval and run replay) solidcraft-cli bench-open [--parse-only] [--display LEVEL [--edit NAME EXPR]] solidcraft-cli bench-agent [--scale F] [--save design.solidcraft] [--script-out script.json] - solidcraft-cli snapshot --out shot.png [--width W] [--height H] [--view iso|front|back|top|bottom|left|right] + solidcraft-cli snapshot [--in design.solidcraft|part.step] --out shot.png [--width W] [--height H] [--view iso|front|back|top|bottom|left|right] + solidcraft-cli snapshot --in --out shot.png ... + (as with run, --in starts from a design and the script runs on top of it; an option snapshot doesn't take is an error) solidcraft-cli render --out image.png [--setup NAME] [--width W] [--height H] [--transparent] [--overwrite] [--timeout SECONDS] solidcraft-cli exec [json-params] solidcraft-cli commands @@ -213,26 +215,17 @@ fn cmd_run(args: &[String]) -> Result<(), String> { if path.is_none() && base.is_none() { return Err(USAGE.to_string()); } - // `--in` starts from a design or STEP file; the positional script then runs on top of it. - let mut s = match base { - Some(b) => load(b)?, - None => Session::default(), - }; - let results = if args.iter().any(|a| a == "--results") { + let (mut s, results) = if args.iter().any(|a| a == "--results") { // Collect every command's result (scripts only), then process requested files too. + let mut s = match base { + Some(b) => load(b)?, + None => Session::default(), + }; let v = read_json(path.ok_or("--results needs a script")?)?; let r = s.run_script(&v).map_err(|e| e.to_string())?; - Some(Value::Array(r)) + (s, Some(Value::Array(r))) } else { - match (base, path) { - (Some(_), Some(p)) => { - let v = read_json(p)?; - s.run_script(&v).map_err(|e| e.to_string())?; - } - (None, Some(p)) => s = load(p)?, - _ => {} - } - None + (open(base, path)?, None) }; // A feature that fails leaves the bodies as they were before it (a parameter change can // break a feature without failing the command that made it): writing them would hand out @@ -256,6 +249,40 @@ fn cmd_run(args: &[String]) -> Result<(), String> { Ok(()) } +/// The session `run` and `snapshot` start from: `--in` (a design or STEP file) with the +/// positional script run on top of it, or either one alone. +fn open(base: Option<&str>, path: Option<&String>) -> Result { + match (base, path) { + (Some(b), Some(p)) => { + let mut s = load(b)?; + s.run_script(&read_json(p)?).map_err(|e| e.to_string())?; + Ok(s) + } + (Some(b), None) => load(b), + (None, Some(p)) => load(p), + (None, None) => Err(USAGE.to_string()), + } +} + +/// Refuses an option `cmd` doesn't take, and one missing its value: a typo, or another +/// command's option, would otherwise be ignored without a word. +fn known_flags(cmd: &str, args: &[String], valued: &[&str]) -> Result<(), String> { + let mut i = 0; + while let Some(a) = args.get(i) { + if valued.contains(&a.as_str()) { + match args.get(i + 1) { + Some(v) if !v.starts_with("--") => i += 2, + _ => return Err(format!("{cmd}: {a} needs a value")), + } + } else if a.starts_with("--") { + return Err(format!("{cmd} doesn't take {a} (it takes {})", valued.join(", "))); + } else { + i += 1; + } + } + Ok(()) +} + /// Every failing timeline feature, one per line, as an error; Ok when none fails. fn timeline_ok(s: &Session) -> Result<(), String> { let failed: Vec = s.model.results.iter().filter_map(|r| r.error.as_ref().map(|e| format!(" {}: {e}", r.name))).collect(); @@ -304,11 +331,14 @@ fn cmd_script(args: &[String]) -> Result<(), String> { } fn cmd_snapshot(args: &[String]) -> Result<(), String> { - let path = args.first().ok_or_else(|| USAGE.to_string())?; + known_flags("snapshot", args, &["--in", "--out", "--width", "--height", "--view"])?; let out = flag(args, "--out").ok_or("snapshot needs --out FILE.png")?; - let w = flag(args, "--width").and_then(|x| x.parse().ok()).unwrap_or(1280usize); - let h = flag(args, "--height").and_then(|x| x.parse().ok()).unwrap_or(800usize); - let s = load(path)?; + let size = |name: &str, default: usize| match flag(args, name) { + None => Ok(default), + Some(x) => x.parse().ok().filter(|n| (16..=8192).contains(n)).ok_or_else(|| format!("{name} must be 16 to 8192 pixels, not `{x}`")), + }; + let (w, h) = (size("--width", 1280)?, size("--height", 800)?); + let s = open(flag(args, "--in"), positional(args))?; let mut cam = solidcraft_engine::view::home_camera(&s); if let Some(v) = flag(args, "--view") { cam.set_view(StandardView::parse(v).ok_or_else(|| format!("unknown view `{v}`"))?); diff --git a/apps/solidcraft-cli/tests/run_results.rs b/apps/solidcraft-cli/tests/run_results.rs index b4820c8b..5b805ad2 100644 --- a/apps/solidcraft-cli/tests/run_results.rs +++ b/apps/solidcraft-cli/tests/run_results.rs @@ -132,3 +132,41 @@ fn legacy_extension_and_format_tag_still_open() { let reopened = solidcraft_engine::io::read_design(&std::fs::read(&resaved).unwrap()).unwrap(); assert!(reopened.to_json().contains("\"solidcraft/")); } + +/// `snapshot` starts from `--in` as `run` does (its script runs on that design), and refuses an +/// option it doesn't take rather than ignoring it. +#[test] +fn snapshot_runs_its_script_on_the_in_design() { + let dir = Scratch::new(); + let base = dir.0.join("base.solidcraft"); + json_output(cli().arg("run").arg(dir.script()).arg("--save").arg(&base).output().unwrap()); + let shifted = dir.0.join("shifted.json"); + std::fs::write( + &shifted, + json!({"commands": [{"command": "solid.box", "params": {"length": 2, "width": 3, "height": 4, "corner": [10, 0, 0]}}]}).to_string(), + ) + .unwrap(); + let shot = |name: &str, args: &[&std::ffi::OsStr]| { + let png = dir.0.join(name); + json_output(cli().arg("snapshot").args(args).arg("--out").arg(&png).args(["--width", "64", "--height", "48"]).output().unwrap()); + std::fs::read(&png).unwrap() + }; + let alone = shot("alone.png", &[shifted.as_os_str()]); + let on_base = shot("on_base.png", &[shifted.as_os_str(), "--in".as_ref(), base.as_os_str()]); + let base_only = shot("base.png", &["--in".as_ref(), base.as_os_str()]); + assert_ne!(alone, on_base, "the script ran on the --in design"); + assert_ne!(base_only, on_base); + for (bad, says) in [(["--frobnicate", "1"], "doesn't take --frobnicate"), (["--width", "wide"], "--width must be 16 to 8192")] { + let output = cli().arg("snapshot").arg(&base).arg("--out").arg(dir.0.join("x.png")).args(bad).output().unwrap(); + assert_eq!(output.status.code(), Some(1)); + assert!(String::from_utf8_lossy(&output.stderr).contains(says), "{}", String::from_utf8_lossy(&output.stderr)); + } +} + +/// An unknown command's error suggests the id that was meant. +#[test] +fn unknown_command_suggests_the_closest_id() { + let output = cli().args(["exec", "file.open"]).output().unwrap(); + assert_eq!(output.status.code(), Some(1)); + assert!(String::from_utf8_lossy(&output.stderr).contains("did you mean `doc.open`?"), "{}", String::from_utf8_lossy(&output.stderr)); +} diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index fe64cb2d..07f2c088 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -269,3 +269,45 @@ pub fn find_command(id: &str) -> Option<&'static CommandSpec> { let all = command_specs(); all.iter().find(|c| c.id == id).or_else(|| all.iter().find(|c| c.id.eq_ignore_ascii_case(id))).copied() } + +/// The end of an unknown command's error: up to three ids like `id`, nearest first. Ids a few +/// letters away (`solid.extrud` → `solid.extrude`) come before ids that end in the same word +/// (`file.open` → `doc.open`). Empty when nothing is close. +pub(crate) fn did_you_mean(id: &str) -> String { + let id = id.to_ascii_lowercase(); + if id.len() > 64 { + return String::new(); + } + let last = |s: &str| s.rsplit('.').next().unwrap_or(s).to_string(); + let near = (id.chars().count() / 4).max(2); + let mut found: Vec<(bool, usize, &str)> = command_specs() + .iter() + .filter_map(|c| { + let d = edit_distance(&id, c.id); + (d <= near || last(&id) == last(c.id)).then_some((d > near, d, c.id)) + }) + .collect(); + found.sort(); + let ids: Vec = found.iter().take(3).map(|(_, _, c)| format!("`{c}`")).collect(); + match ids.as_slice() { + [] => String::new(), + [one] => format!(" (did you mean {one}?)"), + [rest @ .., end] => format!(" (did you mean {} or {end}?)", rest.join(", ")), + } +} + +/// Levenshtein distance: the single-character edits that turn `a` into `b`. +fn edit_distance(a: &str, b: &str) -> usize { + let b: Vec = b.chars().collect(); + let mut row: Vec = (0..=b.len()).collect(); + for (i, ca) in a.chars().enumerate() { + let mut prev = row[0]; + row[0] = i + 1; + for (j, cb) in b.iter().enumerate() { + let next = (prev + usize::from(ca != *cb)).min(row[j] + 1).min(row[j + 1] + 1); + prev = row[j + 1]; + row[j + 1] = next; + } + } + row[b.len()] +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 8eae6d82..52f68c60 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -43,7 +43,7 @@ pub const MAX_LINE: usize = 16 << 20; #[derive(Debug, thiserror::Error)] pub enum EngineError { - #[error("unknown command `{0}`")] + #[error("unknown command `{0}`{hint}", hint = cmd::did_you_mean(.0))] UnknownCommand(String), #[error("`{0}` is not available right now: {1}")] Disabled(String, String), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c2358af5..aeae654b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -837,6 +837,20 @@ fn only_own_ids_name_commands() { } } +/// An unknown command's error names the ids it was probably meant to be: a typo's, or the +/// command that ends in the same word. +#[test] +fn unknown_commands_suggest_the_closest_ids() { + let mut s = Session::default(); + let mut msg = |id: &str| s.execute(id, &serde_json::json!({})).unwrap_err().to_string(); + assert_eq!(msg("file.open"), "unknown command `file.open` (did you mean `doc.open`?)"); + assert!(msg("solid.extrud").starts_with("unknown command `solid.extrud` (did you mean `solid.extrude`"), "{}", msg("solid.extrud")); + assert!(msg("Extrude").contains("`solid.extrude`"), "{}", msg("Extrude")); + assert_eq!(msg("FusionFilletEdgesCommand"), "unknown command `FusionFilletEdgesCommand`"); + let long = "x".repeat(10_000); + assert_eq!(msg(&long), format!("unknown command `{long}`")); +} + #[test] fn sample_design_builds() { let mut s = Session::default(); diff --git a/docs/book/src/dev/commands.md b/docs/book/src/dev/commands.md index d6e20066..45172a8f 100644 --- a/docs/book/src/dev/commands.md +++ b/docs/book/src/dev/commands.md @@ -25,7 +25,8 @@ feature to the timeline and let the document evaluate it; they validate their in (units, ranges, references) and return `BadParams` instead of building a broken feature. Only the registry's own dotted ids name commands (matched ignoring case); any other id fails -with `unknown command`. +with `unknown command`, which names up to three close ids: a typo's (`solid.extrud`), or one +ending in the same word (`file.open`: did you mean `doc.open`?). ## Adding a command