From 57c1133a345377dbf809c741b7ba9230c2ca7b98 Mon Sep 17 00:00:00 2001 From: Mert Dede Date: Tue, 27 Oct 2026 07:57:54 +0300 Subject: [PATCH 01/12] M0.9: find layer ids without changing the selection (#996) node.find {root?, name?, type?, text?, component?, limit?} returns {ids, total} for the strict descendants of root (or the current page) that match every filter given: name, text and main component name as case-insensitive substrings, type as an exact type name. It changes neither the selection, the page nor the undo history. Part of #996; fields/outline are left for later. Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: GPT-6.1 Sol Co-Authored-By: GPT-6 Astra --- crates/engine/src/cmd/edit.rs | 75 +++++++++++ crates/engine/src/lib.rs | 2 + crates/engine/src/node_find_tests.rs | 186 +++++++++++++++++++++++++++ crates/engine/src/tests.rs | 3 + crates/mcp/tests/it/agent_tasks.rs | 26 ++++ docs/mcp.md | 11 ++ docs/roadmap.md | 3 +- 7 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 crates/engine/src/node_find_tests.rs diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 7ec63c69..151b5820 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -26,6 +26,74 @@ use select::{ select_parent, select_same, select_sibling, toggle, }; +/// The layer types `node.find` takes, as `NodeKind::type_name` spells them. +const LAYER_TYPES: [&str; 16] = [ + "PAGE", + "FRAME", + "GROUP", + "SECTION", + "RECTANGLE", + "ELLIPSE", + "POLYGON", + "STAR", + "LINE", + "VECTOR", + "TEXT", + "BOOLEAN_OPERATION", + "COMPONENT", + "COMPONENT_SET", + "INSTANCE", + "SLICE", +]; + +fn node_find(s: &mut Session, p: &Value) -> Result { + let cmd = "node.find"; + let filter = |key| -> Result> { + get(p, key).map(|v| v.as_str().map(str::to_lowercase).ok_or_else(|| bad(cmd, format!("`{key}` must be a string")))).transpose() + }; + let name = filter("name")?; + let text = filter("text")?; + let component = filter("component")?; + let kind = filter("type")?; + if let Some(kind) = &kind + && !LAYER_TYPES.iter().any(|t| t.eq_ignore_ascii_case(kind)) + { + return Err(bad(cmd, format!("unknown type; use one of {}", LAYER_TYPES.join(", ")))); + } + let limit = match get(p, "limit") { + None => 100, + Some(v) => v.as_u64().filter(|n| *n <= 10000).ok_or_else(|| bad(cmd, "`limit` must be an integer from 0 to 10000"))? as usize, + }; + let root = opt_id(cmd, p, "root")?.map_or_else(|| s.page(), Ok)?; + let doc = s.doc()?; + // Keep one child iterator per ancestor, not a materialised subtree or all pending siblings. + let mut stack = vec![doc.get(root)?.children.iter()]; + let mut ids = Vec::new(); + let mut total = 0usize; + while let Some(children) = stack.last_mut() { + let Some(&id) = children.next() else { + stack.pop(); + continue; + }; + let n = doc.get(id)?; + stack.push(n.children.iter()); + if name.as_ref().is_some_and(|q| !n.name.to_lowercase().contains(q)) + || kind.as_ref().is_some_and(|q| !n.kind.type_name().eq_ignore_ascii_case(q)) + || text.as_ref().is_some_and(|q| !matches!(&n.kind, NodeKind::Text(t) if t.characters.to_lowercase().contains(q))) + || component.as_ref().is_some_and(|q| { + !matches!(&n.kind, NodeKind::Instance { component, .. } if doc.node(*component).is_some_and(|c| c.name.to_lowercase().contains(q))) + }) + { + continue; + } + total += 1; + if ids.len() < limit { + ids.push(id); + } + } + Ok(json!({"ids": ids, "total": total})) +} + pub fn specs() -> Vec { vec![ spec!("edit.undo", "Undo", "{}", false, |s, _| Ok(json!({"undone": s.undo()?}))), @@ -232,6 +300,13 @@ pub fn specs() -> Vec { let i = req_id("node.get", p, "id")?; Ok(crate::inspect::node_json(s.doc()?, i, 1)) }), + spec!( + "node.find", + "Find layers", + "{root? (strict descendants; default current page), name?, type? (canonical layer type), text?, component? (main component name), limit? (0-10000; default 100)}: every filter must match; name, text and component are case-insensitive substrings, type is exact (any case); returns {ids, total} and does not select", + false, + node_find + ), spec!( "node.reparent", "Move layer into", diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 60f240cb..e45bea15 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -342,6 +342,8 @@ mod menu_gaps_tests; #[cfg(test)] mod nested_override_tests; #[cfg(test)] +mod node_find_tests; +#[cfg(test)] mod node_set_refusals_tests; #[cfg(test)] mod online_tests; diff --git a/crates/engine/src/node_find_tests.rs b/crates/engine/src/node_find_tests.rs new file mode 100644 index 00000000..09bb7e63 --- /dev/null +++ b/crates/engine/src/node_find_tests.rs @@ -0,0 +1,186 @@ +//! Finding layers without changing the selection, page or edit history (#996). + +use figcraft_doc::*; +use serde_json::{Value, json}; +use std::sync::Arc; + +use crate::Session; +use crate::components::tests::{ex, idv}; + +fn add(s: &mut Session, parent: NodeId, kind: NodeKind, name: &str) -> NodeId { + s.doc_mut().unwrap().create(parent, kind, name, 0.0, 0.0, 100.0, 100.0).unwrap() +} + +fn found(s: &mut Session, params: Value, ids: &[NodeId]) { + assert_eq!(ex(s, "node.find", params), json!({"ids": ids, "total": ids.len()})); +} + +#[test] +fn node_find_filters_are_literal_case_insensitive_and_anded() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let frame = add(&mut s, page, NodeKind::Frame, "CAFÉ [Card]"); + let title = add(&mut s, frame, NodeKind::Text(TextData::new("HÉLLO 世界")), "Café Title"); + let other = add(&mut s, frame, NodeKind::Text(TextData::new("Other")), "Title"); + let rect = add(&mut s, page, NodeKind::Rectangle, "HÉLLO 世界"); + found(&mut s, json!({"name": "CAFÉ"}), &[frame, title]); + found(&mut s, json!({"name": "[cArD]"}), &[frame]); + found(&mut s, json!({"type": "tExT"}), &[title, other]); + found(&mut s, json!({"text": "héLLo 世"}), &[title]); + found(&mut s, json!({"name": "title", "type": "TEXT", "text": "世界"}), &[title]); + found(&mut s, json!({"name": "café", "type": "RECTANGLE"}), &[]); + found(&mut s, json!({"text": "héllo", "name": "HÉLLO"}), &[]); + found(&mut s, json!({"text": ""}), &[title, other]); + found(&mut s, json!({"name": ""}), &[frame, title, other, rect]); + found(&mut s, json!({"name": ".*"}), &[]); +} + +#[test] +fn node_find_scope_is_preorder_strict_descendants_in_stored_order() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let root = add(&mut s, page, NodeKind::Frame, "Root"); + let first = add(&mut s, root, NodeKind::Frame, "First"); + let nested = add(&mut s, first, NodeKind::Rectangle, "Nested"); + let second = add(&mut s, root, NodeKind::Rectangle, "Second"); + let outside = add(&mut s, page, NodeKind::Rectangle, "Outside"); + s.doc_mut().unwrap().get_mut(first).unwrap().visible = false; + s.doc_mut().unwrap().get_mut(second).unwrap().locked = true; + found(&mut s, json!({}), &[root, first, nested, second, outside]); + found(&mut s, json!({"root": root}), &[first, nested, second]); + found(&mut s, json!({"root": root, "name": "root"}), &[]); + found(&mut s, json!({"root": nested}), &[]); + found(&mut s, json!({"root": page}), &[root, first, nested, second, outside]); + s.doc_mut().unwrap().reorder(second, 0).unwrap(); + found(&mut s, json!({"root": root}), &[second, first, nested]); +} + +#[test] +fn node_find_other_page_requires_root_and_does_not_switch_pages() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let here = add(&mut s, page, NodeKind::Frame, "Here"); + let other_page = s.doc_mut().unwrap().add_page("Other"); + let root = add(&mut s, other_page, NodeKind::Frame, "There"); + let child = add(&mut s, root, NodeKind::Rectangle, "Child"); + found(&mut s, json!({}), &[here]); + found(&mut s, json!({"root": other_page}), &[root, child]); + found(&mut s, json!({"root": root}), &[child]); + assert_eq!(s.page().unwrap(), page); +} + +#[test] +fn node_find_instances_match_main_name_and_visit_actual_children_only() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let main = add(&mut s, page, NodeKind::Component { description: String::new(), props: Default::default() }, "CAFÉ [Button]"); + let original = add(&mut s, main, NodeKind::Text(TextData::new("Original")), "Original"); + let inst = idv(&ex(&mut s, "instance.create", json!({"component": main}))); + let child = s.doc().unwrap().get(inst).unwrap().children[0]; + ex(&mut s, "node.set", json!({"ids": [inst], "props": {"name": "Copy"}})); + ex(&mut s, "text.set", json!({"id": child, "text": "Changed"})); + let broken = add(&mut s, page, NodeKind::Instance { component: NodeId(u64::MAX), props: Default::default() }, "Missing"); + found(&mut s, json!({"component": "café [bUt"}), &[inst]); + found(&mut s, json!({"component": "", "type": "instance", "name": "copy"}), &[inst]); + found(&mut s, json!({"component": "copy"}), &[]); + found(&mut s, json!({"component": "button", "text": "Changed"}), &[]); + found(&mut s, json!({"root": inst}), &[child]); + found(&mut s, json!({"root": inst, "text": "original"}), &[]); + found(&mut s, json!({"root": inst, "text": "changed"}), &[child]); + found(&mut s, json!({}), &[main, original, inst, child, broken]); +} + +#[test] +fn node_find_accepts_every_canonical_type() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let kinds = [ + NodeKind::Frame, + NodeKind::Group, + NodeKind::Section { contents_hidden: false }, + NodeKind::Rectangle, + NodeKind::Ellipse { start: 0.0, sweep: 1.0, inner: 0.0 }, + NodeKind::Polygon { count: 3 }, + NodeKind::Star { count: 5, inner_ratio: 0.5 }, + NodeKind::Line, + NodeKind::Vector { network: Default::default() }, + NodeKind::Text(TextData::new("Text")), + NodeKind::BooleanOp { op: BoolOp::Union }, + NodeKind::Component { description: String::new(), props: Default::default() }, + NodeKind::ComponentSet { description: String::new() }, + NodeKind::Instance { component: NodeId(u64::MAX), props: Default::default() }, + NodeKind::Slice, + ]; + for kind in kinds { + let name = kind.type_name(); + let id = add(&mut s, page, kind, name); + assert_eq!(ex(&mut s, "node.get", json!({"id": id}))["type"], name); + found(&mut s, json!({"type": name.to_ascii_lowercase()}), &[id]); + } + found(&mut s, json!({"type": "PAGE"}), &[]); +} + +#[test] +fn node_find_limit_caps_ids_not_total_and_zero_counts_only() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let ids: Vec<_> = (0..105).map(|_| add(&mut s, page, NodeKind::Rectangle, "Match")).collect(); + assert_eq!(ex(&mut s, "node.find", json!({})), json!({"ids": &ids[..100], "total": 105})); + assert_eq!(ex(&mut s, "node.find", json!({"limit": 2})), json!({"ids": &ids[..2], "total": 105})); + assert_eq!(ex(&mut s, "node.find", json!({"limit": 0})), json!({"ids": [], "total": 105})); + found(&mut s, json!({"limit": 10000}), &ids); + found(&mut s, json!({"name": "missing", "limit": 0}), &[]); +} + +#[test] +fn node_find_rejects_bad_root_type_filters_and_limits() { + let mut s = Session::new(); + for params in [ + json!({"root": 999999}), + json!({"root": "nope"}), + json!({"root": {"id": 1}}), + json!({"type": "BOOLEANOP"}), + json!({"type": "FRAME "}), + json!({"type": ""}), + json!({"type": 3}), + json!({"name": []}), + json!({"text": true}), + json!({"component": {}}), + json!({"limit": -1}), + json!({"limit": 10001}), + json!({"limit": 1.5}), + json!({"limit": 1.0}), + json!({"limit": "2"}), + json!({"limit": true}), + json!({"limit": []}), + json!({"limit": 1e308}), + ] { + assert!(s.execute("node.find", ¶ms).is_err(), "{params}"); + } + let error = s.execute("node.find", &json!({"type": "x".repeat(10000)})).unwrap_err().to_string(); + assert!(error.contains("unknown type"), "{error}"); + assert!(error.len() < 300, "unknown types are not echoed whole"); +} + +#[test] +fn node_find_leaves_document_selection_page_and_history_untouched() { + let mut s = Session::new(); + let layer = idv(&ex(&mut s, "shape.rectangle", json!({"width": 10, "height": 10}))); + ex(&mut s, "node.set", json!({"ids": [layer], "props": {"name": "Previous edit"}})); + ex(&mut s, "node.set", json!({"ids": [layer], "props": {"name": "Undone edit"}})); + ex(&mut s, "edit.undo", json!({})); + ex(&mut s, "select.set", json!({"ids": [layer]})); + let ds = s.ds().unwrap(); + let (doc, selection, page, undo, redo) = (ds.doc.clone(), ds.selection.clone(), ds.page, ds.history.undo.len(), ds.history.redo.len()); + assert!(undo > 0 && redo > 0); + found(&mut s, json!({"name": "previous"}), &[layer]); + found(&mut s, json!({"name": "missing"}), &[]); + ex(&mut s, "node.find", json!({"limit": 0})); + assert!(s.execute("node.find", &json!({"root": 999999})).is_err()); + let ds = s.ds().unwrap(); + assert!(Arc::ptr_eq(&ds.doc, &doc)); + assert_eq!((&ds.selection, ds.page, ds.history.undo.len(), ds.history.redo.len()), (&selection, page, undo, redo)); + assert_eq!(ex(&mut s, "edit.undo", json!({}))["undone"], true); + assert_ne!(s.doc().unwrap().get(layer).unwrap().name, "Previous edit"); + assert_eq!(s.ds().unwrap().history.undo.len(), undo - 1); +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 811724f2..966e976a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -454,6 +454,9 @@ fn every_command_survives_junk_params() { // would write to it.) json!({"ids": [2, 3], "type": "vector", "path": [5], "width": -1e308, "props": {"path": "M0 0 L1e308 0 L-1e308 1e308", "height": "NaN", "x": 1e308}}), json!({"id": 2, "type": "vector", "path": {"d": 1}, "props": {"path": "M NaN 0 Z", "width": 1e-308}}), + // Layer search roots, filters and limits as junk. + json!({"root": {"id": 2}, "name": [1], "type": "nope", "text": true, "component": [], "limit": -1}), + json!({"root": 999999, "limit": 1e308}), // Batch step names and reference paths as junk. json!({"commands": [{"command": "select.none", "as": "a"}, {"command": "select.none", "as": [1]}, {"command": "select.none", "as": "a"}]}), json!({"commands": [{"command": "select.all", "as": "\u{0}"}, {"command": "node.set", "params": {"ids": [ diff --git a/crates/mcp/tests/it/agent_tasks.rs b/crates/mcp/tests/it/agent_tasks.rs index ae576e12..d087b25e 100644 --- a/crates/mcp/tests/it/agent_tasks.rs +++ b/crates/mcp/tests/it/agent_tasks.rs @@ -307,6 +307,32 @@ fn task_vector_edit_keys() { assert!(c.call("run_command", json!({"command": "vector.edit.state", "params": {}}))["isError"] == true, "Enter left edit mode"); } +#[test] +fn task_node_find_without_selecting_then_use_ids_in_batch() { + let mut c = Client::new(); + let card = c.create(json!({"type": "frame", "name": "Card"})); + let title = c.create(json!({"type": "text", "parent": card, "name": "Title", "text": "Hello"})); + c.ok("select", json!({"ids": [card]})); + let before = c.doc(); + assert_eq!(c.run("node.find", json!({"root": card, "type": "text", "text": "HELLO"})), json!({"ids": [title], "total": 1})); + let after = c.doc(); + for key in ["selection", "currentPage", "history"] { + assert_eq!(after[key], before[key], "{key}"); + } + let out = c.ok( + "batch", + json!({"commands": [ + {"command": "node.find", "params": {"root": card, "name": "title"}}, + {"command": "node.set", "params": {"ids": [{"$step": 0, "field": "ids/0"}], "props": {"name": "Found title"}}} + ]}), + ); + assert_eq!(out[0], json!({"ids": [title], "total": 1})); + assert_eq!(c.node(&title)["name"], "Found title"); + assert_eq!(c.doc()["selection"], before["selection"]); + c.run("edit.undo", json!({})); + assert_eq!(c.node(&title)["name"], "Title"); +} + /// A whole card in one `batch` call: later steps reach the frames earlier steps created through /// `{"$step": n}`, so the tree is built in one round-trip and one undo step. #[test] diff --git a/docs/mcp.md b/docs/mcp.md index 05dfd1e6..9a63f2a2 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -303,6 +303,17 @@ Or in a project's `.mcp.json`: - Programmatic calls never open dialogs. - Verify your work: `inspect_document` / `get_node` for numbers, `screenshot` for pixels. +To find layers without changing the selection, use `run_command` with `node.find`, for example +`{"command": "node.find", "params": {"root": 12, "type": "TEXT", "text": "hello", "limit": 20}}`. +It searches strict descendants of `root`, or the current page without one, in stored child order +(depth-first, parent before children), including hidden and locked layers and instance children. +`name`, `text` (text layers only) and `component` (instances by their main component's name) are +case-insensitive literal substrings; `type` is a canonical layer type, case-insensitive. All +filters must match. The result is `{"ids": [...], "total": N}`: at most `limit` ids (default 100, +0 to 10000), with every match counted in `total`; `limit: 0` counts only. A root on another page +works without switching pages. Unknown roots or types and invalid limits are errors. In a batch, +a later step can use `{"$step": 0, "field": "ids/0"}` to target the first match. + ## Tools | Tool | What it does | diff --git a/docs/roadmap.md b/docs/roadmap.md index 14d734ba..23461637 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # FigCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (finding layers without selecting them (M0.9, part of #996); Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) Forward-looking plan. Where FigCraft stands today: [`ROADMAP.md`](../ROADMAP.md) and [`target-app-parity.md`](target-app-parity.md); the full ranked list: [`gaps.md`](gaps.md). Hours @@ -75,6 +75,7 @@ blockers. | Date | Change | Summary | |---|---|---| +| 2026-10-27 | minor | Finding layers without selecting them (M0.9, part of #996): `node.find` returns ids and a total with AND filters for name, canonical type, text and main component name, under a root or on the current page; ids are limited, the count is not. The selection, page and undo history stay unchanged. Available through `run_command` and batch step references. Regression tests in `crates/engine/src/node_find_tests.rs` and `crates/mcp/tests/it/agent_tasks.rs`. Numbers unchanged | | 2026-10-26 | minor | Dragging a selected unfilled shape (M1, #1370): a press inside a selected stroke-only shape over a filled layer stacked below it moved that layer; the drag-from-inside fallback of #910 now treats any layer painted below the selection like the frame or canvas behind it, so the selection drags, while a layer painted above it still takes the press. Regression test in the selection behaviour suite. Numbers unchanged | | 2026-10-26 | minor | Text truncation (M4, #1375, #1376, #1377): the last line shown takes as much of the next word as fits before the ellipsis ("Lorem consec…", not "Lorem…"); an explicit height, `transform.resize`, a resize handle or fixed sizing on a truncated text keeps it truncating instead of turning it fixed size; Max lines on auto-height text hugs up to that many lines and ends in an ellipsis (`text.maxLines`, `node.set {maxLines}`, Type settings, `.fig` export and import), and `node.get` / `get_node` report `maxLines`. Regression tests in the text and engine behaviour suites. Numbers unchanged | | 2026-10-24 | minor | Installed weights of an embedded font family (M7, #1246): release builds embed some families with only a few styles (Poppins Regular, Italic and Bold), and the first font scan dropped every installed face of such a family, so an installed Poppins SemiBold or Medium was never offered (a rescan did offer them). The first scan now keeps the embedded faces, which still win for the styles they cover, and offers the installed styles they lack beside them. Regression test in the font database tests. Numbers unchanged | From 9d7c435f6b207e2f2edc15f24fbd6c1f103902b7 Mon Sep 17 00:00:00 2001 From: Mert Dede Date: Tue, 27 Oct 2026 08:33:28 +0300 Subject: [PATCH 02/12] #1408: match a Greek final sigma in node.find filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lowercasing a name ending in Σ gives ς, while a query Σ on its own lowercases to σ, so the substring check missed it (CodeRabbit). Both sides now fold ς to σ. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/edit.rs | 18 ++++++++++++------ crates/engine/src/node_find_tests.rs | 10 ++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 151b5820..4ddbd612 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -46,10 +46,16 @@ const LAYER_TYPES: [&str; 16] = [ "SLICE", ]; +/// Lowercase for `node.find`'s substring filters. Greek final sigma becomes σ on both sides, so a +/// query lowercased on its own (`Σ` to `σ`) still matches a name ending in it (`ΟΣ` to `ος`). +fn fold(s: &str) -> String { + s.to_lowercase().replace('ς', "σ") +} + fn node_find(s: &mut Session, p: &Value) -> Result { let cmd = "node.find"; let filter = |key| -> Result> { - get(p, key).map(|v| v.as_str().map(str::to_lowercase).ok_or_else(|| bad(cmd, format!("`{key}` must be a string")))).transpose() + get(p, key).map(|v| v.as_str().map(fold).ok_or_else(|| bad(cmd, format!("`{key}` must be a string")))).transpose() }; let name = filter("name")?; let text = filter("text")?; @@ -77,12 +83,12 @@ fn node_find(s: &mut Session, p: &Value) -> Result { }; let n = doc.get(id)?; stack.push(n.children.iter()); - if name.as_ref().is_some_and(|q| !n.name.to_lowercase().contains(q)) + if name.as_ref().is_some_and(|q| !fold(&n.name).contains(q)) || kind.as_ref().is_some_and(|q| !n.kind.type_name().eq_ignore_ascii_case(q)) - || text.as_ref().is_some_and(|q| !matches!(&n.kind, NodeKind::Text(t) if t.characters.to_lowercase().contains(q))) - || component.as_ref().is_some_and(|q| { - !matches!(&n.kind, NodeKind::Instance { component, .. } if doc.node(*component).is_some_and(|c| c.name.to_lowercase().contains(q))) - }) + || text.as_ref().is_some_and(|q| !matches!(&n.kind, NodeKind::Text(t) if fold(&t.characters).contains(q))) + || component.as_ref().is_some_and( + |q| !matches!(&n.kind, NodeKind::Instance { component, .. } if doc.node(*component).is_some_and(|c| fold(&c.name).contains(q))), + ) { continue; } diff --git a/crates/engine/src/node_find_tests.rs b/crates/engine/src/node_find_tests.rs index 09bb7e63..d8729357 100644 --- a/crates/engine/src/node_find_tests.rs +++ b/crates/engine/src/node_find_tests.rs @@ -35,6 +35,16 @@ fn node_find_filters_are_literal_case_insensitive_and_anded() { found(&mut s, json!({"name": ".*"}), &[]); } +#[test] +fn node_find_matches_a_greek_final_sigma_both_ways() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let os = add(&mut s, page, NodeKind::Text(TextData::new("ΛΟΓΟΣ")), "ΟΣ"); + found(&mut s, json!({"name": "Σ"}), &[os]); + found(&mut s, json!({"name": "ς"}), &[os]); + found(&mut s, json!({"text": "λογοσ"}), &[os]); +} + #[test] fn node_find_scope_is_preorder_strict_descendants_in_stored_order() { let mut s = Session::new(); From 2ff69183d76bd6fd69e51238762c0934ee92e408 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 08:35:46 +0100 Subject: [PATCH 03/12] M7: CSS system font names draw in macOS's System Font `system-ui`, `-apple-system`, `BlinkMacSystemFont` and `.AppleSystemUIFont` join the names `apple_alias` resolves to the installed System Font, as SF Pro and SF UI already do. SVG import keeps a family the SVG lists when a font answers to its name, so on a Mac `font-family="-apple-system, system-ui, sans-serif"` keeps `-apple-system` and draws in System Font; elsewhere the generic stand-ins apply as before. Co-Authored-By: Claude Opus 5.5 --- crates/svg/src/import/text.rs | 27 ++++++++++++++---------- crates/svg/src/import/text_tests.rs | 32 +++++++++++++++++++++++++++++ crates/text/src/fontdb.rs | 10 ++++----- crates/text/src/fontdb_tests.rs | 8 ++++++-- 4 files changed, 59 insertions(+), 18 deletions(-) create mode 100644 crates/svg/src/import/text_tests.rs diff --git a/crates/svg/src/import/text.rs b/crates/svg/src/import/text.rs index 32d37bf1..6c48fd59 100644 --- a/crates/svg/src/import/text.rs +++ b/crates/svg/src/import/text.rs @@ -28,8 +28,11 @@ impl Fonts { fn load() -> Self { Fonts(figcraft_text::fontdb::family_names().into_iter().map(|f| f.to_lowercase()).collect()) } + /// Installed under this name, or under the family it names (`system-ui` on a Mac is its + /// System Font, see `fontdb::apple_alias`). fn has(&self, family: &str) -> bool { - self.0.contains(&family.to_lowercase()) + let key = family.to_lowercase(); + self.0.contains(&key) || figcraft_text::fontdb::apple_alias(&key).is_some_and(|target| self.0.contains(target)) } } @@ -67,8 +70,9 @@ fn generic(name: &str) -> Option<&'static [&'static str]> { }) } -/// The family to use: the first the SVG lists that's installed, else the first it names, else the -/// default. +/// The family to use: the first the SVG lists that's installed (a generic family as the first of +/// its installed stand-ins, unless a font answers to its name: `system-ui` on a Mac), else the +/// first it names, else the default. fn family(list: &[FontFamily], fonts: &Fonts) -> String { let name = |f: &FontFamily| match f { FontFamily::Serif => "serif".to_string(), @@ -80,14 +84,11 @@ fn family(list: &[FontFamily], fonts: &Fonts) -> String { }; for f in list { let n = name(f); - match generic(&n) { - Some(cands) => { - if let Some(c) = cands.iter().find(|c| fonts.has(c)) { - return (*c).to_string(); - } - } - None if fonts.has(&n) => return n, - None => {} + if fonts.has(&n) { + return n; + } + if let Some(c) = generic(&n).and_then(|cands| cands.iter().find(|c| fonts.has(c))) { + return (*c).to_string(); } } list.iter().map(name).find(|n| generic(n).is_none() && !n.is_empty()).unwrap_or_else(|| TextStyle::default().font_family) @@ -339,3 +340,7 @@ fn add_layer( } cx.add(n, parent); } + +#[cfg(test)] +#[path = "text_tests.rs"] +mod tests; diff --git a/crates/svg/src/import/text_tests.rs b/crates/svg/src/import/text_tests.rs new file mode 100644 index 00000000..f267b294 --- /dev/null +++ b/crates/svg/src/import/text_tests.rs @@ -0,0 +1,32 @@ +//! Which family an SVG's `font-family` list gives a text layer. + +use usvg::FontFamily; + +use super::{Fonts, family}; + +fn fonts(installed: &[&str]) -> Fonts { + Fonts(installed.iter().map(|f| f.to_lowercase()).collect()) +} + +fn named(list: &[&str]) -> Vec { + list.iter() + .map(|n| match *n { + "sans-serif" => FontFamily::SansSerif, + n => FontFamily::Named(n.to_string()), + }) + .collect() +} + +/// The names CSS gives the system font keep their name on a Mac, where they draw in its System +/// Font, and fall back as before without it. +#[test] +fn css_system_font_names_keep_their_name_where_the_system_font_is() { + let mac = fonts(&["System Font", "Inter", "Helvetica"]); + assert_eq!(family(&named(&["-apple-system", "BlinkMacSystemFont", "system-ui", "sans-serif"]), &mac), "-apple-system"); + assert_eq!(family(&named(&["BlinkMacSystemFont", "sans-serif"]), &mac), "BlinkMacSystemFont"); + assert_eq!(family(&named(&["system-ui", "sans-serif"]), &mac), "system-ui"); + assert_eq!(family(&named(&["Helvetica", "system-ui"]), &mac), "Helvetica", "an installed family the SVG lists first still wins"); + let elsewhere = fonts(&["Inter", "DejaVu Sans"]); + assert_eq!(family(&named(&["-apple-system", "BlinkMacSystemFont", "system-ui", "sans-serif"]), &elsewhere), "Inter"); + assert_eq!(family(&named(&["-apple-system"]), &elsewhere), "-apple-system", "nothing installed: the first name it gives"); +} diff --git a/crates/text/src/fontdb.rs b/crates/text/src/fontdb.rs index ced18c81..13987171 100644 --- a/crates/text/src/fontdb.rs +++ b/crates/text/src/fontdb.rs @@ -1069,12 +1069,12 @@ fn public_family(family: &str) -> Option { /// The family Apple's system fonts are installed under, for the names designs use for them /// (lowercase): SF Pro and SF UI are the variable `SFNS.ttf` ("System Font"), its text and display -/// cuts its optical sizes. -fn apple_alias(key: &str) -> Option<&'static str> { +/// cuts its optical sizes, as are the names CSS gives the system font (`system-ui`, +/// `-apple-system`, `BlinkMacSystemFont`, `.AppleSystemUIFont`). +pub fn apple_alias(key: &str) -> Option<&'static str> { Some(match key.trim_start_matches('.') { - "sf pro" | "sf pro text" | "sf pro display" | "sf ui text" | "sf ui display" | "sf ns" | "sf ns text" | "sf ns display" | "san francisco" => { - "system font" - } + "sf pro" | "sf pro text" | "sf pro display" | "sf ui text" | "sf ui display" | "sf ns" | "sf ns text" | "sf ns display" | "san francisco" + | "system-ui" | "-apple-system" | "blinkmacsystemfont" | "applesystemuifont" => "system font", "sf compact text" | "sf compact display" => "sf compact", _ => return None, }) diff --git a/crates/text/src/fontdb_tests.rs b/crates/text/src/fontdb_tests.rs index 85d1f3a7..35b9435b 100644 --- a/crates/text/src/fontdb_tests.rs +++ b/crates/text/src/fontdb_tests.rs @@ -340,8 +340,8 @@ fn font_bytes_load_into_the_db() { } } -/// Apple's system font answers to the names designs use for it (SF Pro, SF UI Text, …), unless a -/// font of that name is installed. +/// Apple's system font answers to the names designs and CSS use for it (SF Pro, SF UI Text, +/// system-ui, …), unless a font of that name is installed. #[test] fn apple_system_font_names_find_the_system_font() { let mut db = FontDb::default(); @@ -352,7 +352,11 @@ fn apple_system_font_names_find_the_system_font() { for name in ["SF Pro", "SF Pro Text", "SF Pro Display", "SF UI Text", "SF UI Display", ".SF NS Text", "San Francisco"] { assert!(db.has_family(name), "{name}"); } + for name in ["system-ui", "-apple-system", "BlinkMacSystemFont", ".AppleSystemUIFont"] { + assert_eq!(db.key(name), "system font", "{name}"); + } assert!(!db.has_family("SF Mono")); + assert!(!db.has_family("ui-monospace")); db.pending.insert("sf pro".into(), ("SF Pro".into(), Vec::new())); assert_eq!(db.key("SF Pro"), "sf pro", "an installed SF Pro is itself"); assert_eq!(db.key("SF UI Text"), "system font"); From 64e648ae7e009402f65e57e32def53fe8eacea56 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 09:06:33 +0100 Subject: [PATCH 04/12] M0.9: command docs list every parameter their commands read Before commands can refuse names their docs don't list (#1413), the docs have to list what the code reads. These are the top-level names the run code (and every helper it hands the params to) reads that its doc left out, all checked against the code; nothing changes behaviour: - creation: node.create (characters, points, closed), frame.create, shape.*, text.create and vector.create (index, name, select), image.place (name) - aliases the code already took: comment.* (id | link), annotation.add and annotation.edit (label | text), text.set (characters | text), text.listStyle (style | list), fill.add and stroke.add (paint | color), proto.interaction.add/set (durationMs | duration_ms, delayMs | delay_ms) - options: file.export and export.update (contentsOnly, includeBoundingBox, svgOutlineText, svgSimplifyStroke), comment.next (frame, resolved), layout.suggest (direction), variable.create (description), vector.edit.widthPoint (side), collab.share (key), collab.manageFile (access, label, link, name), fill.pattern (visible, blend) - docs whose shape hid a name: proto.interaction.add (a stray `}` closed the object before delayMs; dest was only in a trailer), proto.interaction.set (patch fields in a second object), collab.join (a top-level comma inside the link examples), pref.uiScale (commas in the value's description) Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/canvas/vectoredit/width.rs | 2 +- crates/engine/src/cmd/comment.rs | 52 +++++++++++--------- crates/engine/src/cmd/create.rs | 48 +++++++++++++----- crates/engine/src/cmd/edit.rs | 8 +-- crates/engine/src/cmd/file.rs | 2 +- crates/engine/src/cmd/proto.rs | 4 +- crates/engine/src/cmd/structure.rs | 2 +- crates/engine/src/cmd/styles.rs | 2 +- crates/engine/src/cmd/text.rs | 22 ++++++--- crates/engine/src/cmd/view.rs | 2 +- crates/engine/src/dev/annotations.rs | 4 +- crates/engine/src/sharing/live/join.rs | 6 +-- 12 files changed, 95 insertions(+), 59 deletions(-) diff --git a/crates/engine/src/canvas/vectoredit/width.rs b/crates/engine/src/canvas/vectoredit/width.rs index ba51b043..7977965d 100644 --- a/crates/engine/src/canvas/vectoredit/width.rs +++ b/crates/engine/src/canvas/vectoredit/width.rs @@ -244,7 +244,7 @@ pub(crate) fn specs() -> Vec { vec![spec!( "vector.edit.widthPoint", "Width point", - "{action?: add|set|remove, index?, t? (0-1 along the path), width? (px) | scale? (× stroke weight)} — add needs t (width defaults to the current one); set moves or resizes point `index`; with `side` only that side of the path changes (looking along it): `width` is then the distance from the centre line to that edge and `scale` that side's scale (× stroke weight, as if both sides were that wide)", + "{action?: add|set|remove, index?, t? (0-1 along the path), width? (px) | scale? (× stroke weight), side? (left|right)} — add needs t (width defaults to the current one); set moves or resizes point `index`; with `side` only that side of the path changes (looking along it): `width` is then the distance from the centre line to that edge and `scale` that side's scale (× stroke weight, as if both sides were that wide)", true, width_point )] diff --git a/crates/engine/src/cmd/comment.rs b/crates/engine/src/cmd/comment.rs index d1d360f4..587bd1c6 100644 --- a/crates/engine/src/cmd/comment.rs +++ b/crates/engine/src/cmd/comment.rs @@ -410,7 +410,7 @@ pub fn specs() -> Vec { Ok(json!({"id": id, "link": link(id)})) } ), - spec!("comment.reply", "Reply to comment", "{id, text, author?}", true, |s, p| { + spec!("comment.reply", "Reply to comment", "{id | link, text, author?}", true, |s, p| { let id = req_comment("comment.reply", s, p)?; let who = author(s, p); let r = Message::new(who.clone(), text("comment.reply", p)?, now_secs()); @@ -420,7 +420,7 @@ pub fn specs() -> Vec { c.touched_by(&who); Ok(json!({"reply": c.replies.len() - 1})) }), - spec!("comment.edit", "Edit comment", "{id, text, reply? (index; omit for the first message), author?}", true, |s, p| { + spec!("comment.edit", "Edit comment", "{id | link, text, reply? (index; omit for the first message), author?}", true, |s, p| { let id = req_comment("comment.edit", s, p)?; let t = text("comment.edit", p)?; let who = author(s, p); @@ -441,7 +441,7 @@ pub fn specs() -> Vec { spec!( "comment.react", "React to comment", - "{id, emoji, reply? (index; omit for the first message), on? (default: toggle), author?}", + "{id | link, emoji, reply? (index; omit for the first message), on? (default: toggle), author?}", true, |s, p| { let id = req_comment("comment.react", s, p)?; @@ -463,7 +463,7 @@ pub fn specs() -> Vec { Ok(json!({"on": on})) } ), - spec!("comment.resolve", "Resolve comment", "{id, resolved? (default true)}", true, |s, p| { + spec!("comment.resolve", "Resolve comment", "{id | link, resolved? (default true)}", true, |s, p| { let id = req_comment("comment.resolve", s, p)?; let v = boolean(p, "resolved").unwrap_or(true); if let Some(c) = s.doc_mut()?.comment_mut(id) { @@ -474,7 +474,7 @@ pub fn specs() -> Vec { spec!( "comment.move", "Move comment", - "{id, x, y, anchor? (default true: pin to the innermost frame under the point; false: to the page; or a node id)}", + "{id | link, x, y, anchor? (default true: pin to the innermost frame under the point; false: to the page; or a node id)}", true, |s, p| { let id = req_comment("comment.move", s, p)?; @@ -489,24 +489,30 @@ pub fn specs() -> Vec { ok() } ), - spec!("comment.delete", "Delete comment", "{id, reply? (index: delete one reply; omit to delete the thread), author?}", true, |s, p| { - let id = req_comment("comment.delete", s, p)?; - let who = author(s, p); - let doc = s.doc_mut()?; - let c = doc.comment_mut(id).ok_or_else(|| bad("comment.delete", "no comment"))?; - match message_index("comment.delete", c, p)? { - 0 => { - own("comment.delete", &c.first, &who, "delete this thread")?; - doc.comments.retain(|c| c.id != id); - } - i => { - let m = c.message(i).ok_or_else(|| bad("comment.delete", "no message"))?; - own("comment.delete", m, &who, "delete this reply")?; - c.replies.remove(i - 1); + spec!( + "comment.delete", + "Delete comment", + "{id | link, reply? (index: delete one reply; omit to delete the thread), author?}", + true, + |s, p| { + let id = req_comment("comment.delete", s, p)?; + let who = author(s, p); + let doc = s.doc_mut()?; + let c = doc.comment_mut(id).ok_or_else(|| bad("comment.delete", "no comment"))?; + match message_index("comment.delete", c, p)? { + 0 => { + own("comment.delete", &c.first, &who, "delete this thread")?; + doc.comments.retain(|c| c.id != id); + } + i => { + let m = c.message(i).ok_or_else(|| bad("comment.delete", "no message"))?; + own("comment.delete", m, &who, "delete this reply")?; + c.replies.remove(i - 1); + } } + ok() } - ok() - }), + ), spec!( "comment.markRead", "Mark comment as read", @@ -545,7 +551,7 @@ pub fn specs() -> Vec { spec!( "comment.next", "Next comment", - "{id? (the thread to step from; omitted: the first or last), dir? (1 next, -1 previous; default 1), page?, filter?, sort?, query? (the list, as comment.list), author?}", + "{id? (the thread to step from; omitted: the first or last), dir? (1 next, -1 previous; default 1), page?, filter?, sort?, query?, frame?, resolved? (the list, as comment.list), author?}", false, |s, p| { let dir = num(p, "dir").unwrap_or(1.0); @@ -582,7 +588,7 @@ pub fn specs() -> Vec { } Ok(json!({"zoom": v.zoom})) }), - spec!("comment.link", "Copy link to comment", "{id}", false, |s, p| { + spec!("comment.link", "Copy link to comment", "{id | link}", false, |s, p| { let id = req_comment("comment.link", s, p)?; Ok(json!({"link": link(id)})) }), diff --git a/crates/engine/src/cmd/create.rs b/crates/engine/src/cmd/create.rs index 414a2310..51773629 100644 --- a/crates/engine/src/cmd/create.rs +++ b/crates/engine/src/cmd/create.rs @@ -53,37 +53,61 @@ pub fn specs() -> Vec { spec!( "node.create", "Create layer", - "{type: rectangle|ellipse|polygon|star|line|arrow|vector|text|frame|section|component|slice, x?, y? (relative to the parent frame; groups don't count), width?, height?, parent? (id; default: the page), index?, name?, text? (for text), path? (SVG path data for vector), props? {node.set keys}, select? (default true)}", + "{type: rectangle|ellipse|polygon|star|line|arrow|vector|text|frame|section|component|slice, x?, y? (relative to the parent frame; groups don't count), width?, height?, parent? (id; default: the page), index?, name?, text? | characters? (for text), path? | points? (vector: SVG path data | [[x,y],..]), closed? (vector), props? {node.set keys}, select? (default true)}", true, create ), spec!( "frame.create", "Create frame", - "{preset? (name from FRAME_PRESETS), x?, y?, width?, height?, parent?, name?, props?} (a preset without x/y lands to the right of the page's frames)", + "{preset? (name from FRAME_PRESETS), x?, y?, width?, height?, parent?, index?, name?, props?, select? (default true)} (a preset without x/y lands to the right of the page's frames)", true, frame_create ), - spec!("shape.rectangle", "Rectangle", "{x, y, width, height, parent?, props?}", true, |s, p| create_kind(s, p, "rectangle")), - spec!("shape.ellipse", "Ellipse", "{x, y, width, height, parent?, props?}", true, |s, p| create_kind(s, p, "ellipse")), - spec!("shape.polygon", "Polygon", "{x, y, width, height, parent?, props? {pointCount}}", true, |s, p| create_kind(s, p, "polygon")), - spec!("shape.star", "Star", "{x, y, width, height, parent?, props? {pointCount, innerRadius}}", true, |s, p| create_kind(s, p, "star")), - spec!("shape.line", "Line", "{x, y, width (length), parent?, props? {rotation}}", true, |s, p| create_kind(s, p, "line")), - spec!("shape.arrow", "Arrow", "{x, y, width (length), parent?, props?}", true, |s, p| create_kind(s, p, "arrow")), - spec!("text.create", "Text", "{text, x, y, parent?, width? (fixed width wraps), props? (fontSize, fontFamily, …)}", true, |s, p| { - create_kind(s, p, "text") + spec!("shape.rectangle", "Rectangle", "{x, y, width, height, parent?, index?, name?, props?, select? (default true)}", true, |s, p| { + create_kind(s, p, "rectangle") }), + spec!("shape.ellipse", "Ellipse", "{x, y, width, height, parent?, index?, name?, props?, select? (default true)}", true, |s, p| create_kind( + s, p, "ellipse" + )), + spec!( + "shape.polygon", + "Polygon", + "{x, y, width, height, parent?, index?, name?, props? {pointCount}, select? (default true)}", + true, + |s, p| create_kind(s, p, "polygon") + ), + spec!( + "shape.star", + "Star", + "{x, y, width, height, parent?, index?, name?, props? {pointCount, innerRadius}, select? (default true)}", + true, + |s, p| create_kind(s, p, "star") + ), + spec!("shape.line", "Line", "{x, y, width (length), parent?, index?, name?, props? {rotation}, select? (default true)}", true, |s, p| { + create_kind(s, p, "line") + }), + spec!("shape.arrow", "Arrow", "{x, y, width (length), parent?, index?, name?, props?, select? (default true)}", true, |s, p| create_kind( + s, p, "arrow" + )), + spec!( + "text.create", + "Text", + "{text | characters, x, y, parent?, index?, name? (default: the first line), width? (fixed width wraps), props? (fontSize, fontFamily, …), select? (default true)}", + true, + |s, p| { create_kind(s, p, "text") } + ), spec!( "vector.create", "Vector", - "{path (SVG path data) | points [[x,y],..], closed?, x?, y?, width?, height? (scale the path; one keeps its proportion), parent?, props?}", + "{path (SVG path data) | points [[x,y],..], closed?, x?, y?, width?, height? (scale the path; one keeps its proportion), parent?, index?, name?, props?, select? (default true)}", true, |s, p| create_kind(s, p, "vector") ), spec!( "image.place", "Place image", - "{data (base64) | path, x?, y?, width?, height?, parent?, target? (an existing layer: the image is added on top of its fills instead of making a layer), mode? (fill|fit|crop|tile)} → {id, image, width, height, imageProfile (its embedded colour profile; null: taken as sRGB)}", + "{data (base64) | path, x?, y?, width?, height?, parent?, name? (default Image), target? (an existing layer: the image is added on top of its fills instead of making a layer), mode? (fill|fit|crop|tile)} → {id, image, width, height, imageProfile (its embedded colour profile; null: taken as sRGB)}", true, place_image ), diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 7ec63c69..365e237b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -245,8 +245,8 @@ pub fn specs() -> Vec { s.doc_mut()?.reorder(i, idx)?; ok() }), - spec!("fill.add", "Add fill", "{ids?, paint? (default #D9D9D9 or white for frames)}", true, |s, p| paint_list(s, p, true, true)), - spec!("stroke.add", "Add stroke", "{ids?, paint? (default black)}", true, |s, p| paint_list(s, p, false, true)), + spec!("fill.add", "Add fill", "{ids?, paint? | color? (default #D9D9D9 or white for frames)}", true, |s, p| paint_list(s, p, true, true)), + spec!("stroke.add", "Add stroke", "{ids?, paint? | color? (default black)}", true, |s, p| paint_list(s, p, false, true)), spec!("fill.remove", "Remove fill", "{ids?, index}", true, |s, p| paint_list(s, p, true, false)), spec!("stroke.remove", "Remove stroke", "{ids?, index}", true, |s, p| paint_list(s, p, false, false)), spec!("fill.set", "Set fill", "{ids?, index?, paint | color, opacity?, visible?, blend?}", true, |s, p| paint_set(s, p, true)), @@ -270,7 +270,7 @@ pub fn specs() -> Vec { spec!( "fill.pattern", "Pattern fill", - "{source (layer id), ids?, index? (default the top fill), tile? (grid|horizontalHex|verticalHex), scale? (1 = the source's size), spacing? [x, y], align? (start|center|end), opacity?} the source layer tiled; it updates as the source changes", + "{source (layer id), ids?, index? (default the top fill), tile? (grid|horizontalHex|verticalHex), scale? (1 = the source's size), spacing? [x, y], align? (start|center|end), opacity?, visible?, blend?} the source layer tiled; it updates as the source changes", true, pattern_fill ), @@ -351,7 +351,7 @@ pub fn specs() -> Vec { spec!( "export.update", "Edit export setting", - "{ids?, index, size? (2x|512w|512h|{by, value}), suffix?, format? (png|jpg|svg|pdf|webp|xml: Android Vector Drawable), bleed? (PDF: points, or with a unit: \"3mm\", \"0.125in\", \"9pt\"), cropMarks?, bleedMarks?, cmyk? (PDF print options), vdCompat? (XML: bestEffort|exact|ignore)}", + "{ids?, index, size? (2x|512w|512h|{by, value}), suffix?, format? (png|jpg|svg|pdf|webp|xml: Android Vector Drawable), bleed? (PDF: points, or with a unit: \"3mm\", \"0.125in\", \"9pt\"), cropMarks?, bleedMarks?, cmyk? (PDF print options), vdCompat? (XML: bestEffort|exact|ignore), contentsOnly?, includeBoundingBox?, svgOutlineText?, svgIdAttribute?, svgSimplifyStroke?}", true, |s, p| { let cmd = "export.update"; diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index fed4bff1..2e63b1c9 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -107,7 +107,7 @@ pub fn specs() -> Vec { spec!( "file.export", "Export", - "{ids? (default selection, else the page), format? (png|jpg|svg|pdf|webp|xml (an Android Vector Drawable, with `warnings` for what it changed or left out), or fig: the whole file as .fig, experimental, with `warnings` for what was left out; other apps may not open every file), scale? (default 1) | size? (2x|512w|512h or {by: scale|width|height, value}: a scale, or a width or height in pixels), path? (else returns base64 `data`), bleed? (PDF: points, or with a unit: \"3mm\", \"0.125in\", \"9pt\"), cropMarks?, bleedMarks?, cmyk? (PDF print options), svgIdAttribute? (SVG ids, XML android:name), vdCompat? (XML: bestEffort (default: approximate with a warning)|exact (leave out what can't be exact, with a warning)|ignore (no warnings))}", + "{ids? (default selection, else the page), format? (png|jpg|svg|pdf|webp|xml (an Android Vector Drawable, with `warnings` for what it changed or left out), or fig: the whole file as .fig, experimental, with `warnings` for what was left out; other apps may not open every file), scale? (default 1) | size? (2x|512w|512h or {by: scale|width|height, value}: a scale, or a width or height in pixels), path? (else returns base64 `data`), bleed? (PDF: points, or with a unit: \"3mm\", \"0.125in\", \"9pt\"), cropMarks?, bleedMarks?, cmyk? (PDF print options), contentsOnly? (default true; false also draws the layers that show in their area), includeBoundingBox? (true: only the layers' own boxes, cutting off shadows and outside strokes), svgOutlineText? (default true), svgSimplifyStroke? (default true), svgIdAttribute? (SVG ids, XML android:name), vdCompat? (XML: bestEffort (default: approximate with a warning)|exact (leave out what can't be exact, with a warning)|ignore (no warnings))}", false, export ), diff --git a/crates/engine/src/cmd/proto.rs b/crates/engine/src/cmd/proto.rs index ce7219f4..32247e82 100644 --- a/crates/engine/src/cmd/proto.rs +++ b/crates/engine/src/cmd/proto.rs @@ -13,7 +13,7 @@ pub fn specs() -> Vec { spec!( "proto.interaction.add", "Add interaction", - "{id?, trigger? {type: onClick|onHover|onPress|onDrag|mouseEnter|mouseLeave|mouseDown|mouseUp|afterDelay {ms}|keyPress {key (a key or a combination: Shift+K, Ctrl+Alt+Delete; or a controller button: Gamepad A|B|X|Y|LB|RB|LT|RT|View|Menu|LS|RS|Up|Down|Left|Right|Home)}}, action {type: navigate {preserveScroll?}|back|scrollTo {offset? [x, y] (where the layer ends up from the top-left)}|openOverlay {position?, anchor? screen|triggerInside|triggerOutside (default screen; the trigger ones measure the position preset on the layer that opens the overlay: outside puts top presets above it and bottom presets below it, edges lined up), offset? [x, y] (position manual: the overlay's top-left from the top-left of the layer that opens it; without it, right below that layer), close_on_outside? (default true), background?}|swapOverlay|closeOverlay|openUrl {url}|changeTo|setVariable {variable, value | expression (text, e.g. \"count + 1\")}|setVariableMode {collection, mode}|conditional {blocks [{condition? | expression? (text, e.g. \"count >= 3\"), actions}]}, dest?}, actions? [..], transition? {type: instant|dissolve|smartAnimate|moveIn|moveOut|push|slideIn|slideOut, direction?: left|right|top|bottom}, durationMs?, easing? linear|easeIn|easeOut|easeInOut|easeInBack|easeOutBack|easeInOutBack|gentle|quick|bouncy|slow|{customBezier {x1, y1, x2, y2}}|{customSpring {mass, stiffness, damping}}}, delayMs? (mouse enter, leave, down and up: how long after the gesture the actions run, up to 600000)} — or {id, dest} for a click-navigate. Returns {index}", + "{id?, trigger? {type: onClick|onHover|onPress|onDrag|mouseEnter|mouseLeave|mouseDown|mouseUp|afterDelay {ms}|keyPress {key (a key or a combination: Shift+K, Ctrl+Alt+Delete; or a controller button: Gamepad A|B|X|Y|LB|RB|LT|RT|View|Menu|LS|RS|Up|Down|Left|Right|Home)}}, action {type: navigate {preserveScroll?}|back|scrollTo {offset? [x, y] (where the layer ends up from the top-left)}|openOverlay {position?, anchor? screen|triggerInside|triggerOutside (default screen; the trigger ones measure the position preset on the layer that opens the overlay: outside puts top presets above it and bottom presets below it, edges lined up), offset? [x, y] (position manual: the overlay's top-left from the top-left of the layer that opens it; without it, right below that layer), close_on_outside? (default true), background?}|swapOverlay|closeOverlay|openUrl {url}|changeTo|setVariable {variable, value | expression (text, e.g. \"count + 1\")}|setVariableMode {collection, mode}|conditional {blocks [{condition? | expression? (text, e.g. \"count >= 3\"), actions}]}, dest?}, actions? [..], dest? (without action: a click-navigate to it), transition? {type: instant|dissolve|smartAnimate|moveIn|moveOut|push|slideIn|slideOut, direction?: left|right|top|bottom}, durationMs? | duration_ms?, easing? linear|easeIn|easeOut|easeInOut|easeInBack|easeOutBack|easeInOutBack|gentle|quick|bouncy|slow|{customBezier {x1, y1, x2, y2}}|{customSpring {mass, stiffness, damping}}, delayMs? | delay_ms? (mouse enter, leave, down and up: how long after the gesture the actions run, up to 600000)} — or {id, dest} for a click-navigate. Returns {index}", true, add ), @@ -48,7 +48,7 @@ pub fn specs() -> Vec { spec!( "proto.interaction.set", "Edit interaction", - "{id?, index, interaction {..}} replaces it; or patch fields: {id?, index, trigger?, action? (first action), actions?, transition?, durationMs?, easing?, delayMs?}", + "{id?, index, interaction? {..} (replaces it whole), trigger?, action? (first action), actions?, transition?, durationMs? | duration_ms?, easing?, delayMs? | delay_ms?} patch fields on top of the current interaction (or of interaction)", true, set ), diff --git a/crates/engine/src/cmd/structure.rs b/crates/engine/src/cmd/structure.rs index 700825f2..58aaa7f9 100644 --- a/crates/engine/src/cmd/structure.rs +++ b/crates/engine/src/cmd/structure.rs @@ -80,7 +80,7 @@ pub fn specs() -> Vec { } ok() }), - spec!("layout.suggest", "Suggest auto layout", "{ids?}", true, layout_add), + spec!("layout.suggest", "Suggest auto layout", "{ids?, direction? (horizontal|vertical|grid)}", true, layout_add), spec!("object.moveToPage", "Move to page", "{ids?, page}", true, move_to_page), spec!("object.setThumbnail", "Set as thumbnail", "{id?}", true, |s, p| { let id = targets(s, p).first().copied().ok_or_else(|| bad("object.setThumbnail", "select a frame"))?; diff --git a/crates/engine/src/cmd/styles.rs b/crates/engine/src/cmd/styles.rs index 9c2922d2..8f212ea6 100644 --- a/crates/engine/src/cmd/styles.rs +++ b/crates/engine/src/cmd/styles.rs @@ -161,7 +161,7 @@ pub fn specs() -> Vec { spec!( "variable.create", "Create variable", - "{collection, name, type: color|number|string|boolean, value (default mode) | values {modeName: value}}", + "{collection, name, type: color|number|string|boolean, value (default mode) | values {modeName: value}, description?}", true, var_create ), diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 3822c5c5..4ad26ad4 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -276,13 +276,19 @@ pub fn specs() -> Vec { .ok_or_else(|| bad("text.decoration", "`decoration`: none|underline|strikethrough"))?; style(s, p, |st| st.decoration = d) }), - spec!("text.listStyle", "List style", "{style: none|bulleted|numbered, ids?} while editing: the selected paragraphs", true, |s, p| { - let l: ListStyle = get(p, "style") - .or_else(|| get(p, "list")) - .and_then(super::props::enum_of) - .ok_or_else(|| bad("text.listStyle", "`style`: none|bulleted|numbered"))?; - set_list(s, p, l, false) - }), + spec!( + "text.listStyle", + "List style", + "{style | list: none|bulleted|numbered, ids?} while editing: the selected paragraphs", + true, + |s, p| { + let l: ListStyle = get(p, "style") + .or_else(|| get(p, "list")) + .and_then(super::props::enum_of) + .ok_or_else(|| bad("text.listStyle", "`style`: none|bulleted|numbered"))?; + set_list(s, p, l, false) + } + ), spec!("text.paragraphIndent", "Paragraph indent", "{indent (px), ids?}", true, |s, p| { let v = finite(req_num("text.paragraphIndent", p, "indent")?).clamp(0.0, 10_000.0); para(s, p, |t| t.paragraph_indent = v) @@ -371,7 +377,7 @@ pub fn specs() -> Vec { spec!("text.endPreview", "End text preview", "{} puts the document back as it was before text.preview → {ended}", false, |s, _| { Ok(json!({"ended": crate::textpreview::end(s)})) }), - spec!("text.set", "Set text", "{id?, characters}", true, |s, p| { + spec!("text.set", "Set text", "{id?, characters | text}", true, |s, p| { let c = string(p, "characters").or_else(|| string(p, "text")).ok_or_else(|| bad("text.set", "`characters` required"))?; let ids = req_targets("text.set", s, p)?; let doc = s.doc_mut()?; diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index 7364557f..f17cde19 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -373,7 +373,7 @@ pub fn specs() -> Vec { spec!( "pref.uiScale", "Interface size", - "{scale?: a size from 0.75 to 3 relative to the system's display scale, 1 being the system's own size, so 1.25 is 125 %, not 125 (outside the range it stops at the nearest end; \"auto\" is 1; omit: just read it)} → {scale}: how large the whole interface is drawn; the canvas zoom is separate", + "{scale? (a size from 0.75 to 3 relative to the system's display scale: 1 is the system's own size, so 1.25 is 125 %, not 125; outside the range it stops at the nearest end; \"auto\" is 1; omit: just read it)} → {scale}: how large the whole interface is drawn; the canvas zoom is separate", false, |s, p| { match get(p, "scale") { diff --git a/crates/engine/src/dev/annotations.rs b/crates/engine/src/dev/annotations.rs index e6937d0e..7e4ef921 100644 --- a/crates/engine/src/dev/annotations.rs +++ b/crates/engine/src/dev/annotations.rs @@ -247,7 +247,7 @@ pub fn specs() -> Vec { spec!( "annotation.add", "Add annotation", - "{id? (default the first selected layer), label? (the note), properties?: [width|height|top|left|rotation|opacity|cornerRadius|direction|gap|padding|alignment|fontFamily|fontStyle|fontSize|lineHeight|letterSpacing|textAlign|fill|stroke|strokeWeight|effects], category? (id or name)} → {id, index} (a note, pinned properties or both)", + "{id? (default the first selected layer), label? | text? (the note), properties?: [width|height|top|left|rotation|opacity|cornerRadius|direction|gap|padding|alignment|fontFamily|fontStyle|fontSize|lineHeight|letterSpacing|textAlign|fill|stroke|strokeWeight|effects], category? (id or name)} → {id, index} (a note, pinned properties or both)", true, |s, p| { let cmd = "annotation.add"; @@ -270,7 +270,7 @@ pub fn specs() -> Vec { spec!( "annotation.edit", "Edit annotation", - "{id?, index, label?, properties?: [..] (replaces the pinned properties), category? (id or name; null: none)}", + "{id?, index, label? | text?, properties?: [..] (replaces the pinned properties), category? (id or name; null: none)}", true, |s, p| { let cmd = "annotation.edit"; diff --git a/crates/engine/src/sharing/live/join.rs b/crates/engine/src/sharing/live/join.rs index 972198ef..38101d85 100644 --- a/crates/engine/src/sharing/live/join.rs +++ b/crates/engine/src/sharing/live/join.rs @@ -166,7 +166,7 @@ pub fn specs() -> Vec { spec!( "collab.share", "Share with a join code", - "{via?: \"online\" (default: the open file is kept on the server in Preferences, `pref.publishServer`, and the server hands out the codes) | \"local\" (this app hosts it for people on the same network; no server), access?: \"edit\" (default) | \"view\" (online only: which code `code` is), server? (online: another server's address, with ?key=… when it needs its key), name? (online: the file's name there), port?, bind?: \"lan\" (default) | \"local\" (this machine only) | an IP address (local)} → the open file's session as `collab.peers` gives it: `code` is what others type to join (`collab.join {code}`, the Join field), `link` an invite link. Local: at once, the code carries the host's IPv4 address, port and secret (K7M2Q-…, 20 symbols). Online: status \"connecting\" first; once live `code` is the server's (K7M-2QX-9TD) and `share` is {via, access, shared, editCode, viewCode, editLink, viewLink}: links made for others (the file's own links in `published` stay yours); `code` stays null when the server is older than join codes (give `share.editLink` / `share.viewLink` instead). Called again while shared: only switches which code `code` is (`access`). Called after `collab.stop` on a file that is still on the server: new links and codes for that same file (nothing is published twice). `collab.stop` stops sharing", + "{via?: \"online\" (default: the open file is kept on the server in Preferences, `pref.publishServer`, and the server hands out the codes) | \"local\" (this app hosts it for people on the same network; no server), access?: \"edit\" (default) | \"view\" (online only: which code `code` is), server? (online: another server's address, with ?key=… when it needs its key), key? (online: the server's key, when the address has none), name? (online: the file's name there), port?, bind?: \"lan\" (default) | \"local\" (this machine only) | an IP address (local)} → the open file's session as `collab.peers` gives it: `code` is what others type to join (`collab.join {code}`, the Join field), `link` an invite link. Local: at once, the code carries the host's IPv4 address, port and secret (K7M2Q-…, 20 symbols). Online: status \"connecting\" first; once live `code` is the server's (K7M-2QX-9TD) and `share` is {via, access, shared, editCode, viewCode, editLink, viewLink}: links made for others (the file's own links in `published` stay yours); `code` stays null when the server is older than join codes (give `share.editLink` / `share.viewLink` instead). Called again while shared: only switches which code `code` is (`access`). Called after `collab.stop` on a file that is still on the server: new links and codes for that same file (nothing is published twice). `collab.stop` stops sharing", false, share ), @@ -180,7 +180,7 @@ pub fn specs() -> Vec { spec!( "collab.join", "Join live session", - "{code: a join code (K7M-2QX-9TD from a server, asked at the server in Preferences; a longer one for the local network; spaces, dashes and case don't matter, O reads as 0 and I or L as 1) | link: figcraft://join?host=…&file=…&token=…, figcraft://join?server=ws(s)://…&file=…&token=… or ws(s)://host:port/?file=…&token=… (a session an app hosts, or a server's share link; a code here works too)} → the new session: {session, status: \"connecting\", …}; its file opens in a tab of its own (other files' sessions go on) and `collab.peers` tells when it's live (`sessions` lists it until then, and if it fails)", + "{code | link (a join code: K7M-2QX-9TD from a server, asked at the server in Preferences; a longer one for the local network; spaces, dashes and case don't matter, O reads as 0 and I or L as 1; or an invite link: figcraft://join?host=…&file=…&token=…, figcraft://join?server=ws(s)://…&file=…&token=… or ws(s)://host:port/?file=…&token=…, from a session an app hosts or a server's share link; either key takes either)} → the new session: {session, status: \"connecting\", …}; its file opens in a tab of its own (other files' sessions go on) and `collab.peers` tells when it's live (`sessions` lists it until then, and if it fails)", false, join ), @@ -208,7 +208,7 @@ pub fn specs() -> Vec { spec!( "collab.manageFile", "Manage published file", - "{file (its id), server?, owner? (the server's address and the owner token you got when you published the file without the key; each defaults to what is recorded for one of your online files, see `collab.files`), action: links | createLink {access?: view (default) | edit, label?} | revoke {link (a link's id)} | share (others get new links and join codes) | unshare (the links and codes others got are revoked; the file stays yours on the server) | rename {name} | remove (the file is deleted from the server and leaves your online files)} → {links: [{id, access, label, created}]} | {id, access, link (a new share link, shown once)} | {shared, code, viewCode, editLink, viewLink} | {done}", + "{file (its id), server?, owner? (the server's address and the owner token you got when you published the file without the key; each defaults to what is recorded for one of your online files, see `collab.files`), action: links | createLink | revoke | share (others get new links and join codes) | unshare (the links and codes others got are revoked; the file stays yours on the server) | rename | remove (the file is deleted from the server and leaves your online files), access? (createLink: view (default) | edit), label? (createLink), link? (revoke: a link's id), name? (rename)} → {links: [{id, access, label, created}]} | {id, access, link (a new share link, shown once)} | {shared, code, viewCode, editLink, viewLink} | {done}", false, manage_file ), From ff4fe00294bdb53df5dc3c2b80deefede2b9ccdb Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 09:06:58 +0100 Subject: [PATCH 05/12] M0.9: commands refuse parameter names they don't take (#1413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A misspelt parameter name was ignored and the command reported success, answering as if the parameter hadn't been given (`comment.list {"filterr": "open"}` listed resolved threads). `Session::execute` now refuses names the command's params doc doesn't list, naming them and what the command takes, before the command runs, so every path into a command (CLI, batches, control channel, MCP, plugins, the UI) gets the check. The names come from the existing docs, parsed once with the registry: each top-level item's first word and the words joined to it by `|`, `/` or `+`, bracketed text and what follows `:`, `=` or `→` being description, alternative groups (`{x, y} | {points}`) included. `ids` and `id` are always taken (`params::targets` reads either); node.set, whose doc is its props list, keeps its own check. The junk sweep runs each junk object as given and cut down to the names a command takes, so the junk values still reach the parsers. Tests that passed names their command never read (comment.resolve's author, collab.host's link, node.get's depth) pass only what it takes. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd.rs | 112 +++++++++++++++++- crates/engine/src/history_behaviour_tests.rs | 2 +- crates/engine/src/lib.rs | 2 + crates/engine/src/live_tests.rs | 4 +- crates/engine/src/session/exec.rs | 1 + crates/engine/src/tests.rs | 18 ++- crates/engine/src/unknown_params_tests.rs | 85 +++++++++++++ .../ui-egui/src/gradient_shorthand_tests.rs | 2 +- docs/control-protocol.md | 3 +- 9 files changed, 218 insertions(+), 11 deletions(-) create mode 100644 crates/engine/src/unknown_params_tests.rs diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index a247ed52..c046cd19 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -63,6 +63,8 @@ macro_rules! spec { struct Registry { list: Vec, index: HashMap<&'static str, usize>, + /// The parameter names each command's doc lists (`list` order; see [`doc_param_names`]). + names: Vec>>, } fn registry() -> &'static Registry { @@ -123,7 +125,8 @@ fn registry() -> &'static Registry { log::warn!("duplicate command id {}", s.id); } } - Registry { list, index } + let names = list.iter().map(|s| doc_param_names(s.params)).collect(); + Registry { list, index, names } }) } @@ -136,6 +139,113 @@ pub fn find_command(id: &str) -> Option<&'static CommandSpec> { r.index.get(id).and_then(|i| r.list.get(*i)) } +/// The parameter names command `id` takes, from its doc; `None` when the doc isn't a `{…}` list +/// (`node.set` documents its props, and refuses the ones it doesn't know itself). +pub fn param_names(id: &str) -> Option<&'static [&'static str]> { + let r = registry(); + r.index.get(id).and_then(|i| r.names.get(*i)).and_then(|n| n.as_deref()) +} + +/// Refuse params naming something the command doesn't take: a misspelt name would otherwise be +/// ignored and the command answer as if it hadn't been given. `ids` and `id` are always taken: +/// the layers a command acts on (`params::targets` reads either). +pub fn refuse_unknown_params(id: &str, params: &Value) -> Result<()> { + let (Some(names), Some(given)) = (param_names(id), params.as_object()) else { return Ok(()) }; + let taken = |k: &str| matches!(k, "ids" | "id") || names.contains(&k); + let unknown: Vec = given.keys().filter(|k| !taken(k)).map(|k| format!("`{k}`")).collect(); + if unknown.is_empty() { + return Ok(()); + } + let s = if unknown.len() == 1 { "" } else { "s" }; + let takes = if names.is_empty() { "none".to_string() } else { names.join(", ") }; + Err(crate::bad(id, format!("unknown parameter{s} {} (it takes {takes})", unknown.join(", ")))) +} + +/// The top-level items of a `{…}` params doc (`{ids?, direction? (horizontal|vertical)}` → +/// `ids?`, ` direction? (horizontal|vertical)`), with those of the alternatives after it +/// (`{x, y} | {points}`); `None` when the doc doesn't start with one. +pub(crate) fn doc_items(doc: &'static str) -> Option> { + let mut body = doc.trim_start().strip_prefix('{')?; + let (mut items, mut depth, mut start) = (Vec::new(), 0usize, 0); + loop { + let mut rest = None; + for (i, c) in body.char_indices() { + match c { + '{' | '[' | '(' => depth += 1, + '}' | ']' | ')' if depth > 0 => depth -= 1, + '}' | ',' if depth == 0 => { + items.push(body.get(start..i).unwrap_or_default()); + start = i + c.len_utf8(); + if c == '}' { + rest = body.get(start..); + break; + } + } + _ => {} + } + } + let rest = rest?; + match rest.trim_start().strip_prefix('|').and_then(|r| r.trim_start().strip_prefix('{')) { + Some(next) => (body, start) = (next, 0), + None => return Some(items), + } + } +} + +/// The names one doc item gives: its first word and the words joined to it by `|`, `/` or `+` +/// (`id?/ids?`, `path | data (base64) + name`, `value? + newValue?`), without the `?` of optional +/// ones. Bracketed text describes a name, and what follows a `:`, `=` or `→` its value. +pub(crate) fn item_names(item: &'static str) -> Vec<&'static str> { + let mut outside = Vec::new(); + let (mut depth, mut start) = (0usize, None); + for (i, c) in item.char_indices() { + let value_follows = depth == 0 && matches!(c, ':' | '=' | '→'); + let in_word = depth == 0 && !value_follows && !c.is_whitespace() && !matches!(c, '(' | '[' | '{' | ')' | ']' | '}'); + match c { + '(' | '[' | '{' => depth += 1, + ')' | ']' | '}' => depth = depth.saturating_sub(1), + _ => {} + } + match (in_word, start) { + (true, None) => start = Some(i), + (false, Some(s)) => { + outside.push(item.get(s..i).unwrap_or_default()); + start = None; + } + _ => {} + } + if value_follows { + break; + } + } + if let Some(s) = start { + outside.push(item.get(s..).unwrap_or_default()); + } + let is_name = + |n: &&str| n.chars().next().is_some_and(|c| c.is_ascii_alphabetic() || c == '_') && n.chars().all(|c| c.is_ascii_alphanumeric() || c == '_'); + let mut words = outside.into_iter(); + let (mut names, mut word) = (Vec::new(), words.next()); + while let Some(w) = word { + names.extend(w.split(['/', '|']).map(|n| n.trim_end_matches('?')).filter(is_name)); + word = match words.next() { + Some("|" | "/" | "+") => words.next(), + _ => None, + }; + } + names +} + +/// Every name a `{…}` params doc lists, once each (see [`item_names`]). +fn doc_param_names(doc: &'static str) -> Option> { + let mut names = Vec::new(); + for n in doc_items(doc)?.into_iter().flat_map(item_names) { + if !names.contains(&n) { + names.push(n); + } + } + Some(names) +} + /// `{"ok": true}` shorthand. pub fn ok() -> Result { Ok(serde_json::json!({"ok": true})) diff --git a/crates/engine/src/history_behaviour_tests.rs b/crates/engine/src/history_behaviour_tests.rs index 45e0c7bf..8cd5612e 100644 --- a/crates/engine/src/history_behaviour_tests.rs +++ b/crates/engine/src/history_behaviour_tests.rs @@ -188,7 +188,7 @@ fn threads_replies_and_resolving() { |c| c.replies.iter().map(|m| m.text.as_str()).eq(["Sure", "Done"]), "replies stay in the order they were written", ), - ("comment.resolve", json!({"id": c, "author": "Sam"}), |c| c.resolved, "anyone in the file can resolve a thread, not just its author"), + ("comment.resolve", json!({"id": c}), |c| c.resolved, "anyone in the file can resolve a thread, not just its author (it takes no author)"), ("comment.resolve", json!({"id": c, "resolved": false}), |c| !c.resolved, "a resolved thread can be reopened"), ("comment.resolve", json!({"id": c}), |c| c.resolved, "resolving again marks it resolved"), ( diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ea135ca1..37fcaf7d 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -484,6 +484,8 @@ mod tiff_tests; #[cfg(test)] mod transform_behaviour_tests; #[cfg(test)] +mod unknown_params_tests; +#[cfg(test)] mod variable_tests; #[cfg(test)] mod variables_behaviour_tests; diff --git a/crates/engine/src/live_tests.rs b/crates/engine/src/live_tests.rs index 5cdccf69..1bae1b10 100644 --- a/crates/engine/src/live_tests.rs +++ b/crates/engine/src/live_tests.rs @@ -158,8 +158,8 @@ fn viewport(page: NodeId, x: f64, y: f64, w: f64, h: f64) -> Presence { #[test] fn without_a_link_live_sessions_are_unavailable() { let mut s = Session::new(); - for cmd in ["collab.host", "collab.join"] { - let e = s.execute(cmd, &json!({"link": "figcraft://join?host=a:1&file=f&token=t"})).unwrap_err(); + for (cmd, p) in [("collab.host", json!({})), ("collab.join", json!({"link": "figcraft://join?host=a:1&file=f&token=t"}))] { + let e = s.execute(cmd, &p).unwrap_err(); assert!(matches!(e, EngineError::Disabled(..)), "{cmd}: {e}"); } assert_eq!(s.execute("collab.leave", &json!({})).unwrap()["ended"], false); diff --git a/crates/engine/src/session/exec.rs b/crates/engine/src/session/exec.rs index add12ac5..90bcb407 100644 --- a/crates/engine/src/session/exec.rs +++ b/crates/engine/src/session/exec.rs @@ -18,6 +18,7 @@ impl Session { /// as it was. pub fn execute(&mut self, id: &str, params: &Value) -> Result { let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.to_string()))?; + crate::cmd::refuse_unknown_params(id, params)?; board::refuse_on_board(self, id)?; // Revert replaces the document's uid: refuse before ending previews or syncing branches. if id == "file.revert" && self.live().doc_uid.is_some() { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 811724f2..fd64e7ab 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -478,11 +478,19 @@ fn every_command_survives_junk_params() { continue; } for j in &junk { - let mut s = sess(); - let _ = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})); - let _ = s.execute("text.create", &json!({"text": "t"})); - if let Err(EngineError::Internal { cmd, msg }) = s.execute(spec.id, j) { - panic!("{cmd} panicked on {j}: {msg}"); + // Each junk object as given (names the command doesn't take are refused) and cut down + // to the names it takes, so the junk values still reach its parsers. + let mut taken = j.clone(); + if let (Some(names), Some(o)) = (crate::cmd::param_names(spec.id), taken.as_object_mut()) { + o.retain(|k, _| matches!(k.as_str(), "ids" | "id") || names.contains(&k.as_str())); + } + for params in if taken == *j { vec![j] } else { vec![j, &taken] } { + let mut s = sess(); + let _ = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})); + let _ = s.execute("text.create", &json!({"text": "t"})); + if let Err(EngineError::Internal { cmd, msg }) = s.execute(spec.id, params) { + panic!("{cmd} panicked on {params}: {msg}"); + } } } } diff --git a/crates/engine/src/unknown_params_tests.rs b/crates/engine/src/unknown_params_tests.rs new file mode 100644 index 00000000..4124085f --- /dev/null +++ b/crates/engine/src/unknown_params_tests.rs @@ -0,0 +1,85 @@ +//! A parameter name a command doesn't take is refused, not ignored: a misspelt name would make the +//! command answer as if it hadn't been given (#1413). + +use serde_json::{Value, json}; + +use crate::Session; +use crate::cmd::{all_commands, param_names}; + +fn err(s: &mut Session, cmd: &str, params: Value) -> String { + match s.execute(cmd, ¶ms) { + Err(e) => e.to_string(), + Ok(v) => panic!("{cmd} {params} ran: {v}"), + } +} + +/// The issue's repro: a misspelt `filter` listed the resolved thread as if no filter were given. +#[test] +fn a_misspelt_parameter_is_refused_and_named() { + let mut s = Session::new(); + s.execute("comment.add", &json!({"x": 10, "y": 10, "text": "a"})).unwrap(); + s.execute("comment.resolve", &json!({"id": 1})).unwrap(); + let e = err(&mut s, "comment.list", json!({"filterr": "open"})); + assert!(e.contains("unknown parameter `filterr`") && e.contains("it takes page, filter, sort"), "{e}"); + assert_eq!(s.execute("comment.list", &json!({"filter": "open"})).unwrap()["comments"].as_array().map(Vec::len), Some(0)); + let e = err(&mut s, "edit.undo", json!({"steps": 2, "stepz": 1})); + assert!(e.contains("unknown parameters `steps`, `stepz` (it takes none)"), "{e}"); +} + +/// `ids` and `id` name the layers a command acts on, documented or not (`params::targets` reads +/// either), so `{id}` keeps working where a doc only says `ids?`. +#[test] +fn ids_and_id_are_always_taken() { + let mut s = Session::new(); + let r = crate::params::id_of(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()["id"]).unwrap(); + assert_eq!(param_names("align.left"), Some(&["ids"][..])); + s.execute("align.left", &json!({"id": r})).unwrap(); + s.execute("edit.undo", &json!({"ids": []})).unwrap(); +} + +/// Names a doc gives as alternatives (`id? | ids?`, `id?/ids?`, `path | data`) are all taken, and +/// `null` params or no params at all stay fine. +#[test] +fn documented_alternatives_and_empty_params_are_taken() { + assert_eq!(param_names("vector.fillRule").map(|n| n.contains(&"id") && n.contains(&"ids")), Some(true)); + assert_eq!(param_names("text.setFeatures").map(|n| n.contains(&"id") && n.contains(&"ids")), Some(true)); + assert_eq!(param_names("file.import").map(|n| n.contains(&"path") && n.contains(&"data") && n.contains(&"name")), Some(true)); + assert_eq!(param_names("file.export").map(|n| n.contains(&"scale") && n.contains(&"size")), Some(true)); + assert_eq!(param_names("collab.share").map(|n| n.contains(&"via") && !n.contains(&"local")), Some(true)); + assert_eq!(param_names("component.editProperty").map(|n| n.contains(&"value") && n.contains(&"newValue")), Some(true)); + // Docs whose shape used to hide names: a stray brace, a comma in an example, a second object. + assert_eq!(param_names("proto.interaction.add").map(|n| n.contains(&"delayMs") && n.contains(&"dest")), Some(true)); + assert_eq!(param_names("collab.join"), Some(&["code", "link"][..])); + assert_eq!(param_names("proto.interaction.set").map(|n| n.contains(&"trigger") && n.contains(&"interaction")), Some(true)); + // Alternative groups: `{x, y} | {points: …}`. + assert_eq!(param_names("vector.edit.paint"), Some(&["x", "y", "points"][..])); + assert_eq!(param_names("vector.edit.cut"), Some(&["segment", "t", "vertex", "from", "to"][..])); + // `node.set` documents its props and checks them itself. + assert_eq!(param_names("node.set"), None); + let mut s = Session::new(); + for p in [json!(null), json!({})] { + s.execute("select.all", &p).unwrap(); + } +} + +/// Every top-level item of every `{…}` params doc gives a name, so no parameter a doc describes is +/// refused. +#[test] +fn every_doc_item_names_a_parameter() { + let nameless: Vec = all_commands() + .iter() + .flat_map(|c| crate::cmd::doc_items(c.params).unwrap_or_default().into_iter().map(move |i| (c.id, i))) + .filter(|(_, i)| !i.trim().is_empty() && crate::cmd::item_names(i).is_empty()) + .map(|(id, i)| format!("{id}: `{i}`")) + .collect(); + assert!(nameless.is_empty(), "doc items without a name: {nameless:#?}"); +} + +/// A batch step's unknown parameter is refused, naming the step. +#[test] +fn batch_steps_refuse_unknown_parameters() { + let mut s = Session::new(); + let e = + err(&mut s, "command.batch", json!({"commands": [{"command": "select.all"}, {"command": "comment.list", "params": {"filterr": "open"}}]})); + assert!(e.contains("comment.list") && e.contains("step 1: unknown parameter `filterr`"), "{e}"); +} diff --git a/crates/ui-egui/src/gradient_shorthand_tests.rs b/crates/ui-egui/src/gradient_shorthand_tests.rs index db368f8e..0e5a9f0f 100644 --- a/crates/ui-egui/src/gradient_shorthand_tests.rs +++ b/crates/ui-egui/src/gradient_shorthand_tests.rs @@ -14,7 +14,7 @@ fn engine_execute_takes_the_gradient_shorthand() { let r = exec(&mut u, "shape.rectangle", json!({"x": 0, "y": 0, "width": 200, "height": 100})).unwrap()["id"].clone(); let short = json!({"type": "linearGradient", "stops": ["#ff0000", "#0000ff"], "from": [0, 0.5], "to": [1, 0.5]}); exec(&mut u, "node.set", json!({"ids": [r], "fills": [short]})).unwrap(); - let n = exec(&mut u, "node.get", json!({"id": r, "depth": 0})).unwrap(); + let n = exec(&mut u, "node.get", json!({"id": r})).unwrap(); let f = &n["fills"][0]; assert_eq!((f["type"].clone(), f["kind"].clone(), f["width"].clone()), (json!("gradient"), json!("linear"), json!([0.0, 1.5])), "{n}"); assert_eq!(f["stops"].as_array().map(Vec::len), Some(2), "{n}"); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 448a9979..8c03bda5 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -97,7 +97,8 @@ Run any command (the same ids as the menus, shortcuts, Actions palette, CLI and calls never open dialogs: dialog requests are dropped. Returns the command's result (creation commands return `{"id": N}`). Paints (`fills`, `strokes`, `paint`) take a colour, the full paint JSON or the gradient shorthand `{"type":"linearGradient","stops":["#hex","#hex"],"from"?,"to"?}` -(as in `docs/mcp.md`). +(as in `docs/mcp.md`). A parameter the command's doc (`engine.commands`) doesn't list is an error +naming it and the ones it takes, never ignored; `ids` and `id` are always taken. ```json {"id":1,"method":"engine.execute","params":{"command":"shape.rectangle","params":{"x":10,"y":10,"width":100,"height":50}}} From 27c11858e1aca954705c4f88891f9a45fdef12f8 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 09:06:59 +0100 Subject: [PATCH 06/12] M0.9: MCP tools and batch steps refuse names they don't take (#1413) `run_command {"command": "inspect.code", "args": {...}}` ran the command without its params, on the selection. MCP tool arguments are now checked against each tool's input schema (and the objects it describes inside, such as a batch's steps; open objects like `params` take anything), and a step key other than `command`, `params` and `as` is refused in command.batch, figcraft-cli scripts and the MCP batch tool. Co-Authored-By: Claude Opus 5.5 --- apps/figcraft-cli/src/files.rs | 4 +++ crates/engine/src/automation/batch.rs | 12 +++++++++ crates/engine/src/cmd/edit.rs | 1 + crates/engine/src/unknown_params_tests.rs | 7 +++-- crates/mcp/src/tools.rs | 32 +++++++++++++++++++++++ crates/mcp/tests/it/protocol.rs | 7 +++++ docs/cli.md | 4 ++- docs/mcp.md | 4 +++ 8 files changed, 68 insertions(+), 3 deletions(-) diff --git a/apps/figcraft-cli/src/files.rs b/apps/figcraft-cli/src/files.rs index 1ba9d60a..8e689ec0 100644 --- a/apps/figcraft-cli/src/files.rs +++ b/apps/figcraft-cli/src/files.rs @@ -366,6 +366,7 @@ pub fn parse_script(text: &str) -> Result, String> { .enumerate() .map(|(i, s)| { let cmd = s.get("command").and_then(Value::as_str).ok_or_else(|| format!("step {i}: `command` (a string) is required"))?; + figcraft_engine::batch::refuse_unknown_step_keys(i, &s).map_err(|e| e.to_string())?; let params = match s.get("params") { None | Some(Value::Null) => json!({}), Some(p @ Value::Object(_)) => p.clone(), @@ -443,6 +444,9 @@ mod tests { assert!(parse_script("[{\"command\":\"a\",\"params\":3}]").is_err()); assert!(parse_script("not json\n").is_err()); assert!(parse_script("42").is_err()); + // `args` for `params` would run the step without them (#1413). + let e = parse_script(r#"[{"command":"a"},{"command":"b","args":{"x":1}}]"#).unwrap_err().to_string(); + assert!(e.contains("step 1: unknown key `args`"), "{e}"); } #[test] diff --git a/crates/engine/src/automation/batch.rs b/crates/engine/src/automation/batch.rs index 5c4550ec..aa11a2bb 100644 --- a/crates/engine/src/automation/batch.rs +++ b/crates/engine/src/automation/batch.rs @@ -39,6 +39,18 @@ pub fn step_name(step: usize, json: &Value) -> Result> { } } +/// Refuse a step key other than `command`, `params` and `as` (`step` is its index, for the +/// error): `args` in place of `params` would run the command without them. +pub fn refuse_unknown_step_keys(step: usize, json: &Value) -> Result<()> { + let Some(o) = json.as_object() else { return Ok(()) }; + let unknown: Vec = o.keys().filter(|k| !matches!(k.as_str(), "command" | "params" | "as")).map(|k| format!("`{k}`")).collect(); + if unknown.is_empty() { + return Ok(()); + } + let s = if unknown.len() == 1 { "" } else { "s" }; + Err(bad("command.batch", format!("step {step}: unknown key{s} {} (a step takes command, params and as)", unknown.join(", ")))) +} + /// The steps of one batch run so far: their results, and which step each `as` name belongs to. /// A nested `command.batch` step gets its own scope, so its references are to its own steps. #[derive(Clone, Debug, Default)] diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 365e237b..e1a2e6bd 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -384,6 +384,7 @@ pub fn specs() -> Vec { for (i, c) in a.iter().enumerate() { let cmd = c.get("command").and_then(Value::as_str).ok_or_else(|| bad("command.batch", format!("step {i} has no `command`")))?; + crate::batch::refuse_unknown_step_keys(i, c)?; let params = c.get("params").cloned().unwrap_or(json!({})); out.push(BatchStep { name: crate::batch::step_name(i, c)?, ..BatchStep::new(cmd, params) }); } diff --git a/crates/engine/src/unknown_params_tests.rs b/crates/engine/src/unknown_params_tests.rs index 4124085f..02290ba3 100644 --- a/crates/engine/src/unknown_params_tests.rs +++ b/crates/engine/src/unknown_params_tests.rs @@ -75,10 +75,13 @@ fn every_doc_item_names_a_parameter() { assert!(nameless.is_empty(), "doc items without a name: {nameless:#?}"); } -/// A batch step's unknown parameter is refused, naming the step. +/// A batch step key other than `command`, `params` and `as` (`args` for `params`) is refused, and +/// so is a step's unknown parameter, naming the step. #[test] -fn batch_steps_refuse_unknown_parameters() { +fn batch_steps_refuse_unknown_keys_and_parameters() { let mut s = Session::new(); + let e = err(&mut s, "command.batch", json!({"commands": [{"command": "select.all"}, {"command": "inspect.code", "args": {"id": 2}}]})); + assert!(e.contains("step 1: unknown key `args`"), "{e}"); let e = err(&mut s, "command.batch", json!({"commands": [{"command": "select.all"}, {"command": "comment.list", "params": {"filterr": "open"}}]})); assert!(e.contains("comment.list") && e.contains("step 1: unknown parameter `filterr`"), "{e}"); diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 1b23191e..46cc1a83 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -414,12 +414,44 @@ fn friendly_params(p: &mut Value) { // ---------- dispatch ---------- +/// Refuse argument names `schema` doesn't list, at the top and in the objects it describes inside +/// (a batch's steps); objects it leaves open (`params`) take any. `path` leads the names in the +/// error (`commands[0].`). +fn refuse_unknown_args(schema: &Value, args: &Value, path: &str) -> Result<(), String> { + if let (Some(props), Some(given)) = (schema.get("properties").and_then(Value::as_object), args.as_object()) { + let unknown: Vec = given.keys().filter(|k| !props.contains_key(*k)).map(|k| format!("`{path}{k}`")).collect(); + if !unknown.is_empty() { + let s = if unknown.len() == 1 { "" } else { "s" }; + let takes: Vec<&str> = props.keys().map(String::as_str).collect(); + let takes = if takes.is_empty() { "none".to_string() } else { takes.join(", ") }; + return Err(format!("unknown argument{s} {} (it takes {takes})", unknown.join(", "))); + } + for (k, v) in given { + if let Some(inner) = props.get(k) { + refuse_unknown_args(inner, v, &format!("{path}{k}."))?; + } + } + } + if let (Some(items), Some(list)) = (schema.get("items"), args.as_array()) { + for (i, v) in list.iter().enumerate() { + refuse_unknown_args(items, v, &format!("{}[{i}].", path.trim_end_matches('.')))?; + } + } + Ok(()) +} + /// Run one tool. Every failure becomes an `isError` result (never a panic). pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult { let a = match args_obj(args) { Ok(a) => a, Err(e) => return ToolResult::error(e), }; + let tools = tool_definitions(); + if let Some(schema) = tools.iter().find(|t| t.get("name").and_then(Value::as_str) == Some(name)).and_then(|t| t.get("inputSchema")) + && let Err(e) = refuse_unknown_args(schema, &Value::Object(a.clone()), "") + { + return ToolResult::error(e); + } let r = match name { "list_commands" => list_commands(b, &a), "run_command" => run_command(b, &a), diff --git a/crates/mcp/tests/it/protocol.rs b/crates/mcp/tests/it/protocol.rs index 617790f4..5cc45209 100644 --- a/crates/mcp/tests/it/protocol.rs +++ b/crates/mcp/tests/it/protocol.rs @@ -75,6 +75,13 @@ fn tools_call_happy_and_error_paths() { assert!(c.err("pointer", json!({"x": 1})).contains("`y`")); assert!(c.err("type_text", json!({"text": "hi"})).contains("not editing text")); assert!(c.err("open_file", json!({"path": "/definitely/not/here.figcraft"})).contains("here.figcraft")); + // Names a tool or a command doesn't take are refused, not ignored (#1413). + let e = c.err("run_command", json!({"command": "inspect.code", "args": {"id": id, "language": "html"}})); + assert!(e.contains("unknown argument `args` (it takes command, params)"), "{e}"); + let e = c.err("batch", json!({"commands": [{"command": "comment.list", "args": {"filter": "open"}}]})); + assert!(e.contains("unknown argument `commands[0].args`"), "{e}"); + let e = c.err("run_command", json!({"command": "comment.list", "params": {"filterr": "open"}})); + assert!(e.contains("unknown parameter `filterr`"), "{e}"); // Protocol-level errors. let r = c.rpc("tools/call", json!({"name": "no_such_tool", "arguments": {}})); assert_eq!(r["error"]["code"], -32602); diff --git a/docs/cli.md b/docs/cli.md index 39863bea..80bab2a4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -133,7 +133,9 @@ Runs commands in a headless session (a new document, or `--in`), printing one JS ``` The whole script is **one undo step**, and it is **all-or-nothing**, exactly like running the same -commands through `command.batch` (the MCP tool `batch`, the control channel, plugins). A step named +commands through `command.batch` (the MCP tool `batch`, the control channel, plugins). A step key +other than `command`, `params` and `as`, or a parameter the command's doc (`figcraft-cli commands`) +doesn't list, fails the script with an error naming it (`ids` and `id` are always taken). A step named with `"as"` can be used by later steps: `{"$ref": "name"}` in a step's params is the `id` in that step's result (`{"$step": n}` does the same by zero-based position), and `"field"` takes another key or a slash path such as `ids/0`. So a step can target a layer an earlier step of the same script diff --git a/docs/mcp.md b/docs/mcp.md index 05dfd1e6..01754735 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1866,6 +1866,10 @@ turned off 0°. Tool failures come back as results with `isError: true` and a message; protocol problems (unknown tool, malformed JSON, wrong parameter types) are JSON-RPC errors. Nothing crashes the server. +A name a tool or command doesn't take (`run_command {args}` for `params`, a step key other than +`command`, `params` and `as`, or a parameter its `list_commands` entry doesn't list, such as +`comment.list {filterr}`) is an error that lists the names it does take, never ignored; `ids` and +`id` are taken by every command. ## Resources From 40b9288f70d602d024eb963dbb895c7d4ddbdaca Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 09:19:56 +0100 Subject: [PATCH 07/12] M1: opening a file that is already open says when it changed on disk, and can read it again `file.open` on a path already open in a tab switched to it and returned its index, even when another process had changed the file since, so the caller kept looking at the old copy with nothing saying so (#997). The tab's disk stamp from #1330 (`DocState::on_disk`) now tells: - the result says `stale: true` when the file on disk differs from what the tab read or last saved; - `reload: true` reads the file again into the tab, as Revert to saved does (refused while the tab has unsaved changes or is in a live session), and the result says `reloaded: true`. MCP's `open_file` takes `reload` and passes it on. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/file.rs | 20 ++-------- crates/engine/src/cmd/file/open.rs | 39 +++++++++++++++++++- crates/engine/src/save_disk_changes_tests.rs | 34 +++++++++++++++++ crates/mcp/src/tools.rs | 4 ++ crates/mcp/tests/it/agent_tasks.rs | 4 ++ docs/mcp.md | 2 +- 6 files changed, 83 insertions(+), 20 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index fed4bff1..d1830d22 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -33,7 +33,7 @@ pub(crate) use load::{load_fig, read_capped, recovered, variables_param, write}; pub use open::MAX_RECENT; #[cfg(test)] use open::append_pages; -use open::{import, open, open_recent}; +use open::{import, open, open_recent, revert}; pub(crate) use save::save_ends_preview; pub use save::{FileSnapshot, file_bytes, thumbnail}; use save::{color_profile, mark_saved, save, save_as}; @@ -72,7 +72,7 @@ pub fn specs() -> Vec { spec!( "file.open", "Open", - "{path | data (base64) + name (tells the format; opens as a new unsaved document), variables? (REST: the local variables JSON as an object, JSON text or a path; default `.variables.json` or `variables.json` next to the file)} (.figcraft, .json: FigCraft or REST API file JSON, .fig (best effort), .sketch (Sketch 48 or newer, best effort), .svg, .png, .jpg …); REST, .fig and .sketch imports return `warnings`; a file opened anew returns `summary` {name, pages, layers, images} (one that was already open just returns its `index`)", + "{path | data (base64) + name (tells the format; opens as a new unsaved document), variables? (REST: the local variables JSON as an object, JSON text or a path; default `.variables.json` or `variables.json` next to the file), reload? (a file already open: read it again into its tab, which must have no unsaved changes)} (.figcraft, .json: FigCraft or REST API file JSON, .fig (best effort), .sketch (Sketch 48 or newer, best effort), .svg, .png, .jpg …); REST, .fig and .sketch imports return `warnings`; a file opened anew returns `summary` {name, pages, layers, images} (one that was already open returns its `index`, with `stale: true` when the file changed on disk since the tab read or saved it, or `reloaded: true` after `reload`)", false, open ), @@ -188,21 +188,7 @@ pub fn specs() -> Vec { Ok(json!({"index": s.active})) }), spec!("file.revert", "Revert to saved", "{} leave the live session first, if any", false, |s, _| { - let path = s.ds()?.path.clone().ok_or_else(|| EngineError::Disabled("file.revert".into(), "the file was never saved".into()))?; - // Stamped before reading: a change made during the read makes the next save refuse. - let on_disk = crate::library::stat(&path).ok(); - let (loaded, _) = load_path(&path, None)?; - let i = s.active; - let uid = s.ds()?.uid; - if let Some(slot) = s.docs.get_mut(i) { - *slot = DocState::new(loaded.doc, Some(path.clone())) - .with_versions(loaded.versions) - .with_branches(loaded.branches) - .with_images_left_in_file(&path, loaded.images_past_budget); - slot.on_disk = on_disk; - } - // Edits on its branch tabs are reverted too. - crate::branches::close_tabs(s, uid, None); + revert(s)?; ok() }), spec!( diff --git a/crates/engine/src/cmd/file/open.rs b/crates/engine/src/cmd/file/open.rs index 6b724ced..169d71fd 100644 --- a/crates/engine/src/cmd/file/open.rs +++ b/crates/engine/src/cmd/file/open.rs @@ -62,11 +62,26 @@ pub(super) fn open(s: &mut Session, p: &Value) -> Result { return open_data(s, &d, p, variables.as_ref()); } let path = req_string("file.open", p, "path")?; - // Already open: switch to it. + // Already open: switch to it, and say when the file changed on disk since this tab read or + // saved it (`stale`), or read it again (`reload`). if let Some(i) = s.docs.iter().position(|d| d.path.as_deref() == Some(path.as_str())) { s.active = i; remember(s, &path); - return Ok(json!({"index": i})); + let stale = s.ds()?.on_disk.as_ref().is_some_and(|read| crate::library::stat(&path).ok().as_ref() != Some(read)); + if !boolean(p, "reload").unwrap_or(false) { + return Ok(if stale { json!({"index": i, "stale": true}) } else { json!({"index": i}) }); + } + if s.ds()?.is_dirty() { + return Err(EngineError::Disabled( + "file.open".into(), + "the open copy has unsaved changes: save them, or Revert to saved to drop them".into(), + )); + } + if s.live().doc_uid.is_some() { + return Err(EngineError::Disabled("file.open".into(), "leave the live session before reading the file again".into())); + } + revert(s)?; + return Ok(json!({"index": i, "reloaded": true})); } // Stamped before reading: a change made during the read makes the next save refuse. let on_disk = crate::library::stat(&path).ok(); @@ -95,6 +110,26 @@ pub(super) fn open(s: &mut Session, p: &Value) -> Result { Ok(out) } +/// Replace the active tab's document with its file as saved (`file.revert`, `file.open {reload}`). +pub(super) fn revert(s: &mut Session) -> Result<()> { + let path = s.ds()?.path.clone().ok_or_else(|| EngineError::Disabled("file.revert".into(), "the file was never saved".into()))?; + // Stamped before reading: a change made during the read makes the next save refuse. + let on_disk = crate::library::stat(&path).ok(); + let (loaded, _) = load_path(&path, None)?; + let i = s.active; + let uid = s.ds()?.uid; + if let Some(slot) = s.docs.get_mut(i) { + *slot = DocState::new(loaded.doc, Some(path.clone())) + .with_versions(loaded.versions) + .with_branches(loaded.branches) + .with_images_left_in_file(&path, loaded.images_past_budget); + slot.on_disk = on_disk; + } + // Edits on its branch tabs are reverted too. + crate::branches::close_tabs(s, uid, None); + Ok(()) +} + /// [`open`] for a file given as base64 `data` (a file dropped without a path on disk): `name` tells /// the format. It opens as a new document with no path, so the first save asks where to put it. fn open_data(s: &mut Session, data: &str, p: &Value, variables: Option<&Value>) -> Result { diff --git a/crates/engine/src/save_disk_changes_tests.rs b/crates/engine/src/save_disk_changes_tests.rs index 807daa9e..7a0505c9 100644 --- a/crates/engine/src/save_disk_changes_tests.rs +++ b/crates/engine/src/save_disk_changes_tests.rs @@ -74,3 +74,37 @@ fn auto_save_leaves_changes_made_on_disk_alone() { assert_eq!(page_on_disk(&path), "CLI VENCEU"); let _ = std::fs::remove_file(&path); } + +/// The name of the page the active tab shows. +fn page_in_tab(s: &Session) -> String { + let ds = s.ds().unwrap(); + ds.doc.node(ds.page).unwrap().name.clone() +} + +/// Opening a file that is already open says when it changed on disk since the tab read or saved +/// it, and `reload` reads it again into the tab, unless the tab has unsaved changes (#997). +#[test] +fn opening_an_open_file_says_it_is_stale_and_reloads_on_request() { + let path = temp_file("reopen"); + let mut setup = Session::new(); + setup.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + setup.execute("file.save", &json!({"path": path})).unwrap(); + let mut app = Session::new(); + let index = app.execute("file.open", &json!({"path": path})).unwrap()["index"].clone(); + let before = page_in_tab(&app); + assert_eq!(app.execute("file.open", &json!({"path": path})).unwrap(), json!({"index": index}), "unchanged: not stale"); + let mut cli = Session::new(); + cli.execute("file.open", &json!({"path": path})).unwrap(); + cli.execute("page.rename", &json!({"name": "Renamed on disk by the CLI"})).unwrap(); + cli.execute("file.save", &json!({})).unwrap(); + assert_eq!(app.execute("file.open", &json!({"path": path})).unwrap(), json!({"index": index, "stale": true})); + assert_eq!(page_in_tab(&app), before, "without reload the tab keeps what it read"); + assert_eq!(app.execute("file.open", &json!({"path": path, "reload": true})).unwrap(), json!({"index": index, "reloaded": true})); + assert_eq!(page_in_tab(&app), "Renamed on disk by the CLI"); + assert_eq!(app.execute("file.open", &json!({"path": path})).unwrap(), json!({"index": index}), "read again: no longer stale"); + app.execute("shape.rectangle", &json!({"x": 20, "y": 0, "width": 10, "height": 10})).unwrap(); + let e = app.execute("file.open", &json!({"path": path, "reload": true})).unwrap_err().to_string(); + assert!(e.contains("unsaved changes"), "{e}"); + assert!(app.ds().unwrap().is_dirty(), "the unsaved edit is still there"); + let _ = std::fs::remove_file(&path); +} diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 1b23191e..beca8700 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -259,6 +259,7 @@ pub fn tool_definitions() -> Vec { json!({ "path": string("File path"), "variables": {"description": "REST API file JSON only: the local variables JSON (`GET /v1/files/:key/variables/local`) as a path or the JSON object. Default: `.variables.json` or `variables.json` next to the file."}, + "reload": boolean("A file that is already open: read it again from disk into its tab (refused while the tab has unsaved changes). Without it, the result says `stale: true` when the file changed on disk since the tab read or saved it"), }), &["path"], ), @@ -992,6 +993,9 @@ fn open_file(b: &mut dyn Backend, a: &Map) -> Result params["variables"] = v.clone(), Some(_) => return Err("`variables` must be a path or the local variables JSON object".into()), } + if let Some(reload) = a.get("reload").filter(|v| !v.is_null()) { + params["reload"] = reload.clone(); + } let r = exec(b, "file.open", params)?; let doc = b.call("document.inspect", json!({"depth": 1}))?; Ok(ToolResult::json(&json!({"opened": r, "document": doc}))) diff --git a/crates/mcp/tests/it/agent_tasks.rs b/crates/mcp/tests/it/agent_tasks.rs index ae576e12..b338642a 100644 --- a/crates/mcp/tests/it/agent_tasks.rs +++ b/crates/mcp/tests/it/agent_tasks.rs @@ -236,6 +236,10 @@ fn task_export_png_and_svg() { c.ok("save_file", json!({"path": file.to_string_lossy()})); let opened = c.ok("open_file", json!({"path": file.to_string_lossy()})); assert!(opened["document"]["pages"][0]["children"].to_string().contains("Banner")); + // The tab is that file: `reload` reads it again from disk (#997). + let reread = c.ok("open_file", json!({"path": file.to_string_lossy(), "reload": true})); + assert_eq!(reread["opened"]["reloaded"], true, "{reread}"); + assert!(reread["document"]["pages"][0]["children"].to_string().contains("Banner")); std::fs::remove_dir_all(&dir).ok(); } diff --git a/docs/mcp.md b/docs/mcp.md index 05dfd1e6..876efe99 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -317,7 +317,7 @@ Or in a project's `.mcp.json`: | `select {ids}` | Replace the selection (`[]` deselects). | | `export {ids, format, scale \| size, path?, bleed?, cropMarks?, bleedMarks?, cmyk?, contentsOnly?, includeBoundingBox?, svgOutlineText?, svgIdAttribute?, svgSimplifyStroke?, vdCompat?}` | png, jpg, svg, webp, pdf, xml (an Android Vector Drawable: `svgIdAttribute` writes `android:name`, `vdCompat` is `bestEffort` (default: approximated with a warning), `exact` (left out with a warning) or `ignore` (no warnings: overlapping layer opacity dropped, even-odd fills kept as `fillType="evenOdd"`, diamond gradients left out), and the result lists `warnings` for what was changed or left out), or `fig`: the whole file as `.fig`, experimental (see Exporting `.fig` files below); `size` is `2x`, `512w` or `512h`. The image covers what the layers paint (shadows towards their offset, strokes, unclipped children, a mask group's mask; hidden layers take no room; a slice is its area with everything under it, or only the frame or group it sits in while `contentsOnly` is on); `contentsOnly: false` also draws every other layer showing in that area, `includeBoundingBox` exports the layers' own boxes; SVG: `svgOutlineText: false` keeps ``, `svgIdAttribute` writes unique ids from layer names, `svgSimplifyStroke: false` writes every stroke as an outline. The same options go in export settings (`export.add` / `export.update`). PDF: frames one page each (sized in points), anything else together on one page; print options: `bleed` (points or a length like `"3mm"`, `"0.125in"`), crop and bleed marks, CMYK colour (see Exports below), also taken by `file.export`, `file.exportLayers`, `file.exportFramesPdf` and stored per export setting by `export.add` / `export.update`; `run_command` `file.exportFramesPdf {page?, path?}` writes every top-level frame of a page. Without `path`, images come back as MCP image content, SVG as text, XML as text followed by `{format, bytes, warnings}`. Layers' own export settings: `run_command` with `export.add` / `export.update {index, size?, suffix?, format?}` / `export.remove` and `file.exportLayers {ids?, dir?, assets?}` (every layer once per setting, named `.`, a slash in a layer name making a folder; `assets: true` exports the Inspect tab's assets instead: icons as SVG, images as PNG). | | `screenshot {ids?, scale?, maxSize?, window?}` | PNG of the page (with background) or of layers, scaled to fit `maxSize` (default 1600). `window: true` captures the app window in front when attached, a window id (see Windows below) that window. | -| `open_file {path, variables?}` / `save_file {path?}` | Open `.figcraft`, `.json` (FigCraft JSON or REST API file JSON with its variables, see below), `.fig` (best effort, see below), `.sketch` (Sketch 48 and newer, best effort), `.svg` (see Importing SVG below) or an image; save as `.figcraft`. `open_file` returns `opened` (for a file opened anew, a `summary` of `name`, `pages`, `layers` and `images`, and `warnings` or `repairs`; a file that was already open just gives its `index`) and the document's `document` outline. | +| `open_file {path, variables?, reload?}` / `save_file {path?}` | Open `.figcraft`, `.json` (FigCraft JSON or REST API file JSON with its variables, see below), `.fig` (best effort, see below), `.sketch` (Sketch 48 and newer, best effort), `.svg` (see Importing SVG below) or an image; save as `.figcraft`. `open_file` returns `opened` (for a file opened anew, a `summary` of `name`, `pages`, `layers` and `images`, and `warnings` or `repairs`; a file that was already open gives its `index`, with `stale: true` when the file changed on disk since that tab read or saved it; `reload: true` reads it again into the tab, refused while the tab has unsaved changes) and the document's `document` outline. | | `undo` / `redo` | Step through history. | | `pointer {kind, x, y, to?, steps?, tool?, shift, alt, cmd, pressure?, pressureTo?}` | Mouse gestures in page coordinates with the current tool: `click`, `double`, `drag`, `down`, `move`, `up`, `hover`. A dragged layer drops into the frame or auto layout slot under the pointer, except one inside a group, a boolean group or an instance (outside its slots), which only moves. `pressure` (0–1) is pen pressure; a drag goes from `pressure` to `pressureTo` (see Brushes and pen pressure). | | `key {key, shift, alt, cmd}` | A shortcut or key as if typed on the canvas (every default shortcut, tool keys, Escape, Delete, arrows, number keys for opacity, arrows that pan the view when nothing is selected; while editing text, the text shortcuts and then text editing keys). Keys are names like `A`, `0`, `Enter`, `ArrowLeft`, `=`, `/`, or egui's (`Plus`, `Questionmark`); `cmd` is Cmd on macOS and Ctrl elsewhere. | From 3567bef862fa9d784a4768a416524711eb867eb7 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 09:35:23 +0100 Subject: [PATCH 08/12] #1460: commands and tools that pass their params on list what they pass file.placeImage runs image.place's code and file.exportSvg file.export's, so their docs now list the names those read (width, height, target, mode; scale or size and the export options). MCP's create_node passes points, closed and select to node.create, and pointer and key read ctrl, so their schemas list them too. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/file.rs | 26 +++++++++++++++++------ crates/engine/src/unknown_params_tests.rs | 3 +++ crates/mcp/src/tools.rs | 6 +++++- crates/mcp/tests/it/protocol.rs | 4 ++++ 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 2e63b1c9..c37d49e4 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -118,13 +118,19 @@ pub fn specs() -> Vec { false, export_layers ), - spec!("file.exportSvg", "Export selection as SVG", "{ids?, path?}", false, |s, p| { - let mut q = p.clone(); - if let Some(o) = q.as_object_mut() { - o.insert("format".into(), json!("svg")); + spec!( + "file.exportSvg", + "Export selection as SVG", + "{ids?, path?, scale? | size?, contentsOnly?, includeBoundingBox?, svgOutlineText?, svgIdAttribute?, svgSimplifyStroke? (as file.export)}", + false, + |s, p| { + let mut q = p.clone(); + if let Some(o) = q.as_object_mut() { + o.insert("format".into(), json!("svg")); + } + export(s, &q) } - export(s, &q) - }), + ), spec!( "file.exportFramesPdf", "Export frames to PDF", @@ -139,7 +145,13 @@ pub fn specs() -> Vec { true, import ), - spec!("file.placeImage", "Place image", "{path | data, x?, y?}", true, super::create::place_image), + spec!( + "file.placeImage", + "Place image", + "{path | data, x?, y?, width?, height?, parent?, name?, target?, mode? (as image.place)}", + true, + super::create::place_image + ), spec!("file.close", "Close tab", "{index?, force? (discard unsaved changes)}", false, close), spec!("file.moveTab", "Move tab", "{from, to} (indices among open documents)", false, |s, p| { let from = req_index("file.moveTab", p, "from")?; diff --git a/crates/engine/src/unknown_params_tests.rs b/crates/engine/src/unknown_params_tests.rs index 02290ba3..f0773f25 100644 --- a/crates/engine/src/unknown_params_tests.rs +++ b/crates/engine/src/unknown_params_tests.rs @@ -51,6 +51,9 @@ fn documented_alternatives_and_empty_params_are_taken() { assert_eq!(param_names("proto.interaction.add").map(|n| n.contains(&"delayMs") && n.contains(&"dest")), Some(true)); assert_eq!(param_names("collab.join"), Some(&["code", "link"][..])); assert_eq!(param_names("proto.interaction.set").map(|n| n.contains(&"trigger") && n.contains(&"interaction")), Some(true)); + // Commands that run another command's code take its names too. + assert_eq!(param_names("file.placeImage").map(|n| ["width", "height", "target", "mode"].iter().all(|k| n.contains(k))), Some(true)); + assert_eq!(param_names("file.exportSvg").map(|n| ["scale", "size", "contentsOnly", "svgOutlineText"].iter().all(|k| n.contains(k))), Some(true)); // Alternative groups: `{x, y} | {points: …}`. assert_eq!(param_names("vector.edit.paint"), Some(&["x", "y", "points"][..])); assert_eq!(param_names("vector.edit.cut"), Some(&["segment", "t", "vertex", "from", "to"][..])); diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 46cc1a83..b03a0af8 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -154,6 +154,9 @@ pub fn tool_definitions() -> Vec { "name": string("Layer name"), "text": string("Characters, for type text"), "path": string("SVG path data, for type vector (e.g. \"M0 0 L100 0 L50 80 Z\")"), + "points": {"type": "array", "items": {"type": "array", "items": {"type": "number"}}, "description": "For type vector, instead of path: the points [[x, y], ...]"}, + "closed": boolean("For type vector: close the path"), + "select": boolean("Select the new layer (default true)"), "props": {"type": "object", "description": "Any set_properties keys applied at creation, e.g. {\"fill\":\"#3366FF\",\"cornerRadius\":12,\"fontSize\":24,\"layout\":\"vertical\",\"gap\":8,\"padding\":16}"}, }), &["type"], @@ -288,6 +291,7 @@ pub fn tool_definitions() -> Vec { "shift": boolean("Shift held"), "alt": boolean("Alt/Option held"), "cmd": boolean("Cmd (macOS) / Ctrl held"), + "ctrl": boolean("Ctrl held (the Control key on macOS)"), "pressure": num("Pen pressure 0-1 (the Pencil records it as the stroke's width profile; a drag starts with it)"), "pressureTo": num("Pen pressure 0-1 at a drag's end (interpolated along it)"), }), @@ -300,7 +304,7 @@ pub fn tool_definitions() -> Vec { "Press key", "Press a key or shortcut as if typed on the canvas: a menu shortcut (e.g. {\"key\":\"G\",\"cmd\":true} groups, {\"key\":\"A\",\"alt\":true,\"shift\":false} …), a tool key (R, O, T, F, V…), Escape, Delete, Enter, arrows (nudge; shift = big nudge). While editing text, keys go to the text editor (Backspace, Delete, Left, Right, Home, End, Enter, Escape, SelectAll).", obj( - json!({"key": string("Key name: a letter/digit, Escape, Delete, Backspace, Enter, ArrowLeft…"), "shift": boolean("Shift"), "alt": boolean("Alt/Option"), "cmd": boolean("Cmd/Ctrl")}), + json!({"key": string("Key name: a letter/digit, Escape, Delete, Backspace, Enter, ArrowLeft…"), "shift": boolean("Shift"), "alt": boolean("Alt/Option"), "cmd": boolean("Cmd/Ctrl"), "ctrl": boolean("Ctrl (the Control key on macOS)")}), &["key"], ), false, diff --git a/crates/mcp/tests/it/protocol.rs b/crates/mcp/tests/it/protocol.rs index 5cc45209..85401a82 100644 --- a/crates/mcp/tests/it/protocol.rs +++ b/crates/mcp/tests/it/protocol.rs @@ -91,6 +91,10 @@ fn tools_call_happy_and_error_paths() { assert_eq!(r["error"]["code"], -32602); // The document is unharmed by all of that. assert_eq!(c.page_children().len(), 1); + // Arguments the tools pass on are in their schemas, so they still go through. + c.ok("create_node", json!({"type": "vector", "points": [[0, 0], [10, 0], [5, 8]], "closed": true, "select": false})); + c.ok("key", json!({"key": "Escape", "ctrl": false})); + c.ok("pointer", json!({"kind": "hover", "x": 1, "y": 1, "ctrl": false})); } #[test] From 8fadd5d8e5c7387619b7d4980405ac3025691523 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 09:46:27 +0100 Subject: [PATCH 09/12] #1460: file.exportSvg's doc says which of file.export's options it takes "(as file.export)" read as all of them, but the PDF and XML options are refused there, so it now says they are file.export's SVG options. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/file.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index c37d49e4..816890ab 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -121,7 +121,7 @@ pub fn specs() -> Vec { spec!( "file.exportSvg", "Export selection as SVG", - "{ids?, path?, scale? | size?, contentsOnly?, includeBoundingBox?, svgOutlineText?, svgIdAttribute?, svgSimplifyStroke? (as file.export)}", + "{ids?, path?, scale? | size?, contentsOnly?, includeBoundingBox?, svgOutlineText?, svgIdAttribute?, svgSimplifyStroke? (file.export's SVG options)}", false, |s, p| { let mut q = p.clone(); From fd81433a23d7e7546244939fbf2f43fc4f43c264 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 11:09:09 +0700 Subject: [PATCH 10/12] #1460: text.style lists the text-decoration and variation keys the Typography panel sends, paragraph spacing on selected characters goes to node.set, MCP builds its tool catalogue once, and a test for the params panels build --- crates/engine/src/cmd/text.rs | 2 +- crates/mcp/src/tools.rs | 5 +- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/export_panel.rs | 4 +- crates/ui-egui/src/panels/typography.rs | 9 +-- crates/ui-egui/src/ui_params_tests.rs | 77 +++++++++++++++++++++++ 6 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 crates/ui-egui/src/ui_params_tests.rs diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 35edfed9..b6b61841 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -396,7 +396,7 @@ pub fn specs() -> Vec { spec!( "text.style", "Style text range", - "{id?, start?, end?, fontFamily?, fontStyle?, fontWeight?, fontSize?, lineHeight?, letterSpacing?, textDecoration?, textCase?, hyperlink?, textFills?}", + "{id?, start?, end?, fontFamily?, fontStyle?, fontWeight?, fontVariations? | variationSettings?, fontSize?, lineHeight?, letterSpacing?, textDecoration?, textDecorationStyle?, textDecorationThickness?, textDecorationOffset?, textDecorationSkipInk?, textDecorationColor?, textCase?, hyperlink?, textFills?}", true, |s, p| { let ids = req_targets("text.style", s, p)?; diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index b03a0af8..13f02972 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -450,8 +450,9 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult { Ok(a) => a, Err(e) => return ToolResult::error(e), }; - let tools = tool_definitions(); - if let Some(schema) = tools.iter().find(|t| t.get("name").and_then(Value::as_str) == Some(name)).and_then(|t| t.get("inputSchema")) + // Built once: the catalogue is the same on every call. + static TOOLS: std::sync::LazyLock> = std::sync::LazyLock::new(tool_definitions); + if let Some(schema) = TOOLS.iter().find(|t| t.get("name").and_then(Value::as_str) == Some(name)).and_then(|t| t.get("inputSchema")) && let Err(e) = refuse_unknown_args(schema, &Value::Object(a.clone()), "") { return ToolResult::error(e); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ef55faf8..ec4a61ab 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -195,6 +195,8 @@ mod text_enter_tests; #[cfg(test)] mod titlebar_tests; #[cfg(test)] +mod ui_params_tests; +#[cfg(test)] mod ui_scale_tests; #[cfg(test)] mod variables_export_ui_tests; diff --git a/crates/ui-egui/src/panels/export_panel.rs b/crates/ui-egui/src/panels/export_panel.rs index cec8ebc0..06b34f2c 100644 --- a/crates/ui-egui/src/panels/export_panel.rs +++ b/crates/ui-egui/src/panels/export_panel.rs @@ -247,12 +247,12 @@ pub(crate) fn print_options(ui: &mut Ui, id: Id, p: &PrintSettings) -> Option Value { +pub(crate) fn print_params(p: &PrintSettings) -> Value { json!({"bleed": p.bleed(), "cropMarks": p.crop_marks, "bleedMarks": p.bleed_marks, "cmyk": p.cmyk}) } /// `file.export` params for `setting`: its size, format and options (PDF: its print options). -fn setting_params(setting: &ExportSetting) -> Value { +pub(crate) fn setting_params(setting: &ExportSetting) -> Value { let mut params = serde_json::to_value(setting).unwrap_or_else(|_| json!({})); if let Value::Object(m) = &mut params { m.remove("print"); diff --git a/crates/ui-egui/src/panels/typography.rs b/crates/ui-egui/src/panels/typography.rs index a95af934..299bb57b 100644 --- a/crates/ui-egui/src/panels/typography.rs +++ b/crates/ui-egui/src/panels/typography.rs @@ -158,10 +158,11 @@ fn path_row(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId], ids: } /// Set character `props` on the selected chars of the text being edited, else on whole layers -/// (through `node.set`, which records overrides inside instances). -fn commit_text(app: &mut FigApp, ids: &[NodeId], range: Option<(NodeId, usize, usize)>, props: Value) { +/// (through `node.set`, which records overrides inside instances). Paragraph spacing is always +/// the whole layer's: `text.style` doesn't take it. +pub(crate) fn commit_text(app: &mut FigApp, ids: &[NodeId], range: Option<(NodeId, usize, usize)>, props: Value) { match range { - Some((_, a, b)) if a != b => { + Some((_, a, b)) if a != b && props.get("paragraphSpacing").is_none() => { app.run("text.style", text_style_params(ids, range, props)); } _ => app.set_props(ids, props), @@ -170,7 +171,7 @@ fn commit_text(app: &mut FigApp, ids: &[NodeId], range: Option<(NodeId, usize, u /// `text.style` params for `props` on the selected chars of the text being edited, else on the /// whole of every layer in `ids` (previews; commits go through [`commit_text`]). -fn text_style_params(ids: &[NodeId], range: Option<(NodeId, usize, usize)>, mut props: Value) -> Value { +pub(crate) fn text_style_params(ids: &[NodeId], range: Option<(NodeId, usize, usize)>, mut props: Value) -> Value { match range { Some((id, a, b)) if a != b => { props["id"] = json!(id); diff --git a/crates/ui-egui/src/ui_params_tests.rs b/crates/ui-egui/src/ui_params_tests.rs new file mode 100644 index 00000000..6ee71dfc --- /dev/null +++ b/crates/ui-egui/src/ui_params_tests.rs @@ -0,0 +1,77 @@ +//! Params the panels build at run time name only what their commands take: a name a command +//! doesn't list is refused (#1413), which would break the control that sent it. + +use figcraft_doc::{ExportFormat, ExportSetting, NodeId, PrintSettings, VdCompat}; +use figcraft_engine::Session; +use serde_json::{Value, json}; + +use crate::{FigApp, NoServices}; + +fn app_with(cmd: &str, params: Value) -> (FigApp, NodeId) { + let mut s = Session::new(); + let id = NodeId(s.execute(cmd, ¶ms).unwrap()["id"].as_u64().unwrap()); + (FigApp::new(s, Box::new(NoServices)), id) +} + +/// The Export section: a setting with every option off its default exports in every format, and +/// its edits (the setting's own keys, the PDF print options) apply. +#[test] +fn export_settings_with_every_option_are_taken() { + let (mut app, id) = app_with("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10})); + app.session.execute("export.add", &json!({"ids": [id]})).unwrap(); + let print = PrintSettings { bleed: 3.0, crop_marks: true, bleed_marks: true, cmyk: true }; + for format in ExportFormat::ALL { + let setting = ExportSetting { + format, + print, + contents_only: false, + include_bounding_box: true, + svg_outline_text: false, + svg_id_attribute: true, + svg_simplify_stroke: false, + vd_compat: VdCompat::Exact, + ..ExportSetting::default() + }; + let mut p = crate::export_panel::setting_params(&setting); + p["ids"] = json!([id]); + app.session.execute("file.export", &p).unwrap_or_else(|e| panic!("file.export {p}: {e}")); + p["index"] = json!(0); + app.session.execute("export.update", &p).unwrap_or_else(|e| panic!("export.update {p}: {e}")); + } + let mut p = crate::export_panel::print_params(&print); + p["ids"] = json!([id]); + p["index"] = json!(0); + app.session.execute("export.update", &p).unwrap_or_else(|e| panic!("export.update {p}: {e}")); +} + +/// The Typography section on selected characters: every character property it sets goes through +/// `text.style`, and paragraph spacing (the whole layer's) through `node.set`. +#[test] +fn typography_edits_on_selected_characters_are_taken() { + let (mut app, id) = app_with("text.create", json!({"text": "Typography", "x": 0, "y": 0})); + let range = Some((id, 0, 4)); + for (key, v) in [ + ("fontFamily", json!("Inter")), + ("fontStyle", json!("Bold")), + ("fontSize", json!(20)), + ("lineHeight", json!(24)), + ("letterSpacing", json!(1)), + ("textDecoration", json!("underline")), + ("textDecorationStyle", json!("wavy")), + ("textDecorationThickness", json!(2)), + ("textDecorationOffset", json!(3)), + ("textDecorationSkipInk", json!(false)), + ("textDecorationColor", json!("#FF0000")), + ("textCase", json!("upper")), + ] { + crate::typography::commit_text(&mut app, &[id], range, json!({ key: v })); + assert_eq!(app.ui.last_error, None, "{key}"); + } + // A named instance of a variable font: its style, and the custom axis values cleared. + crate::typography::commit_text(&mut app, &[id], range, json!({"fontStyle": "Regular", "fontVariations": null})); + assert_eq!(app.ui.last_error, None, "fontVariations"); + crate::typography::commit_text(&mut app, &[id], range, json!({"paragraphSpacing": 12})); + assert_eq!(app.ui.last_error, None, "paragraphSpacing"); + let spacing = app.session.doc().unwrap().node(id).and_then(|n| n.kind.text()).map(|t| t.paragraph_spacing); + assert_eq!(spacing, Some(12.0)); +} From 38f63e2da08dac531c5bdcabcc2fa9b51f8940ee Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 11:41:58 +0700 Subject: [PATCH 11/12] #1408: label node.find in the catalogs that label node.get --- crates/ui-egui/locales/ar.toml | 1 + crates/ui-egui/locales/cs.toml | 1 + crates/ui-egui/locales/de.toml | 1 + crates/ui-egui/locales/es.toml | 1 + crates/ui-egui/locales/fi.toml | 1 + crates/ui-egui/locales/fr.toml | 1 + crates/ui-egui/locales/id.toml | 1 + crates/ui-egui/locales/it.toml | 1 + crates/ui-egui/locales/ja.toml | 1 + crates/ui-egui/locales/ka.toml | 1 + crates/ui-egui/locales/ko.toml | 1 + crates/ui-egui/locales/nl.toml | 1 + crates/ui-egui/locales/pl.toml | 1 + crates/ui-egui/locales/pt-BR.toml | 1 + crates/ui-egui/locales/ru.toml | 1 + crates/ui-egui/locales/sv.toml | 1 + crates/ui-egui/locales/th.toml | 1 + crates/ui-egui/locales/tr.toml | 1 + crates/ui-egui/locales/vi.toml | 1 + crates/ui-egui/locales/zh-Hans.toml | 1 + 20 files changed, 20 insertions(+) diff --git a/crates/ui-egui/locales/ar.toml b/crates/ui-egui/locales/ar.toml index f252e114..12ed7851 100644 --- a/crates/ui-egui/locales/ar.toml +++ b/crates/ui-egui/locales/ar.toml @@ -2130,6 +2130,7 @@ status = "draft — needs native review" "measure.distances" = "قياس المسافات" "measure.padding" = "قياس حشو التخطيط التلقائي" "node.create" = "إنشاء طبقة" +"node.find" = "بحث عن الطبقات" "node.get" = "جلب طبقة" "node.reorder" = "إعادة ترتيب الطبقة" "node.reparent" = "نقل الطبقة إلى داخل" diff --git a/crates/ui-egui/locales/cs.toml b/crates/ui-egui/locales/cs.toml index f8f86df9..ba6d2721 100644 --- a/crates/ui-egui/locales/cs.toml +++ b/crates/ui-egui/locales/cs.toml @@ -2026,6 +2026,7 @@ status = "draft — needs native review" "measure.distances" = "Měřit vzdálenosti" "measure.padding" = "Měřit vnitřní odsazení automatického rozvržení" "node.create" = "Vytvořit vrstvu" +"node.find" = "Najít vrstvy" "node.get" = "Získat vrstvu" "node.reorder" = "Změnit pořadí vrstvy" "node.reparent" = "Přesunout vrstvu do" diff --git a/crates/ui-egui/locales/de.toml b/crates/ui-egui/locales/de.toml index 127900c9..c73bf5aa 100644 --- a/crates/ui-egui/locales/de.toml +++ b/crates/ui-egui/locales/de.toml @@ -1980,6 +1980,7 @@ status = "draft — needs native review" "measure.distances" = "Abstände messen" "measure.padding" = "Innenabstand des Auto-Layouts messen" "node.create" = "Ebene erstellen" +"node.find" = "Ebenen suchen" "node.get" = "Ebene abrufen" "node.reorder" = "Ebene neu anordnen" "node.reparent" = "Ebene verschieben in" diff --git a/crates/ui-egui/locales/es.toml b/crates/ui-egui/locales/es.toml index dd6b4ed1..e879efc7 100644 --- a/crates/ui-egui/locales/es.toml +++ b/crates/ui-egui/locales/es.toml @@ -2118,6 +2118,7 @@ status = "draft — needs native review" "measure.distances" = "Medir distancias" "measure.padding" = "Medir margen interno del diseño automático" "node.create" = "Crear capa" +"node.find" = "Buscar capas" "node.get" = "Obtener capa" "node.reorder" = "Reordenar capa" "node.reparent" = "Mover capa dentro de" diff --git a/crates/ui-egui/locales/fi.toml b/crates/ui-egui/locales/fi.toml index f1a3bbc0..1ced2d53 100644 --- a/crates/ui-egui/locales/fi.toml +++ b/crates/ui-egui/locales/fi.toml @@ -2069,6 +2069,7 @@ status = "draft — needs native review" "measure.distances" = "Mittaa etäisyyksiä" "measure.padding" = "Mittaa automaattisen asettelun sisämarginaali" "node.create" = "Luo taso" +"node.find" = "Etsi tasoja" "node.get" = "Hae taso" "node.reorder" = "Muuta tason järjestystä" "node.reparent" = "Siirrä taso kohteeseen" diff --git a/crates/ui-egui/locales/fr.toml b/crates/ui-egui/locales/fr.toml index 7c80a3e3..34f17e47 100644 --- a/crates/ui-egui/locales/fr.toml +++ b/crates/ui-egui/locales/fr.toml @@ -2070,6 +2070,7 @@ status = "draft — needs native review" "measure.distances" = "Mesurer les distances" "measure.padding" = "Mesurer la marge intérieure de la mise en page auto" "node.create" = "Créer un calque" +"node.find" = "Rechercher des calques" "node.get" = "Obtenir le calque" "node.reorder" = "Réordonner le calque" "node.reparent" = "Déplacer le calque dans" diff --git a/crates/ui-egui/locales/id.toml b/crates/ui-egui/locales/id.toml index b1755027..c2aabe15 100644 --- a/crates/ui-egui/locales/id.toml +++ b/crates/ui-egui/locales/id.toml @@ -2030,6 +2030,7 @@ status = "draft — needs native review" "measure.distances" = "Ukur jarak" "measure.padding" = "Ukur padding auto layout" "node.create" = "Buat layer" +"node.find" = "Cari layer" "node.get" = "Ambil layer" "node.reorder" = "Ubah urutan layer" "node.reparent" = "Pindahkan layer ke dalam" diff --git a/crates/ui-egui/locales/it.toml b/crates/ui-egui/locales/it.toml index d31ab7b6..d2903478 100644 --- a/crates/ui-egui/locales/it.toml +++ b/crates/ui-egui/locales/it.toml @@ -1983,6 +1983,7 @@ status = "draft — needs native review" "measure.distances" = "Misura distanze" "measure.padding" = "Misura margini interni dell'auto layout" "node.create" = "Crea livello" +"node.find" = "Trova livelli" "node.get" = "Ottieni livello" "node.reorder" = "Riordina livello" "node.reparent" = "Sposta livello dentro" diff --git a/crates/ui-egui/locales/ja.toml b/crates/ui-egui/locales/ja.toml index ec117721..ab5ea247 100644 --- a/crates/ui-egui/locales/ja.toml +++ b/crates/ui-egui/locales/ja.toml @@ -2050,6 +2050,7 @@ status = "draft — needs native review" "measure.distances" = "距離を測定" "measure.padding" = "オートレイアウトのパディングを測定" "node.create" = "レイヤーを作成" +"node.find" = "レイヤーを検索" "node.get" = "レイヤーを取得" "node.reorder" = "レイヤーの順序を変更" "node.reparent" = "レイヤーを中に移動" diff --git a/crates/ui-egui/locales/ka.toml b/crates/ui-egui/locales/ka.toml index b5d19efb..a69c20e1 100644 --- a/crates/ui-egui/locales/ka.toml +++ b/crates/ui-egui/locales/ka.toml @@ -2122,6 +2122,7 @@ status = "draft — reviewed by a native speaker, awaiting a maintainer's approv "measure.distances" = "მანძილების გაზომვა" "measure.padding" = "ავტოგანლაგების შიდა დაშორების გაზომვა" "node.create" = "შრის შექმნა" +"node.find" = "შრეების ძებნა" "node.get" = "შრის მიღება" "node.reorder" = "შრის გადალაგება" "node.reparent" = "შრის შიგნით გადატანა" diff --git a/crates/ui-egui/locales/ko.toml b/crates/ui-egui/locales/ko.toml index 397d9507..0dfe1190 100644 --- a/crates/ui-egui/locales/ko.toml +++ b/crates/ui-egui/locales/ko.toml @@ -2086,6 +2086,7 @@ status = "draft — needs native review" "measure.distances" = "거리 측정" "measure.padding" = "자동 레이아웃 패딩 측정" "node.create" = "레이어 만들기" +"node.find" = "레이어 찾기" "node.get" = "레이어 가져오기" "node.reorder" = "레이어 순서 변경" "node.reparent" = "레이어를 안으로 이동" diff --git a/crates/ui-egui/locales/nl.toml b/crates/ui-egui/locales/nl.toml index d9905f2c..910424c2 100644 --- a/crates/ui-egui/locales/nl.toml +++ b/crates/ui-egui/locales/nl.toml @@ -2068,6 +2068,7 @@ status = "draft — needs native review" "measure.distances" = "Afstanden meten" "measure.padding" = "Padding van auto layout meten" "node.create" = "Laag maken" +"node.find" = "Lagen zoeken" "node.get" = "Laag ophalen" "node.reorder" = "Laag herordenen" "node.reparent" = "Laag verplaatsen naar" diff --git a/crates/ui-egui/locales/pl.toml b/crates/ui-egui/locales/pl.toml index c935fca1..faca0993 100644 --- a/crates/ui-egui/locales/pl.toml +++ b/crates/ui-egui/locales/pl.toml @@ -2081,6 +2081,7 @@ name = "Polski" "measure.distances" = "Zmierz odległości" "measure.padding" = "Zmierz marginesy automatycznego układu" "node.create" = "Utwórz warstwę" +"node.find" = "Znajdź warstwy" "node.get" = "Pobierz warstwę" "node.reorder" = "Zmień kolejność warstwy" "node.reparent" = "Przenieś warstwę do" diff --git a/crates/ui-egui/locales/pt-BR.toml b/crates/ui-egui/locales/pt-BR.toml index 16d6eab7..a81f464c 100644 --- a/crates/ui-egui/locales/pt-BR.toml +++ b/crates/ui-egui/locales/pt-BR.toml @@ -2083,6 +2083,7 @@ status = "draft — needs native review" "measure.distances" = "Medir distâncias" "measure.padding" = "Medir espaçamento interno do layout automático" "node.create" = "Criar camada" +"node.find" = "Localizar camadas" "node.get" = "Obter camada" "node.reorder" = "Reordenar camada" "node.reparent" = "Mover camada para dentro de" diff --git a/crates/ui-egui/locales/ru.toml b/crates/ui-egui/locales/ru.toml index d25f7d14..a9647e66 100644 --- a/crates/ui-egui/locales/ru.toml +++ b/crates/ui-egui/locales/ru.toml @@ -2081,6 +2081,7 @@ name = "Русский" "measure.distances" = "Измерить расстояния" "measure.padding" = "Измерить внутренние отступы автолейаута" "node.create" = "Создать слой" +"node.find" = "Найти слои" "node.get" = "Получить слой" "node.reorder" = "Изменить порядок слоя" "node.reparent" = "Переместить слой в" diff --git a/crates/ui-egui/locales/sv.toml b/crates/ui-egui/locales/sv.toml index 8718b060..f7ce9853 100644 --- a/crates/ui-egui/locales/sv.toml +++ b/crates/ui-egui/locales/sv.toml @@ -2126,6 +2126,7 @@ name = "Svenska" "measure.distances" = "Mät avstånd" "measure.padding" = "Mät padding" "node.create" = "Skapa nod" +"node.find" = "Sök lager" "node.get" = "Hämta nod" "node.reorder" = "Ändra ordning" "node.reparent" = "Byt överordnad nod" diff --git a/crates/ui-egui/locales/th.toml b/crates/ui-egui/locales/th.toml index 560975a1..a1253073 100644 --- a/crates/ui-egui/locales/th.toml +++ b/crates/ui-egui/locales/th.toml @@ -2046,6 +2046,7 @@ status = "draft — needs native review" "measure.distances" = "วัดระยะห่าง" "measure.padding" = "วัดระยะขอบในของออโต้เลย์เอาต์" "node.create" = "สร้างเลเยอร์" +"node.find" = "ค้นหาเลเยอร์" "node.get" = "ดึงเลเยอร์" "node.reorder" = "เรียงลำดับเลเยอร์ใหม่" "node.reparent" = "ย้ายเลเยอร์เข้าไปใน" diff --git a/crates/ui-egui/locales/tr.toml b/crates/ui-egui/locales/tr.toml index a36e21d2..460757a4 100644 --- a/crates/ui-egui/locales/tr.toml +++ b/crates/ui-egui/locales/tr.toml @@ -2030,6 +2030,7 @@ name = "Türkçe" "measure.distances" = "Mesafeleri ölç" "measure.padding" = "Otomatik yerleşim iç boşluğunu ölç" "node.create" = "Katman oluştur" +"node.find" = "Katman bul" "node.get" = "Katmanı getir" "node.reorder" = "Katmanı yeniden sırala" "node.reparent" = "Katmanı içine taşı" diff --git a/crates/ui-egui/locales/vi.toml b/crates/ui-egui/locales/vi.toml index e130afbf..aacd5401 100644 --- a/crates/ui-egui/locales/vi.toml +++ b/crates/ui-egui/locales/vi.toml @@ -1984,6 +1984,7 @@ status = "draft — needs native review" "measure.distances" = "Đo khoảng cách" "measure.padding" = "Đo khoảng đệm bố cục tự động" "node.create" = "Tạo lớp" +"node.find" = "Tìm lớp" "node.get" = "Lấy lớp" "node.reorder" = "Sắp xếp lại lớp" "node.reparent" = "Di chuyển lớp vào trong" diff --git a/crates/ui-egui/locales/zh-Hans.toml b/crates/ui-egui/locales/zh-Hans.toml index 1081afad..534b2b0d 100644 --- a/crates/ui-egui/locales/zh-Hans.toml +++ b/crates/ui-egui/locales/zh-Hans.toml @@ -1983,6 +1983,7 @@ status = "draft — needs native review" "measure.distances" = "测量距离" "measure.padding" = "测量自动布局内边距" "node.create" = "创建图层" +"node.find" = "查找图层" "node.get" = "获取图层" "node.reorder" = "调整图层顺序" "node.reparent" = "将图层移入" From a48f43cd349996cf326cc30e5734cddbb8db08f0 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 11:49:37 +0700 Subject: [PATCH 12/12] #1408: import opt_id for node.find now that edit.rs names its imports --- crates/engine/src/cmd/edit.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 310c4750..de74264c 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -8,8 +8,8 @@ use serde_json::{Map, Value, json}; use super::{CommandSpec, ok}; use crate::params::{ - blend, boolean, color, get, id, id_of, num, num_or, opt_index, paint_of, parse, req_id, req_index, req_num, req_string, req_targets, string, - targets, + blend, boolean, color, get, id, id_of, num, num_or, opt_id, opt_index, paint_of, parse, req_id, req_index, req_num, req_string, req_targets, + string, targets, }; use crate::{BatchStep, EngineError, Result, Session, bad, spec};