aleleba/designcraft
1
0
Fork 0
mirror of https://github.com/storytold/designcraft.git synced 2026-11-11 16:30:16 -06:00
Code Issues Packages Projects Releases Wiki Activity

Refuse command params the description doesn't name

Browse Source 
Callers from outside the app now get a parameter error for a key a
command's params description doesn't name, instead of the command
running with its defaults: `text.select {story, start, end}` and
`style.list {bogus: 1}` are refused with "unknown parameter `…`" and the
nearest or the accepted keys.

- engine: `cmd::params` (ported from VectorCraft #1442) reads the keys
  from each params description; `check_params` / `check_keys`.
  `Session::execute` itself stays lenient so old journals replay; in the
  engine's own tests it checks too, as the audit of the descriptions.
- MCP: tools/call answers -32602 for an argument a tool doesn't take and
  for params a command doesn't take (`execute`, every `batch` step before
  the first runs); the headless backend checks as well.
- Control channel (`engine.execute`) and `designcraft-cli run --cmd`.
- Descriptions now name every key their commands read (table target
  cells, anchored objects, gap.move, file.place, data merge, prefs.set,
  …); type.char / type.para stay unchecked (their attributes may be the
  params object itself; the setters refuse unknown attributes).
- Tests that passed keys the commands never read are fixed, and the pen
  tool no longer sends the unread `notifyPen`.

Fixes #776
This commit is contained in:
Gert Pellin
2026-10-13 17:25:27 +02:00
parent e862e0a613
commit 68596ea696
28 changed files with 665 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ROADMAP.md
+1
View File
@@ -129,6 +129,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m
Newest first.
- **2026-10-13** · Commands refuse parameter keys their params description doesn't name (#776): the control channel, MCP (`-32602`, every `batch` step checked before the first runs) and `designcraft-cli --cmd` answer with an “unknown parameter” error naming the nearest or the accepted keys instead of running with defaults; journals still replay. Descriptions of the table, anchored-object, data-merge and placing commands now name every key they read. No readiness percentage change.
- **2026-10-12** · macOS Dock Quit uses the same unsaved-document prompts as window and menu Quit. Repeated requests keep the current answers; Cancel retains every tab and unsaved recovery checkpoint, and completed Save/Don't Save answers exit through the ordinary event loop. No readiness percentage change.
- **2026-10-12** · Orderly native exit writes the latest recovery state after finalizing current quit approvals; incomplete or stale plans keep their documents, and Undo to the saved state clears stale recovery. Unavailable or failed recovery is reported. Cancelable macOS Dock Quit remains open (#544). No readiness percentage change.
- **2026-10-12** · Recovered documents keep a checkpoint until saved, closed or reverted, including a second crash before autosave. Restarted document IDs and explicit recovery folders preserve other snapshots; failed republishing retains the original. No readiness percentage change.
apps/designcraft-cli/src/main.rs
+2
View File
@@ -252,6 +252,8 @@ fn run(args: &[String]) -> Result<(), String> {
let (id, p) = c.split_once('=').unwrap_or((&c, "{}"));
let p: Value = serde_json::from_str(p).map_err(|e| format!("--cmd {id}: {e}"))?;
let p = designcraft_engine::script::resolve(&p, &results).map_err(|e| format!("--cmd {id}: {e}"))?;
// A key the command's description doesn't name is an error, not a default (#776).
designcraft_engine::check_params(id, &p).map_err(|e| e.to_string())?;
let r = s.execute(id, &p).map_err(|e| e.to_string())?;
results.push(r.clone());
if !r.is_null() {
apps/designcraft-cli/tests/cli.rs
+16
View File
@@ -67,6 +67,22 @@ fn run_cmd_references_and_describe() {
assert!(v.as_array().unwrap().iter().all(|c| c.to_string().contains("footnote")));
}
/// #776: a key the command doesn't take fails the run (`--cmd`) or the step (`script`) instead of
/// being ignored.
#[test]
fn unknown_params_fail() {
let out = cli().args(["run", "--cmd", r#"style.list={"bogus":1}"#]).output().unwrap();
assert!(!out.status.success());
assert!(String::from_utf8_lossy(&out.stderr).contains("unknown parameter `bogus`; the command takes no parameters"));
let mut child = cli().args(["script", "-"]).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap();
child.stdin.take().unwrap().write_all(b"document.inspect\nfile.new {\"widht\": 300}\n").unwrap();
let out = child.wait_with_output().unwrap();
assert!(!out.status.success());
let v: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
assert_eq!(v["failedIndex"], 1);
assert!(v.to_string().contains("did you mean `width`?"), "{v}");
}
/// `designcraft-cli --help` printed usage to stderr and exited 1; asking for help is not an
/// error (#243). It now goes to stdout and succeeds, while an unknown command keeps the same
/// usage on stderr and fails.
crates/engine/src/cmd/anchored.rs
+2 -2
View File
@@ -18,7 +18,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Insert Anchored Object…",
["Object", "Anchored Object"],
None,
"{ids, story?, pos? (default: the text insertion point), position?: inline|aboveLine|custom, yOffset?, align?: left|center|right, spaceBefore?, spaceAfter?, xRelative?/yRelative?: anchor|textFrame|columnEdge|pageMargin|pageEdge, xOffset?, objectPoint?/refPoint?: 0..8, keepWithinColumn?} — moves the items into the text",
"{ids, story?, pos? (default: the text insertion point), position?: inline|aboveLine|custom, yOffset?, align?: left|center|right, spaceBefore?, spaceAfter?, xRelative? | yRelative?: anchor|textFrame|columnEdge|pageMargin|pageEdge, xOffset?, objectPoint? | refPoint?: 0..8, keepWithinColumn?} — moves the items into the text",
has_doc,
insert
),
@@ -27,7 +27,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Anchored Object Options…",
["Object", "Anchored Object"],
None,
"{story?, index? (default: the object at the insertion point), position?: inline|aboveLine|custom, yOffset?, align?: left|center|right, spaceBefore?, spaceAfter?, xRelative?/yRelative?: anchor|textFrame|columnEdge|pageMargin|pageEdge, xOffset?, objectPoint?/refPoint?: 0..8, keepWithinColumn?}",
"{story?, index? (default: the object at the insertion point), position?: inline|aboveLine|custom, yOffset?, align?: left|center|right, spaceBefore?, spaceAfter?, xRelative? | yRelative?: anchor|textFrame|columnEdge|pageMargin|pageEdge, xOffset?, objectPoint? | refPoint?: 0..8, keepWithinColumn?}",
has_doc,
options
),
crates/engine/src/cmd/datamerge/mod.rs
+2 -2
View File
@@ -44,7 +44,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Data Fields",
[],
None,
"{csv? | rows? | path? | bytes?, name?} → [{name, kind, uses}]; no params reads the linked source and does not attach one",
"{csv? | rows? | path? | bytes? | base64?, name?, delimiter?, sheet?} → [{name, kind, uses}]; no params reads the linked source and does not attach one",
has_doc,
fields_cmd
),
@@ -74,7 +74,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Create Merged Document…",
[],
None,
"{csv? | rows? | path? | bytes?, name?, records?, one?, range?, perPage?, arrange?, insets?, columnSpacing?, rowSpacing?, fitting?, center?, linkImages?, limit?} → {records, pages, missingImages, oversetStories, warnings}",
"{csv? | rows? | path? | bytes? | base64?, name?, delimiter?, sheet?, records?, one?, range?, perPage?, arrange?, insets?, columnSpacing?, rowSpacing?, fitting?, center?, linkImages?, limit?, spread?: removed (passing it is an error)} → {records, pages, missingImages, oversetStories, warnings}",
has_doc,
merge_cmd
),
crates/engine/src/cmd/export.rs
+1 -1
View File
@@ -882,7 +882,7 @@ mod print_group_tests {
fn print_transparency_groups_blend_in_cmyk() {
let mut s = page_with(&logo());
let text = |s: &mut Session| {
let r = s.execute("file.exportPdf", &json!({"compress": false})).unwrap();
let r = s.execute("file.exportPdf", &json!({"compressImages": false})).unwrap();
String::from_utf8_lossy(&base64_decode(r["base64"].as_str().unwrap())).into_owned()
};
let t = text(&mut s);
crates/engine/src/cmd/file.rs
+2 -2
View File
@@ -13,7 +13,7 @@ use crate::{DocState, EngineError, Result, Session};
pub fn specs() -> Vec<CommandSpec> {
vec![
cmd!(noundo "file.new", "Document…", ["File", "New"], Some("Cmd+N"),
"{preset?: \"Letter\"|\"A4\"|…, width?, height?, pages?, facingPages?, columns?, gutter?, margins?: number|{top,bottom,inside,outside}, bleed?, title?}",
"{preset?: \"Letter\"|\"A4\"|…, width?, height?, pages?, facingPages?, columns?, gutter?, margins?: number|{top,bottom,inside,outside}, bleed?, title?, primaryTextFrame?: bool}",
always, file_new),
cmd!(noundo "file.newSample", "Sample Document", ["Help"], None, "{} — a multi-page magazine sample", always, file_sample),
cmd!(query "file.presets", "Document Presets", [], None, "{}", always, |_, _| Ok(serde_json::to_value(PRESETS).unwrap_or_default())),
@@ -102,7 +102,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Place…",
["File"],
Some("Cmd+D"),
"{path?|base64?, name?, frame?: id (place into), spread?, x?, y?, width?, pdfPage?: n (1-based, Image Import Options), pdfCrop?: crop|trim|bleed|art|media, layoutPage?: n (IDML / .designcraft: that page's objects as a group)} — places an image (into the selected empty frame if any); text files (.txt, .docx, .rtf, .md) and Excel workbooks (.xlsx, as a table) go into the insertion point, the selected frame or a new frame on `page`/`rect` — {autoflow?: adds pages with threaded frames until the text fits, removeStyles?, styleMap?: {imported name: document style}, styleConflicts?: useExisting|redefine|autoRename}",
"{path?|base64?, name?, frame?: id (place into), spread?, x?, y?, width?, height? (video/audio), pdfPage?: n (1-based, Image Import Options), pdfCrop?: crop|trim|bleed|art|media, layoutPage?: n (IDML / .designcraft: that page's objects as a group), page?, rect?: [x0,y0,x1,y1] (text: a new frame there), autoflow?: adds pages with threaded frames until the text fits, removeStyles?, styleMap?: {imported name: document style}, styleConflicts?: useExisting|redefine|autoRename} — places an image (into the selected empty frame if any); text files (.txt, .docx, .rtf, .md) and Excel workbooks (.xlsx, as a table) go into the insertion point, the selected frame or a new frame on `page`/`rect`",
has_doc,
file_place
),
crates/engine/src/cmd/find.rs
+2 -2
View File
@@ -25,7 +25,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Change All (Object)",
[],
None,
"{…find.objects criteria, change: {fill?, stroke?, strokeWeight?, opacity?}} → {changed}",
"{…the criteria of find.objects, change: {fill?, stroke?, strokeWeight?, opacity?}} → {changed}",
has_doc,
|s, p| {
let ids = find_objects(&s.doc()?.doc, p);
@@ -65,7 +65,7 @@ pub fn specs() -> Vec<CommandSpec> {
has_doc,
change
),
cmd!(noundo "find.next", "Find Next", ["Edit"], Some("Cmd+Alt+F"), "{find, grep?, …} — selects the next match after the caret", has_doc, find_next),
cmd!(noundo "find.next", "Find Next", ["Edit"], Some("Cmd+Alt+F"), "{find, grep?, caseSensitive?, wholeWord?} — selects the next match after the caret", has_doc, find_next),
]
}
crates/engine/src/cmd/mathexpr.rs
+1 -1
View File
@@ -40,7 +40,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Insert Math Expression…",
["Type"],
None,
"{latex, size?: points (12), x?, y?, spread?} — at the text insertion point (inline), else as a frame; or {id, latex} to edit one",
"{latex, size?: points (12), x?, y?, spread?} or {id, latex, size?} — at the text insertion point (inline), else as a frame; with `id`: edits that equation",
has_doc,
insert
),
crates/engine/src/cmd/mod.rs
+1
View File
@@ -34,6 +34,7 @@ mod notes;
mod object;
mod overrides;
mod package;
pub mod params;
mod path_type;
mod paths;
mod pdflayers;
crates/engine/src/cmd/object.rs
+4 -4
View File
@@ -32,7 +32,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Create Frame",
[],
None,
"{spread?: index | {kind: doc|parent, index} (a parent spread: index into `parents` of document.inspect), rect: [x0,y0,x1,y1] (spread coords), shape?: rectangle|ellipse|polygon, content?: graphic|text|unassigned, sides?: 6, text?: string, caret?: bool, vertical?: bool (text: a new vertical story)}",
"{spread?: index | {kind: doc|parent, index} (a parent spread: index into `parents` of document.inspect), rect: [x0,y0,x1,y1] (spread coords), shape?: rectangle|ellipse|polygon, content?: graphic|text|unassigned, sides?: 6, starInset?: 0–100 (%), text?: string, caret?: bool, vertical?: bool (text: a new vertical story)}",
has_doc,
frame_create
),
@@ -532,7 +532,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Text Frame Options…",
["Object"],
Some("Cmd+B"),
"{columns?, gutter?, inset?: number|[t,l,b,r] (null keeps that side), verticalJustification?: top|center|bottom|justify, firstBaseline?, autoSize?, ignoreWrap?, balanceColumns?, columnRule?: bool, columnRuleWeight? (pt), columnRuleColor? (swatch name), columnRuleTint? (0..1), columnRuleOffset? (pt, horizontal), columnRuleTopInset? (pt), columnRuleBottomInset? (pt), vertical?: bool (sets the story direction of the frames' stories, as Type ▸ Story Direction), ids?}",
"{columns?, gutter?, inset?: number|[t,l,b,r] (null keeps that side), verticalJustification?: top|center|bottom|justify, firstBaseline?, autoSize?, columnsKind?: fixedNumber|fixedWidth|flexibleWidth, ignoreWrap?, balanceColumns?, columnRule?: bool, columnRuleWeight? (pt), columnRuleColor? (swatch name), columnRuleTint? (0..1), columnRuleOffset? (pt, horizontal), columnRuleTopInset? (pt), columnRuleBottomInset? (pt), vertical?: bool (sets the story direction of the frames' stories, as Type ▸ Story Direction), ids?}",
has_selection,
text_frame_options
),
@@ -599,7 +599,7 @@ pub fn specs() -> Vec<CommandSpec> {
has_selection,
fitting_options
),
cmd!("object.rename", "Rename", [], None, "{id, name}", has_doc, |s, p| {
cmd!("object.rename", "Rename", [], None, "{id | ids?, name}", has_doc, |s, p| {
let name = str_param(p, "name").unwrap_or("").to_string();
set_flag(s, p, move |i| i.name = name.clone(), false)
}),
@@ -682,7 +682,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Move Gap",
[],
None,
"{at: [x, y] (spread), delta | dx/dy, spread?} — Gap tool: the gap at `at` between objects (or an object and the page edge) moves by `delta`; the objects on both sides resize → {axis, changed}",
"{at: [x, y] (spread), delta?: pt | dx? | dy?: a drag (the part along the gap's axis counts), spread?} — Gap tool: the gap at `at` between objects (or an object and the page edge) moves by `delta`; the objects on both sides resize → {axis, changed}",
has_doc,
gap_move
),
crates/engine/src/cmd/params.rs
+230
View File
@@ -0,0 +1,230 @@
//! The parameter keys a command accepts, read from its params description (#776).
//!
//! A description starts with the params object: `{x, width?: pt, name?|title?: …, sides=6, …}`, or
//! with the objects a command takes instead of one another: `{path} or {name, dataBase64}`,
//! `{name} | {names: […]}`. Each entry of an object starts with a key, or with alternatives split
//! by `|`; a `?` marks an optional one, and a `:`, `=` or `(` starts what the entry says about it.
//! An entry that starts with `…` stands for keys listed elsewhere: those of each command it names
//! after `as` or `of` (`…settings as document.new`), or, naming none, any key. Objects inside a
//! value aren't described key by key, so only the top level is checked.
//!
//! Callers outside the app (the control channel, MCP, the CLI) are answered with a parameter error
//! for a key the description doesn't name ([`check_params`]): a misspelt `widht` no longer runs the
//! command with its default. [`Session::execute`](crate::Session::execute) itself takes what it is
//! given, so the journal of an older version still replays.
//!
//! Ported from VectorCraft (storytold/vectorcraft#1442).
use serde_json::Value;
use super::{bad, find_command};
use crate::Result;
/// What a params description says about the keys of the params object.
#[derive(Debug, Default, PartialEq)]
pub struct ParamKeys<'a> {
/// The keys it names.
pub keys: Vec<&'a str>,
/// Its `…` entries, without the `…`.
pub more: Vec<&'a str>,
}
/// The text between the braces of the object `doc` starts with and what follows it, or why it
/// has none.
fn object_body(doc: &str) -> std::result::Result<(&str, &str), String> {
// (Named, not written in the `Err`: these are for the tests of the descriptions, not messages
// the app shows, which the message catalogs would have to translate.)
let (no_object, not_closed) = ("the description doesn't start with the params object `{…}`", "the params object `{…}` isn't closed");
if !doc.starts_with('{') {
return Err(no_object.into());
}
let (mut depth, mut quote) = (0usize, None);
for (i, c) in doc.char_indices() {
match (quote, c) {
(Some(q), _) if c == q => quote = None,
(Some(_), _) => {}
(None, '"' | '`') => quote = Some(c),
(None, '{' | '[' | '(') => depth += 1,
(None, '}' | ']' | ')') => {
depth = depth.saturating_sub(1);
if depth == 0 {
return Ok((doc.get(1..i).unwrap_or_default(), doc.get(i + 1..).unwrap_or_default()));
}
}
_ => {}
}
}
Err(not_closed.into())
}
/// `s` split at each `sep` that is outside brackets, quotes and backticks.
fn split_top(s: &str, sep: char) -> Vec<&str> {
let (mut out, mut depth, mut quote, mut start) = (Vec::new(), 0usize, None, 0);
for (i, c) in s.char_indices() {
match (quote, c) {
(Some(q), _) if c == q => quote = None,
(Some(_), _) => {}
(None, '"' | '`') => quote = Some(c),
(None, '{' | '[' | '(') => depth += 1,
(None, '}' | ']' | ')') => depth = depth.saturating_sub(1),
(None, _) if c == sep && depth == 0 => {
out.push(s.get(start..i).unwrap_or_default());
start = i + c.len_utf8();
}
_ => {}
}
}
out.push(s.get(start..).unwrap_or_default());
out
}
fn is_key(s: &str) -> bool {
let mut chars = s.chars();
chars.next().is_some_and(|c| c.is_ascii_alphabetic() || c == '_') && chars.all(|c| c.is_ascii_alphanumeric() || c == '_')
}
/// The keys a params description names, or what in it doesn't read as one.
pub fn doc_keys(doc: &str) -> std::result::Result<ParamKeys<'_>, String> {
let mut out = ParamKeys::default();
let (mut body, mut rest) = object_body(doc)?;
let mut entries = split_top(body, ',');
// `{…} or {…}`, `{…} | {…}`: the objects the command takes instead.
while let Some(next) = rest.trim_start().strip_prefix("or ").or_else(|| rest.trim_start().strip_prefix("| ")).filter(|r| r.starts_with('{')) {
(body, rest) = object_body(next)?;
entries.extend(split_top(body, ','));
}
for entry in entries.into_iter().map(str::trim).filter(|e| !e.is_empty()) {
if let Some(more) = entry.strip_prefix('…') {
out.more.push(more.trim());
continue;
}
// `a?|b?: note` names two keys, and so does `a?: note | b?: note`; in `a?: x|y` the `y`
// is a value.
let mut noted = false;
for alternative in split_top(entry, '|') {
let head = alternative.split([':', '=', '(']).next().unwrap_or_default();
let rest = alternative.get(head.len()..).unwrap_or_default();
let name = head.trim().trim_end_matches('?').trim_end();
if !noted {
if !is_key(name) {
let start: String = entry.chars().take(60).collect();
let why = format!("`{start}` doesn't start with a parameter name");
return Err(why);
}
out.keys.push(name);
noted = !rest.is_empty();
} else if is_key(name) && (head.trim_end().ends_with('?') || rest.starts_with(':')) {
out.keys.push(name);
}
}
}
Ok(out)
}
/// The commands whose keys a `…` entry stands for: those it names after `as` or `of` (`settings as
/// file.new`, `the PDF options of document.exportPdf`), not one it only points to (`see …`).
fn named_commands(more: &str) -> impl Iterator<Item = &'static super::CommandSpec> + '_ {
let words: Vec<&str> = more.split_whitespace().collect();
let id = |w: &str| find_command(w.trim_matches(|c: char| !c.is_ascii_alphanumeric()));
(1..words.len()).filter_map(move |i| id(words.get(i)?).filter(|_| matches!(words.get(i - 1), Some(&"as" | &"of"))))
}
/// Every key command `doc` accepts, the keys of the commands its `…` entries name included.
/// `None`: any key (a `…` entry naming no command, or a description that doesn't parse, which
/// the tests rule out).
fn accepted(doc: &'static str, depth: usize) -> Option<Vec<&'static str>> {
let parsed = doc_keys(doc).ok()?;
let mut keys = parsed.keys;
// A command that acts on `ids` takes one object as `id` too (see `cmd::targets`).
if keys.contains(&"ids") && !keys.contains(&"id") {
keys.push("id");
}
for more in parsed.more {
let mut named = named_commands(more).peekable();
// A reference that leads back here would never end: descriptions refer one or two deep.
if named.peek().is_none() || depth > 4 {
return None;
}
for spec in named {
// A command that isn't checked yet doesn't vouch for the ones that take its keys.
if UNCHECKED.contains(&spec.id) {
return None;
}
keys.extend(accepted(spec.params, depth + 1)?);
}
}
Some(keys)
}
/// The edits (insert, delete, replace, swap of neighbours) that turn `a` into `b`, ignoring case.
fn distance(a: &str, b: &str) -> usize {
let (a, b): (Vec<char>, Vec<char>) = (a.chars().map(|c| c.to_ascii_lowercase()).collect(), b.chars().map(|c| c.to_ascii_lowercase()).collect());
let (mut before, mut prev): (Vec<usize>, Vec<usize>) = (Vec::new(), (0..=b.len()).collect());
for (i, ca) in a.iter().enumerate() {
let mut row = vec![i + 1];
for (j, cb) in b.iter().enumerate() {
let at = |r: &[usize], k: usize| r.get(k).copied().unwrap_or(usize::MAX / 2);
let mut d = (at(&prev, j) + usize::from(ca != cb)).min(at(&prev, j + 1) + 1).min(at(&row, j) + 1);
if i > 0 && j > 0 && a.get(i - 1) == Some(cb) && b.get(j - 1) == Some(ca) {
d = d.min(at(&before, j - 1) + 1);
}
row.push(d);
}
before = std::mem::replace(&mut prev, row);
}
prev.last().copied().unwrap_or(0)
}
/// The key of `keys` a misspelt `key` most likely meant: the nearest one, when it is near.
fn nearest<'a>(key: &str, keys: &[&'a str]) -> Option<&'a str> {
let near = keys.iter().map(|k| (distance(key, k), *k)).min()?;
(near.0 <= (key.chars().count() / 3).max(1)).then_some(near.1)
}
/// `params` has only keys the description `doc` of command `id` names, or the parameter error
/// naming the first one it doesn't, with the nearest accepted key or the accepted ones.
pub fn check_keys(id: &str, doc: &'static str, params: &Value) -> Result<()> {
let Value::Object(given) = params else { return Ok(()) };
if given.is_empty() {
return Ok(());
}
let Some(keys) = accepted(doc, 0) else { return Ok(()) };
match given.keys().find(|k| !keys.contains(&k.as_str())) {
Some(key) => Err(unknown(id, key, &keys)),
None => Ok(()),
}
}
/// The parameter error of command `id` for `key`, which isn't one of `keys`: with the nearest of
/// them, or all of them.
fn unknown(id: &str, key: &str, keys: &[&str]) -> crate::EngineError {
bad(
id,
match nearest(key, keys) {
Some(near) => format!("unknown parameter `{key}`; did you mean `{near}`?"),
None if keys.is_empty() => format!("unknown parameter `{key}`; the command takes no parameters"),
None => format!("unknown parameter `{key}`; the command takes: {}", keys.join(", ")),
},
)
}
/// Commands whose params aren't checked yet (#776): they read keys in ways the audit of the
/// descriptions couldn't follow (a list of keys in a loop, a whole object read as settings), so
/// their descriptions aren't proven to name every key they take. Sorted; take a command out once
/// its description is checked against its code.
pub const UNCHECKED: &[&str] = &[
// Take the attributes under `attrs` or as the params object itself; the attribute setters
// refuse a key they don't know.
"type.char",
"type.para",
];
/// [`check_keys`] for an engine command. A command the engine doesn't have passes: running it
/// reports that.
pub fn check_params(id: &str, params: &Value) -> Result<()> {
let Some(spec) = find_command(id) else { return Ok(()) };
if UNCHECKED.contains(&id) {
return Ok(());
}
check_keys(id, spec.params, params)
}
crates/engine/src/cmd/prefs.rs
+1 -1
View File
@@ -48,7 +48,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Preferences",
[],
None,
"{showHiddenCharacters?, typographersQuotes?, polygonSides?, starInset?, scaleStrokes?, dimensionsIncludeStroke?, transformationsAreTotals?, absolutePageNumbers?, highlightHj?, highlightKeeps?, highlightCustomTracking?, highlightSubstitutedFonts?, richBlackOutput?, favoriteFonts?: [family], showFontNamesInEnglish?} → all application preferences",
"{showHiddenCharacters?, typographersQuotes?, polygonSides?, starInset?, scaleStrokes?, dimensionsIncludeStroke?, transformationsAreTotals?, absolutePageNumbers?, highlightHj?, highlightKeeps?, highlightCustomTracking?, highlightSubstitutedFonts?, richBlackOutput?, favoriteFonts?: [family], recentFonts?: [family], showFontNamesInEnglish?, smartTextReflow?, autocorrect?, autocorrectList?: [[word, correction]], showAddedText?, showNoteAnchors?, recoveryMinutes?} → all application preferences",
super::always,
|s, p| {
let cur = serde_json::to_value(&s.prefs).map_err(|e| bad("prefs.set", e.to_string()))?;
crates/engine/src/cmd/style.rs
+18 -5
View File
@@ -1260,7 +1260,12 @@ mod color_tests {
let mut session = fill_session();
session.execute("style.paragraph.apply", &json!({"name": "Existing"})).unwrap();
session.execute("style.character.apply", &json!({"name": "Existing"})).unwrap();
assert_fill_rejected(&mut session, command, json!({"name": "Existing", "rename": "Renamed", "chars": {"fill": fill, "size": 24}}));
let mut params = json!({"name": "Existing", "chars": {"fill": fill, "size": 24}});
// Only the edit commands take `rename`.
if command.ends_with(".edit") {
params["rename"] = json!("Renamed");
}
assert_fill_rejected(&mut session, command, params);
}
}
}
@@ -1611,7 +1616,7 @@ mod export_tag_tests {
assert!(html.contains("<h1 class=\"chapter\">Origins</h1>"), "{html}");
assert!(html.contains("<strong class=\"key-term\">word</strong>"), "{html}");
// Tagged PDF: a heading element and a paragraph element.
let r = s.execute("file.exportPdf", &json!({"tagged": true, "compress": false})).unwrap();
let r = s.execute("file.exportPdf", &json!({"tagged": true, "compressImages": false})).unwrap();
let pdf = super::super::file::base64_decode(r["base64"].as_str().unwrap());
let text = String::from_utf8_lossy(&pdf);
assert!(text.contains("/S/H1") && text.contains("/S/P"), "structure elements");
@@ -1728,12 +1733,20 @@ mod sample_color_tests {
#[test]
fn sampling_and_explicit_colour_use_the_same_text_application_path() {
for command in ["color.sample", "object.color"] {
// Each command gets its own key: the eyedropper samples at `at`, object.color takes `color`.
let params = |at: [f64; 2], color: &str, extra: serde_json::Value| {
let mut p = if command == "color.sample" { json!({"at": at}) } else { json!({"color": color}) };
if let (Some(p), Some(extra)) = (p.as_object_mut(), extra.as_object()) {
p.extend(extra.clone());
}
p
};
let (mut s, _) = session_with_image();
let f = s.execute("frame.create", &json!({"rect": [300, 300, 500, 360], "content": "text"})).unwrap();
let story = designcraft_doc::StoryId(f["story"].as_u64().unwrap());
s.execute("text.select", &json!({"story": story.0, "anchor": 0})).unwrap();
let undo = s.doc().unwrap().history.undo.len();
let r = s.execute(command, &json!({"at": [100, 100], "color": "#ff0000"})).unwrap();
let r = s.execute(command, &params([100.0, 100.0], "#ff0000", json!({}))).unwrap();
assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1, "{command}");
s.execute("text.insert", &json!({"text": "Hi"})).unwrap();
let text = s.doc().unwrap().doc.story(story).unwrap();
@@ -1741,11 +1754,11 @@ mod sample_color_tests {
// Explicit ids override text targeting, including with a text selection active.
let fid = ItemId(f["id"].as_u64().unwrap());
s.execute("text.select", &json!({"story": story.0, "anchor": 0, "focus": 2})).unwrap();
let r = s.execute(command, &json!({"at": [180, 100], "color": "#0000ff", "id": fid.0})).unwrap();
let r = s.execute(command, &params([180.0, 100.0], "#0000ff", json!({"id": fid.0}))).unwrap();
assert_eq!(s.doc().unwrap().doc.item(fid).unwrap().fill.swatch, r["swatch"].as_str().unwrap());
let text = s.doc().unwrap().doc.story(story).unwrap();
assert_ne!(text.format_after(0).over.fill.as_deref(), r["swatch"].as_str(), "text unchanged: {command}");
let r = s.execute(command, &json!({"at": [180, 100], "color": "#0000ff", "target": "stroke"})).unwrap();
let r = s.execute(command, &params([180.0, 100.0], "#0000ff", json!({"target": "stroke"}))).unwrap();
let text = s.doc().unwrap().doc.story(story).unwrap();
assert_eq!(text.format_after(0).over.stroke.as_deref(), r["swatch"].as_str(), "selected text stroke: {command}");
}
crates/engine/src/cmd/table.rs
+164 -38
View File
@@ -29,7 +29,7 @@ fn style_specs() -> Vec<CommandSpec> {
"New Cell Style…",
[],
None,
"{name, fromSelection?: bool (the target cell's look), fill?, tint?, insets?: n|[t,l,b,r], vj?: top|center|bottom|justify, stroke?: {weight, color, tint?}, paragraphStyle?} → {name}",
"{name, fromSelection?: bool (the target cell's look), fill?, tint?, insets?: n|[t,l,b,r], vj?: top|center|bottom|justify, stroke?: {weight, color, tint?}, paragraphStyle?, basedOn?: cell style name, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} → {name}",
has_doc,
|s, p| cell_style_create(s, p)
),
@@ -38,7 +38,7 @@ fn style_specs() -> Vec<CommandSpec> {
"Apply Cell Style",
[],
None,
"{name, story?, table?} — the target cells (the whole table when the cursor is in it)",
"{name, story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?} — the target cells (the whole table when the cursor is in it)",
in_table,
cell_style_apply
),
@@ -56,11 +56,19 @@ fn style_specs() -> Vec<CommandSpec> {
"New Table Style…",
[],
None,
"{name, header?, body?, footer?, leftColumn?, rightColumn?: cell style names, border?: {weight, color}, altRows?: {first, firstColor, next, nextColor}, spaceBefore?, spaceAfter?} → {name}",
"{name, basedOn?: table style name, header?, body?, footer?, leftColumn?, rightColumn?: cell style names, border?: {weight, color}, altRows?: {first, firstColor, next, nextColor}, spaceBefore?, spaceAfter?} → {name}",
has_doc,
table_style_create
),
cmd!("style.table.apply", "Apply Table Style", [], None, "{name, story?, table?}", in_table, table_style_apply),
cmd!(
"style.table.apply",
"Apply Table Style",
[],
None,
"{name, story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?}",
in_table,
table_style_apply
),
cmd!(
"style.table.edit",
"Table Style Options…",
@@ -80,7 +88,7 @@ fn table_specs() -> Vec<CommandSpec> {
"Create Table",
[],
Some("Cmd+Alt+Shift+T"),
"{rows?: body rows (4), cols? (4), headerRows? (0), footerRows? (0), width?} — at the text insertion point",
"{rows?: body rows (4), cols? | columns? (4), headerRows? (0), footerRows? (0), width?} — at the text insertion point",
has_text,
insert
),
@@ -89,7 +97,7 @@ fn table_specs() -> Vec<CommandSpec> {
"Convert Text to Table",
["Table"],
None,
"{columnSeparator?: tab|comma} — selected paragraphs become rows",
"{columnSeparator?: tab|comma, width?: table width (default: the column)} — selected paragraphs become rows",
has_text,
convert_from_text
),
@@ -98,20 +106,100 @@ fn table_specs() -> Vec<CommandSpec> {
"Convert Table to Text",
["Table"],
None,
"{} — cells separated by tabs, rows by paragraphs",
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — cells separated by tabs, rows by paragraphs",
in_table,
convert_to_text
),
cmd!("table.insertRow", "Insert Row", [], None, "{where?: above|below, count? (1)}", in_table, |s, p| insert_row(s, p, None)),
cmd!("table.insertRowAbove", "Insert Row Above", ["Table", "Insert"], None, "{count?}", in_table, |s, p| insert_row(s, p, Some(true))),
cmd!("table.insertRowBelow", "Insert Row Below", ["Table", "Insert"], None, "{count?}", in_table, |s, p| insert_row(s, p, Some(false))),
cmd!("table.insertColumn", "Insert Column", [], None, "{where?: left|right, count? (1), width?}", in_table, |s, p| insert_col(s, p, None)),
cmd!("table.insertColumnLeft", "Insert Column Left", ["Table", "Insert"], None, "{count?}", in_table, |s, p| insert_col(s, p, Some(true))),
cmd!("table.insertColumnRight", "Insert Column Right", ["Table", "Insert"], None, "{count?}", in_table, |s, p| insert_col(s, p, Some(false))),
cmd!("table.deleteRow", "Delete Row", ["Table", "Delete"], None, "{} — the rows of the target cells", in_table, delete_rows),
cmd!("table.deleteColumn", "Delete Column", ["Table", "Delete"], None, "{} — the columns of the target cells", in_table, delete_cols),
cmd!("table.delete", "Delete Table", ["Table", "Delete"], None, "{}", in_table, delete_table),
cmd!("table.merge", "Merge Cells", ["Table"], None, "{} — merge the target cell range", in_table, merge),
cmd!(
"table.insertRow",
"Insert Row",
[],
None,
"{where?: above|below, count? (1), story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
|s, p| insert_row(s, p, None)
),
cmd!(
"table.insertRowAbove",
"Insert Row Above",
["Table", "Insert"],
None,
"{count?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
|s, p| insert_row(s, p, Some(true))
),
cmd!(
"table.insertRowBelow",
"Insert Row Below",
["Table", "Insert"],
None,
"{count?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
|s, p| insert_row(s, p, Some(false))
),
cmd!(
"table.insertColumn",
"Insert Column",
[],
None,
"{where?: left|right, count? (1), width?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
|s, p| insert_col(s, p, None)
),
cmd!(
"table.insertColumnLeft",
"Insert Column Left",
["Table", "Insert"],
None,
"{count?, width?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
|s, p| insert_col(s, p, Some(true))
),
cmd!(
"table.insertColumnRight",
"Insert Column Right",
["Table", "Insert"],
None,
"{count?, width?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
|s, p| insert_col(s, p, Some(false))
),
cmd!(
"table.deleteRow",
"Delete Row",
["Table", "Delete"],
None,
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — the rows of the target cells",
in_table,
delete_rows
),
cmd!(
"table.deleteColumn",
"Delete Column",
["Table", "Delete"],
None,
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — the columns of the target cells",
in_table,
delete_cols
),
cmd!(
"table.delete",
"Delete Table",
["Table", "Delete"],
None,
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
delete_table
),
cmd!(
"table.merge",
"Merge Cells",
["Table"],
None,
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — merge the target cell range",
in_table,
merge
),
cmd!(
"table.dropCells",
"Drag Rows or Columns",
@@ -126,7 +214,7 @@ fn table_specs() -> Vec<CommandSpec> {
"Convert Cell to Graphic Cell",
["Table", "Convert Cell Type"],
None,
"{path | base64+name, fit?: proportional|fill (default proportional)} — an image in the target cell (its text is kept but hidden)",
"{path? | base64?, name?: with base64, fit?: proportional|fill (default proportional), story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — an image in the target cell (its text is kept but hidden)",
in_table,
place_graphic
),
@@ -135,7 +223,7 @@ fn table_specs() -> Vec<CommandSpec> {
"Convert Cell to Text Cell",
["Table", "Convert Cell Type"],
None,
"{} — drop the target cells' graphics",
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — drop the target cells' graphics",
in_table,
|s, p| {
let g = target(s, p, "table.textCell")?;
@@ -156,29 +244,43 @@ fn table_specs() -> Vec<CommandSpec> {
"Move Row",
[],
None,
"{from?, to} (0-based; from defaults to the target row) — like dragging a row",
"{from?, to, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} (0-based; from defaults to the target row) — like dragging a row",
in_table,
|s, p| move_rc(s, p, true)
),
cmd!("table.moveColumn", "Move Column", [], None, "{from?, to} (0-based; from defaults to the target column)", in_table, |s, p| move_rc(
s, p, false
)),
cmd!(
"table.moveColumn",
"Move Column",
[],
None,
"{from?, to, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} (0-based; from defaults to the target column)",
in_table,
|s, p| move_rc(s, p, false)
),
cmd!(
"table.sortRows",
"Sort",
["Table"],
None,
"{column?: 0-based (default: the target cell's), descending?: bool} — body rows by that column's text (numbers numerically)",
"{column?: 0-based (default: the target cell's), descending?: bool, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — body rows by that column's text (numbers numerically)",
in_table,
sort_rows
),
cmd!("table.unmerge", "Unmerge Cells", ["Table"], None, "{}", in_table, unmerge),
cmd!(
"table.unmerge",
"Unmerge Cells",
["Table"],
None,
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
unmerge
),
cmd!(
"table.splitHorizontally",
"Split Cell Horizontally",
["Table"],
None,
"{} — each target cell becomes two, one above the other",
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — each target cell becomes two, one above the other",
in_table,
|s, p| split(s, p, true)
),
@@ -187,7 +289,7 @@ fn table_specs() -> Vec<CommandSpec> {
"Split Cell Vertically",
["Table"],
None,
"{} — each target cell becomes two, side by side",
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — each target cell becomes two, side by side",
in_table,
|s, p| split(s, p, false)
),
@@ -196,19 +298,43 @@ fn table_specs() -> Vec<CommandSpec> {
"Cell Options",
[],
None,
"{fill?: swatch, tint?, insets?: n | [t,l,b,r], vj?: top|center|bottom|justify, text?, stroke?: {weight?, color?, tint?, type?: solid|dashed|dotted, edges?: all|outer|inner|top|left|bottom|right}}",
"{fill?: swatch, tint?, insets?: n | [t,l,b,r], vj? | verticalJustification?: top|center|bottom|justify, text?, stroke?: {weight?, color?, tint?, type?: solid|dashed|dotted, edges?: all|outer|inner|top|left|bottom|right}, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
set_cell
),
cmd!("table.setRowHeight", "Row Height", [], None, "{height, mode?: atLeast|exactly}", in_table, set_row_height),
cmd!("table.setColumnWidth", "Column Width", [], None, "{width}", in_table, set_col_width),
cmd!("table.distributeColumns", "Distribute Columns Evenly", ["Table"], None, "{}", in_table, distribute_cols),
cmd!(
"table.setRowHeight",
"Row Height",
[],
None,
"{height, mode?: atLeast|exactly, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
set_row_height
),
cmd!(
"table.setColumnWidth",
"Column Width",
[],
None,
"{width, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
set_col_width
),
cmd!(
"table.distributeColumns",
"Distribute Columns Evenly",
["Table"],
None,
"{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
distribute_cols
),
cmd!(
"table.distributeRows",
"Distribute Rows Evenly",
["Table"],
None,
"{story?, table?, rows?: [a,b]} — equal fixed heights preserving the selected rows' composed total height; may overset cell text; unmerge row-spanning cells first",
"{story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?} — equal fixed heights preserving the selected rows' composed total height; may overset cell text; unmerge row-spanning cells first",
in_table,
distribute_rows
),
@@ -217,23 +343,23 @@ fn table_specs() -> Vec<CommandSpec> {
"Table Options",
[],
None,
"{direction?: leftToRight|rightToLeft, border?: {weight?, color?, tint?, type?}, spaceBefore?, spaceAfter?, headerRows?, footerRows?, repeatHeader?, repeatFooter?, altRows?: {first, firstColor, firstTint, next, nextColor, nextTint, skipFirst, skipLast} | null, altCols?: … | null}",
"{direction?: leftToRight|rightToLeft, border?: {weight?, color?, tint?, type?}, spaceBefore?, spaceAfter?, headerRows?, footerRows?, repeatHeader?, repeatFooter?, altRows?: {first, firstColor, firstTint, next, nextColor, nextTint, skipFirst, skipLast} | null, altCols?: … | null, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}",
in_table,
options
),
cmd!(noundo "table.select", "Select Cells", [], None, "{story?, table?, rows?: [a,b], cols?: [a,b], what?: cell|row|column|table}", in_table_or_ids, select),
cmd!(noundo "table.selectTable", "Select Table", ["Table", "Select"], None, "{}", in_table, |s, p| {
cmd!(noundo "table.select", "Select Cells", [], None, "{story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?, what?: cell|row|column|table}", in_table_or_ids, select),
cmd!(noundo "table.selectTable", "Select Table", ["Table", "Select"], None, "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", in_table, |s, p| {
select(s, &super::with_param(p, "what", json!("table")))
}),
cmd!(noundo "table.selectRow", "Select Row", ["Table", "Select"], None, "{}", in_table, |s, p| {
cmd!(noundo "table.selectRow", "Select Row", ["Table", "Select"], None, "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", in_table, |s, p| {
select(s, &super::with_param(p, "what", json!("row")))
}),
cmd!(noundo "table.selectColumn", "Select Column", ["Table", "Select"], None, "{}", in_table, |s, p| {
cmd!(noundo "table.selectColumn", "Select Column", ["Table", "Select"], None, "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", in_table, |s, p| {
select(s, &super::with_param(p, "what", json!("column")))
}),
cmd!(noundo "table.nextCell", "Next Cell", [], None, "{}", in_cell, |s, _| step_cell(s, true)),
cmd!(noundo "table.prevCell", "Previous Cell", [], None, "{}", in_cell, |s, _| step_cell(s, false)),
cmd!(query "table.get", "Get Table", [], None, "{story?, table?} → rows, columns, cells (text, spans, fill), options", in_table_or_ids, get),
cmd!(query "table.get", "Get Table", [], None, "{story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?} → rows, columns, cells (text, spans, fill), options", in_table_or_ids, get),
]
}
crates/engine/src/cmd/text.rs
+2 -2
View File
@@ -68,7 +68,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Story Column Direction",
["Type", "Story Column Direction"],
None,
"{story?, direction: leftToRight|rightToLeft} — column progression, independent of paragraph direction",
"{story?, frame?: a text frame of the story, direction: leftToRight|rightToLeft} — column progression, independent of paragraph direction",
has_doc,
|s, p| {
let sid = story_of(s, p).ok_or_else(|| bad("story.setDirection", "no story"))?;
@@ -129,7 +129,7 @@ pub fn specs() -> Vec<CommandSpec> {
v["lines"] = json!(cs.line_count());
Ok(v)
}),
cmd!("type.fillWithPlaceholder", "Fill with Placeholder Text", ["Type"], None, "{frame?}", has_text_or_frames, fill_placeholder),
cmd!("type.fillWithPlaceholder", "Fill with Placeholder Text", ["Type"], None, "{frame?, story?}", has_text_or_frames, fill_placeholder),
cmd!(
"type.char",
"Character Formatting",
crates/engine/src/cmd/xref.rs
+1 -1
View File
@@ -13,7 +13,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Insert Cross-Reference…",
["Type", "Hyperlinks & Cross-References"],
None,
"{anchor? | paragraph?: text to find (first paragraph containing it) | story? + para? (0-based), format?: name (default Full Paragraph & Page Number)} — at the insertion point; the text updates itself as the destination moves",
"{anchor? | paragraph?: text to find (first paragraph containing it) | story?: with para? | para?: 0-based paragraph of `story`, format?: name (default Full Paragraph & Page Number)} — at the insertion point; the text updates itself as the destination moves",
in_story_text,
insert
),
crates/engine/src/lib.rs
+7
View File
@@ -25,6 +25,7 @@ use designcraft_compose::Cache;
use designcraft_doc::{Document, LayerId, Selection};
use serde_json::Value;
pub use cmd::params::{check_keys, check_params};
pub use cmd::{CommandInfo, CommandSpec, command_specs, find_command};
pub use designcraft_compose as compose;
pub use designcraft_doc as doc;
@@ -402,6 +403,10 @@ impl Session {
fn execute_unguarded(&mut self, id: &str, params: &Value) -> Result<Value> {
let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.into()))?;
// The crate's own tests are the audit of the params descriptions (#776): a key a test
// passes that its command's description doesn't name fails the test.
#[cfg(test)]
check_params(id, params)?;
if let Err(e) = (spec.enabled)(self) {
// A command that takes `ids` acts on the objects the call names, so it needs no
// selection: with a text caret or nothing selected, `{"ids": [5]}` still runs.
@@ -561,4 +566,6 @@ mod tests;
#[cfg(test)]
mod tests_idml;
#[cfg(test)]
mod tests_params;
#[cfg(test)]
mod tests_table;
crates/engine/src/tests_params.rs
+106
View File
@@ -0,0 +1,106 @@
//! The params descriptions name the keys their commands accept (#776).
use serde_json::json;
use crate::cmd::params::{UNCHECKED, check_keys, check_params, doc_keys};
use crate::{EngineError, Session, command_specs, find_command};
fn keys(doc: &str) -> Vec<&str> {
doc_keys(doc).unwrap().keys
}
#[test]
fn a_description_names_its_keys() {
assert_eq!(keys("{} → {id}"), Vec::<&str>::new());
assert_eq!(
keys("{x, y, width?: pt=612|\"210 mm\", sides=6, name?|title?: a name (default: Untitled-N)} → {index}"),
["x", "y", "width", "sides", "name", "title"]
);
// Brackets, quotes and backticks keep their commas and bars; a nested object isn't listed.
assert_eq!(
keys("{at: [x, y] (page, not view), stops: [{position: pt, leader?: \", \"}], csv?: a `|, ` in a label is a break, on?: bool}"),
["at", "stops", "csv", "on"]
);
// Alternatives after a note are keys when they have their own `?` or `:`; else values.
assert_eq!(
keys("{steps?: n | distance?: pt | smooth?: bool, spacing?: smooth|steps|distance, path? | data?: text | dataBase64?}"),
["steps", "distance", "smooth", "spacing", "path", "data", "dataBase64"]
);
assert_eq!(keys("{latex, size?} or {id, latex}"), ["latex", "size", "id", "latex"]);
let more = doc_keys("{name, …same as style.cell.create, …the format's options}").unwrap();
assert_eq!((more.keys, more.more), (vec!["name"], vec!["same as style.cell.create", "the format's options"]));
for bad in ["same as paint.setFill", "{a, b", "{opacity? 0..1}", "{path | name+dataBase64}", "{xRelative?/yRelative?: anchor}"] {
assert!(doc_keys(bad).is_err(), "{bad}");
}
}
/// Every command's description reads as a params object, so none is left unchecked by accident.
#[test]
fn every_description_parses() {
let bad: Vec<String> = command_specs().iter().filter_map(|c| doc_keys(c.params).err().map(|e| format!("{}: {e}", c.id))).collect();
assert!(bad.is_empty(), "{} descriptions don't parse:\n{}", bad.len(), bad.join("\n"));
let stale: Vec<_> = UNCHECKED.iter().filter(|id| find_command(id).is_none()).collect();
assert!(stale.is_empty() && UNCHECKED.is_sorted(), "UNCHECKED: sorted ids of commands; not commands: {stale:?}");
}
fn refusal(id: &str, params: serde_json::Value) -> String {
match check_params(id, &params) {
Err(EngineError::BadParams { cmd, msg }) if cmd == id => msg,
other => panic!("{id} {params}: {other:?}"),
}
}
/// The examples of #776: both ran and did nothing (or the default) without a word.
#[test]
fn the_issue_examples_are_refused() {
// (The keys are checked in order: `end` comes before `start`.)
assert_eq!(refusal("text.select", json!({"story": 1, "start": 0, "end": 5})), "unknown parameter `end`; the command takes: story, anchor, focus");
assert_eq!(refusal("text.select", json!({"story": 1, "start": 0})), "unknown parameter `start`; the command takes: story, anchor, focus");
assert_eq!(refusal("style.list", json!({"bogus": 1})), "unknown parameter `bogus`; the command takes no parameters");
}
#[test]
fn an_unknown_key_is_named_with_the_nearest_accepted_one() {
assert_eq!(refusal("file.new", json!({"widht": 300})), "unknown parameter `widht`; did you mean `width`?");
assert_eq!(refusal("file.new", json!({"Width": 300})), "unknown parameter `Width`; did you mean `width`?");
assert_eq!(
refusal("line.create", json!({"a": [0, 0], "b": [5, 5], "colour": "red"})),
"unknown parameter `colour`; the command takes: spread, a, b"
);
for (id, ok) in [
("file.new", json!({"width": 300, "title": "a", "margins": {"anything": 1}})),
("file.new", json!({})),
("file.new", json!(null)),
// A command that acts on `ids` takes one object as `id` too.
("transform.move", json!({"dx": 1, "dy": 1, "id": 3})),
// Keys listed by reference.
("style.cell.edit", json!({"name": "a", "fill": "Paper"})),
// On UNCHECKED: the attributes may be the params object itself.
("type.char", json!({"size": 12})),
// Not a command: running it says so.
("no.such", json!({"x": 1})),
] {
assert!(check_params(id, &ok).is_ok(), "{id} {ok}");
}
assert!(check_keys("x", "{a, …same as file.new}", &json!({"a": 1, "width": 2})).is_ok());
assert!(check_keys("x", "{a, …same as file.new}", &json!({"b": 1})).is_err());
// A `…` naming no command stands for any key.
assert!(check_keys("x", "{a, …the format's options}", &json!({"b": 1})).is_ok());
}
/// [`Session::execute`] itself takes what it is given (old journals replay); only callers from
/// outside (control channel, MCP, CLI) are checked. In this crate's tests it checks too, as the
/// audit of the descriptions, so this goes through a journal entry instead.
#[test]
fn journal_entries_pass_the_check() {
let mut s = Session::new();
s.execute("file.new", &json!({})).unwrap();
let f = s.execute("frame.create", &json!({"rect": [36, 36, 300, 200], "content": "text", "text": "Hello world"})).unwrap();
s.execute("text.select", &json!({"story": f["story"], "anchor": 0, "focus": 5})).unwrap();
s.execute("type.char", &json!({"attrs": {"size": 24}})).unwrap();
s.execute("transform.move", &json!({"dx": 10, "dy": 0, "ids": [f["id"]]})).unwrap();
assert!(s.journal.len() >= 4);
for (id, params) in &s.journal {
assert!(check_params(id, params).is_ok(), "{id} {params}");
}
}
crates/engine/tests/param_keys.rs
+14
View File
@@ -0,0 +1,14 @@
//! #776: callers from outside are checked with `check_params`; `Session::execute` itself takes
//! what it is given, so a journal written by an older version still replays.
use designcraft_engine::{Session, check_params};
use serde_json::json;
#[test]
fn execute_replays_what_the_check_refuses() {
let mut s = Session::new();
s.execute("file.new", &json!({})).unwrap();
let params = json!({"bogus": 1});
assert!(check_params("style.list", &params).is_err());
assert!(s.execute("style.list", &params).is_ok());
}
crates/mcp/src/headless.rs
+2
View File
@@ -95,6 +95,8 @@ impl Headless {
}
fn exec(&mut self, id: &str, params: &Value) -> Result<Value, String> {
// A key the command's description doesn't name is an error, not a default (#776).
designcraft_engine::check_params(id, params).map_err(|e| e.to_string())?;
let r = self.session.execute(id, params).map_err(|e| {
let ui_only = ["app.", "view.", "window."].iter().any(|p| id.starts_with(p));
if ui_only && designcraft_engine::find_command(id).is_none() { format!("`{id}` is a UI command; {NEEDS_APP}") } else { e.to_string() }
crates/mcp/src/server.rs
+39
View File
@@ -149,6 +149,7 @@ impl Server {
"tools/call" => {
let name = params.get("name").and_then(Value::as_str).ok_or((INVALID_PARAMS, "missing tool `name`".to_string()))?;
let args = params.get("arguments").cloned().unwrap_or(Value::Null);
check_arguments(name, &args).map_err(|m| (INVALID_PARAMS, m))?;
Ok(call_tool(self.backend.as_mut(), name, &args).to_value())
}
"resources/list" => Ok(json!({"resources": [
@@ -170,3 +171,41 @@ impl Server {
}
}
}
/// Arguments a tool doesn't take, and command params a command doesn't take, are invalid params
/// (#776): a misspelt key is refused instead of being ignored. `execute` and `batch` are checked
/// against the params descriptions of the commands they run, every step before the first runs.
fn check_arguments(name: &str, args: &Value) -> Result<(), String> {
let Some(arguments) = args.as_object() else { return Ok(()) };
if let Some(def) = tool_definitions().into_iter().find(|t| t.get("name").and_then(Value::as_str) == Some(name))
&& let Some(properties) = def.get("inputSchema").and_then(|s| s.get("properties")).and_then(Value::as_object)
&& let Some(key) = arguments.keys().find(|k| !properties.contains_key(*k))
{
return Err(format!("unknown argument `{key}` for `{name}`"));
}
let command = |step: &Value| -> Result<(), String> {
match (step.get("command").and_then(Value::as_str), step.get("params")) {
(Some(id), Some(params)) => designcraft_engine::check_params(id, params).map_err(|e| e.to_string()),
_ => Ok(()),
}
};
match name {
"execute" => command(args),
"batch" => {
let steps = match (arguments.get("commands").and_then(Value::as_array), arguments.get("script").and_then(Value::as_str)) {
(Some(steps), _) => steps.clone(),
// A script that doesn't parse is the tool's to report.
(None, Some(text)) => match designcraft_engine::script::parse(text) {
Ok(steps) => steps.into_iter().map(|s| json!({"command": s.command, "params": s.params})).collect(),
Err(_) => return Ok(()),
},
(None, None) => return Ok(()),
};
for (i, step) in steps.iter().enumerate() {
command(step).map_err(|e| format!("step {i}: {e}"))?;
}
Ok(())
}
_ => Ok(()),
}
}
crates/mcp/src/tests.rs
+38
View File
@@ -500,3 +500,41 @@ fn explicit_typing_rejects_stale_selection_and_closed_document() {
b.session.execute("file.close", &json!({})).unwrap();
assert!(b.call("ui.text", json!({"text":"X"})).is_err());
}
/// #776: a key a command (or a tool) doesn't take is an invalid-params error, not ignored.
#[test]
fn unknown_params_are_invalid_params() {
let mut s = server();
let code_and_message = |v: &Value| (v["error"]["code"].as_i64(), v["error"]["message"].as_str().unwrap_or("").to_string());
let f = ok(&mut s, "execute", json!({"command": "frame.create", "params": {"rect": [36, 36, 300, 200], "content": "text", "text": "Hello"}}));
let v = rpc(
&mut s,
1,
"tools/call",
json!({"name": "execute", "arguments": {"command": "text.select", "params": {"story": f["story"], "start": 0, "end": 5}}}),
);
let (code, msg) = code_and_message(&v);
assert_eq!(code, Some(-32602), "{v}");
assert!(msg.contains("unknown parameter `end`; the command takes: story, anchor, focus"), "{msg}");
let v = rpc(&mut s, 2, "tools/call", json!({"name": "execute", "arguments": {"command": "style.list", "params": {"bogus": 1}}}));
let (code, msg) = code_and_message(&v);
assert_eq!(code, Some(-32602), "{v}");
assert!(msg.contains("unknown parameter `bogus`; the command takes no parameters"), "{msg}");
// A batch is checked step by step before any step runs: the frame isn't created.
let before = ok(&mut s, "inspect_document", json!({}));
for args in [
json!({"commands": [{"command": "frame.create", "params": {"rect": [0, 0, 50, 50]}}, {"command": "file.new", "params": {"widht": 300}}]}),
json!({"script": "frame.create {\"rect\": [0, 0, 50, 50]}\nfile.new {\"widht\": 300}"}),
] {
let v = rpc(&mut s, 3, "tools/call", json!({"name": "batch", "arguments": args}));
let (code, msg) = code_and_message(&v);
assert_eq!(code, Some(-32602), "{v}");
assert!(msg.contains("step 1") && msg.contains("did you mean `width`?"), "{msg}");
}
assert_eq!(ok(&mut s, "inspect_document", json!({})), before);
// An argument the tool doesn't take.
let v = rpc(&mut s, 4, "tools/call", json!({"name": "execute", "arguments": {"command": "style.list", "parms": {}}}));
assert_eq!(code_and_message(&v), (Some(-32602), "unknown argument `parms` for `execute`".to_string()));
// The right keys still run.
ok(&mut s, "execute", json!({"command": "text.select", "params": {"story": f["story"], "anchor": 0, "focus": 5}}));
}
crates/tools/src/pen.rs
+1 -1
View File
@@ -117,7 +117,7 @@ impl PenTool {
match self.path {
None if self.anchors.len() >= 2 => {
let anchors: Vec<Value> = self.anchors.iter().map(anchor_json).collect();
vec![Action::Exec("path.create".into(), json!({"spread": spread_json(sr), "anchors": anchors, "closed": false, "notifyPen": true}))]
vec![Action::Exec("path.create".into(), json!({"spread": spread_json(sr), "anchors": anchors, "closed": false}))]
}
Some(id) => {
let mut p = json!({"id": id, "anchor": anchor_json(last)});
crates/ui-egui/src/control.rs
+4
View File
@@ -114,6 +114,10 @@ pub fn handle(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) ->
let Some(id) = s("command").or(s("id")) else { return err("missing `command`") };
let params = p.get("params").cloned().unwrap_or(json!({}));
let params = if params.is_null() { json!({}) } else { params };
// A key the command's description doesn't name is an error, not a default (#776).
if let Err(e) = designcraft_engine::check_params(id, &params) {
return err(e.to_string());
}
wrap(app.run(id, params))
}
"engine.commands" => ok(all_commands(app)),
crates/ui-egui/src/dialogs.rs
+2 -1
View File
@@ -4312,7 +4312,8 @@ mod tests {
assert_eq!(page.margins.top, 10.0);
assert_eq!(page.margins.outside, 40.0);
}
assert!(doc.spreads.iter().all(|s| s.items.is_empty()), "undocumented fields stay excluded");
// `primaryTextFrame` is documented since #776, so the dialog passes it on (`status` isn't).
assert!(doc.spreads.iter().any(|s| !s.items.is_empty()), "documented fields reach the command");
assert!(app.ui.dialog.is_none());
}
}
docs/control-protocol.md
+1 -1
View File
@@ -31,7 +31,7 @@ so only enable it while you use it. Transport: `apps/designcraft/src/control_ser
| Method | Params | What it does |
|---|---|---|
| `engine.execute` / `ui.menu.invoke` | `{command, params}` | Run any engine or UI command (see `engine.commands`). `ui.menu.invoke` without `params` acts like choosing the menu item: a command labelled "…" that takes parameters opens its dialog |
| `engine.execute` / `ui.menu.invoke` | `{command, params}` | Run any engine or UI command (see `engine.commands`). `ui.menu.invoke` without `params` acts like choosing the menu item: a command labelled "…" that takes parameters opens its dialog. A key the command's params description doesn't name is an error (`unknown parameter …`), not ignored |
| `engine.commands` | — | Every command: id, label, menu path, shortcut, params doc, enabled |
| `document.inspect` | — | Pages, spreads, items, stories (overset), styles, swatches, selection |
| `ui.inspect` | — | Tool, UI state, view (zoom/origin), canvas rect, perf |
docs/mcp.md
+1 -1
View File
@@ -76,7 +76,7 @@ and its second character starts at 3. An offset inside a character snaps back to
| Tool | What it does |
|---|---|
| `list_commands` | Every command (id, label, menu path, shortcut, params, enabled), with `filter` / `enabledOnly` |
| `execute` | Run any command: `{command, params}` |
| `execute` | Run any command: `{command, params}`. A params key the command doesn't take is a JSON-RPC `-32602` (invalid params) error naming it, as is an argument a tool doesn't take; `batch` checks every step before running the first |
| `batch` | Run `commands: [{command, params}]` (or `script` text, one `command.id {json}` per line) in order, stopping at the first error (reports `failedIndex`); `"$N.path"` parameters use earlier results ([agents.md](agents.md)) |
| `inspect_document` | Pages, spreads, items (ids, bounds, fill, story), stories (overset), layers, styles, swatches, selection |
| `get_story` | Text, frames, paragraphs, lines, overset of a story (`story` or text `frame`) |