From 5c2e182388212a4fab1e070ee17a3c285b307dfe Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 12:47:49 -0800 Subject: [PATCH 1/2] MCP: a malformed object id is refused as such, not reported as missing Commands that need an object id read it with `id_param`, which returns None for a value that is not a non-negative integer, so `{"id": "5"}` or `{"id": -1}` was refused with "missing id". They now read it with `object_param`, which refuses such a value with "`id` must be a whole number", naming the key. This covers `document.node`, the `path.*` anchor and handle commands, the `text.*` range and path commands, the mesh, envelope and slice point commands, `layer.target`, `layer.move`, `node.move`, `select.magicWand`, and the `source`, `target`, `group` and `path` keys of `appearance.transfer`, `appearance.copyFrom`, the `livePaint.*` commands and `text.createInPath`. The optional `group` of `livePaint.fill` and `parent` of `node.move` are refused the same way when given. Before, a malformed `parent` moved the object to the top level and a malformed `group` fell back to the selection. --- crates/engine/src/cmd/appearance.rs | 10 +-- crates/engine/src/cmd/buildcmds.rs | 4 +- crates/engine/src/cmd/buildcmds/live_paint.rs | 13 +-- crates/engine/src/cmd/distortcmds.rs | 10 +-- crates/engine/src/cmd/draw2.rs | 4 +- crates/engine/src/cmd/draw2/anchors.rs | 10 +-- crates/engine/src/cmd/draw2/cut.rs | 4 +- crates/engine/src/cmd/draw2/freehand.rs | 9 +- crates/engine/src/cmd/edit.rs | 6 +- crates/engine/src/cmd/layer.rs | 19 +++-- crates/engine/src/cmd/live.rs | 2 +- crates/engine/src/cmd/live/envelope.rs | 4 +- crates/engine/src/cmd/live/mesh.rs | 6 +- crates/engine/src/cmd/path.rs | 16 ++-- crates/engine/src/cmd/slices.rs | 4 +- crates/engine/src/cmd/textedit.rs | 20 ++--- crates/engine/src/cmd/typecmd.rs | 10 +-- crates/engine/src/cmd/wand.rs | 4 +- crates/engine/src/tests/params.rs | 84 +++++++++++++++++++ docs/mcp.md | 8 +- 20 files changed, 172 insertions(+), 75 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 2dfeb497d0..7f6f8cca12 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -32,8 +32,8 @@ use super::opacitymask::percent; use super::paint::paint_from; use super::{AppearanceAttrs, EyedropperAttrs, FillAttrs, StrokeAttrs}; use super::{ - CommandSpec, NodeId, Result, Session, bad, check_numbers, check_switches, cmd, has_doc, has_selection, id_param, ids_param, index, leaf_targets, - ok, paint_targets, str_param, switch_or, targets, + CommandSpec, NodeId, Result, Session, bad, check_numbers, check_switches, cmd, has_doc, has_selection, ids_param, index, leaf_targets, + object_param, ok, paint_targets, str_param, switch_or, targets, }; use crate::EngineError; @@ -726,8 +726,8 @@ fn clear(n: &mut Node) { fn transfer(s: &mut Session, p: &Value) -> Result { const C: &str = "appearance.transfer"; - let source = id_param(p, "source").ok_or_else(|| bad(C, "missing `source` id"))?; - let target = id_param(p, "target").ok_or_else(|| bad(C, "missing `target` id"))?; + let source = object_param(p, "source", C)?.ok_or_else(|| bad(C, "missing `source` id"))?; + let target = object_param(p, "target", C)?.ok_or_else(|| bad(C, "missing `target` id"))?; if source == target { return Err(bad(C, "`source` and `target` are the same object")); } @@ -1201,7 +1201,7 @@ fn target_contents(s: &mut Session, p: &Value) -> Result { /// Sample enabled attributes, refusing object transparency that a text range cannot store. fn copy_from(s: &mut Session, p: &Value) -> Result { const C: &str = "appearance.copyFrom"; - let clicked = id_param(p, "source").ok_or_else(|| bad(C, "missing `source` id"))?; + let clicked = object_param(p, "source", C)?.ok_or_else(|| bad(C, "missing `source` id"))?; let (reverse, append) = (switch_or(p, "reverse", false, C)?, switch_or(p, "append", false, C)?); let defaults_only = switch_or(p, "defaultsOnly", false, C)?; if defaults_only && (reverse || append || p.get("ids").is_some()) { diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index d392449247..055bd748af 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/crates/engine/src/cmd/buildcmds.rs @@ -29,8 +29,8 @@ use super::edit::{outermost, selected_roots}; use super::paint::paint_from; use super::pathops::{Beside, shape_node}; use super::{ - CommandSpec, EngineError, Result, Session, always, bad, check_numbers, cmd, color_value, f64_or, finite_numbers, has_doc, id_param, named_ids, - number_or, point_param, str_param, switch_or, + CommandSpec, EngineError, Result, Session, always, bad, check_numbers, cmd, color_value, f64_or, finite_numbers, has_doc, named_ids, number_or, + object_param, point_param, str_param, switch_or, }; mod live_paint; diff --git a/crates/engine/src/cmd/buildcmds/live_paint.rs b/crates/engine/src/cmd/buildcmds/live_paint.rs index 207182d40a..31cd4eaf40 100644 --- a/crates/engine/src/cmd/buildcmds/live_paint.rs +++ b/crates/engine/src/cmd/buildcmds/live_paint.rs @@ -1,7 +1,7 @@ use super::{ Appearance, Arc, Beside, Document, EDGE_NAME, EngineError, FACE_NAME, GapOptions, GapSize, LIVE_PAINT_NAME, Node, NodeId, Paint, PathData, Point, - Rect, Result, SOURCES_NAME, Selection, Session, SubPath, Value, b, bad, build_children, edge_near, face_at, finite, id_param, ids_json, - is_live_paint, json, number_or, paint_from, po, point_param, repaint_from, replace_roots, root_ids, source_nodes, source_shapes, switch_or, + Rect, Result, SOURCES_NAME, Selection, Session, SubPath, Value, b, bad, build_children, edge_near, face_at, finite, ids_json, is_live_paint, + json, number_or, object_param, paint_from, po, point_param, repaint_from, replace_roots, root_ids, source_nodes, source_shapes, switch_or, }; /// `options` as stored on a Live Paint group: none for the defaults. @@ -274,7 +274,7 @@ pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result { let pt = point_param(p, "point").filter(|q| finite(*q)).ok_or_else(|| bad(C, "missing point [x,y]"))?; crate::cmd::paint::check_keep_model(p, C)?; let paint = paint_from(s, p)?.unwrap_or_else(|| s.paint.fill.clone()); - let group = id_param(p, "group"); + let group = object_param(p, "group", C)?; let roots = if group.is_none() { root_ids(s, p)? } else { vec![] }; let (g, face) = s.edit("Live Paint Bucket", |d, sel| { let gid = match group { @@ -312,7 +312,7 @@ pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result { pub(super) fn lp_stroke_edge(s: &mut Session, p: &Value) -> Result { const C: &str = "livePaint.strokeEdge"; let pt = point_param(p, "point").filter(|q| finite(*q)).ok_or_else(|| bad(C, "missing point [x,y]"))?; - let g = id_param(p, "group").ok_or_else(|| bad(C, "missing group"))?; + let g = object_param(p, "group", C)?.ok_or_else(|| bad(C, "missing group"))?; crate::cmd::paint::check_keep_model(p, C)?; let paint = paint_from(s, p)?.unwrap_or_else(|| s.paint.stroke.clone()); let width = number_or(p, "width", s.paint.stroke_width.max(0.0), C)?; @@ -335,7 +335,10 @@ pub(super) fn lp_stroke_edge(s: &mut Session, p: &Value) -> Result { } pub(super) fn lp_info(s: &mut Session, p: &Value) -> Result { - let g = id_param(p, "group").or_else(|| id_param(p, "id")).ok_or_else(|| bad("livePaint.info", "missing group"))?; + let g = match object_param(p, "group", "livePaint.info")? { + Some(g) => g, + None => object_param(p, "id", "livePaint.info")?.ok_or_else(|| bad("livePaint.info", "missing group"))?, + }; let d = &s.doc()?.doc; let n = d.node(g).filter(|n| is_live_paint(n)).ok_or_else(|| bad("livePaint.info", format!("{g} is not a Live Paint group")))?; let faces: Vec = b::faces(n) diff --git a/crates/engine/src/cmd/distortcmds.rs b/crates/engine/src/cmd/distortcmds.rs index 17638477a9..2dbb7a9a18 100644 --- a/crates/engine/src/cmd/distortcmds.rs +++ b/crates/engine/src/cmd/distortcmds.rs @@ -15,8 +15,8 @@ use vectorcraft_tools::distort::{PinSet, arap, collect_points, mesh_for, project use super::edit::{duplicate_in, outermost, refuse_symbol_part, selected_roots}; use super::{ - CommandSpec, Result, Session, bad, check_numbers, check_switches, cmd, f64_or, f64_req, finite_numbers, has_doc, has_selection, id_param, - matrix_param, named_id, named_ids, number_or, ok, point_indices, point_param, str_param, switch_or, targets, + CommandSpec, Result, Session, bad, check_numbers, check_switches, cmd, f64_or, f64_req, finite_numbers, has_doc, has_selection, matrix_param, + named_id, named_ids, number_or, object_param, ok, point_indices, point_param, str_param, switch_or, targets, }; use crate::EngineError; @@ -305,7 +305,7 @@ fn same_points(a: Option<&WidthProfile>, b: Option<&WidthProfile>) -> bool { fn width_point_set(s: &mut Session, p: &Value) -> Result { const C: &str = "stroke.widthPoint.set"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let t = f64_req(p, "t", C)?.clamp(0.0, 1.0); let (left, right) = (f64_req(p, "left", C)?, f64_req(p, "right", C)?); if !(left.is_finite() && right.is_finite()) || left < 0.0 || right < 0.0 || left.max(right) > 1.0e5 { @@ -362,7 +362,7 @@ fn width_point_set(s: &mut Session, p: &Value) -> Result { fn width_point_copy(s: &mut Session, p: &Value) -> Result { const C: &str = "stroke.widthPoint.copy"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let t = f64_req(p, "t", C)?.clamp(0.0, 1.0); // When the profile comes out as it was (the copy replaces a side of a discontinuous point that has // its widths already), the document and its undo history stay as they are. @@ -382,7 +382,7 @@ fn width_point_copy(s: &mut Session, p: &Value) -> Result { fn width_point_remove(s: &mut Session, p: &Value) -> Result { const C: &str = "stroke.widthPoint.remove"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let indices = match (p.get("indices"), p.get("index")) { (Some(a), _) => point_indices(a), (None, Some(i)) => point_indices(&json!([i])), diff --git a/crates/engine/src/cmd/draw2.rs b/crates/engine/src/cmd/draw2.rs index 7d1c1208d8..d469c7a613 100644 --- a/crates/engine/src/cmd/draw2.rs +++ b/crates/engine/src/cmd/draw2.rs @@ -18,8 +18,8 @@ use super::create::{add_look, insert_look}; use super::newart::NewArt; use super::pathops::{Beside, remove_each}; use super::{ - CommandSpec, Result, Session, bad, check_numbers, cmd, f64_or, has_anchors, has_doc, has_selection, id_param, index, number_or, ok, str_param, - switch_or, + CommandSpec, Result, Session, bad, check_numbers, cmd, f64_or, has_anchors, has_doc, has_selection, index, number_or, object_param, ok, + str_param, switch_or, }; use crate::EngineError; diff --git a/crates/engine/src/cmd/draw2/anchors.rs b/crates/engine/src/cmd/draw2/anchors.rs index dbca59b00d..10254f3cd3 100644 --- a/crates/engine/src/cmd/draw2/anchors.rs +++ b/crates/engine/src/cmd/draw2/anchors.rs @@ -1,7 +1,7 @@ use super::common::{MAX_INPUT, path_kind, path_mut, point_entry, usize_opt, usize_req, within_input_bound}; use super::{ Anchor, BTreeSet, Beside, Color, Document, EngineError, Node, NodeId, NodeKind, Paint, PathData, Point, Result, Selection, Session, Value, Vec2, - add_look, bad, check_numbers, f64_or, id_param, json, number_or, ok, po, str_param, switch_or, + add_look, bad, check_numbers, f64_or, json, number_or, object_param, ok, po, str_param, switch_or, }; // ---------- Curvature ---------- @@ -41,7 +41,7 @@ enum CurvatureOp { pub(super) fn curvature_edit(s: &mut Session, p: &Value) -> Result { use vectorcraft_tools::draw2::{EndCurve, curvature_close, curvature_extend, curvature_insert, curvature_move}; const C: &str = "path.curvatureEdit"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let si = usize_opt(p, "subpath", C)?.unwrap_or(0); check_numbers(p, &["x", "y", "t"], C)?; let at = match (p.get("x").and_then(Value::as_f64), p.get("y").and_then(Value::as_f64)) { @@ -106,7 +106,7 @@ pub(super) fn curvature_edit(s: &mut Session, p: &Value) -> Result { pub(super) fn remove_anchor(s: &mut Session, p: &Value) -> Result { const C: &str = "path.removeAnchor"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let si = usize_opt(p, "subpath", C)?.unwrap_or(0); let ai = usize_req(p, "anchor", C)?; let curvature = switch_or(p, "curvature", false, C)?; @@ -220,7 +220,7 @@ fn remove_anchors_of( pub(super) fn convert_anchor(s: &mut Session, p: &Value) -> Result { const C: &str = "path.convertAnchor"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let si = usize_opt(p, "subpath", C)?.unwrap_or(0); let ai = usize_req(p, "anchor", C)?; let smooth = str_param(p, "to") == Some("smooth"); @@ -257,7 +257,7 @@ pub(super) fn convert_anchor(s: &mut Session, p: &Value) -> Result { pub(super) fn reshape_segment(s: &mut Session, p: &Value) -> Result { const C: &str = "path.reshapeSegment"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let si = usize_opt(p, "subpath", C)?.unwrap_or(0); let seg = usize_req(p, "segment", C)?; let t = number_or(p, "t", 0.5, C)?.clamp(0.05, 0.95); diff --git a/crates/engine/src/cmd/draw2/cut.rs b/crates/engine/src/cmd/draw2/cut.rs index 126fa57a1b..645c184b14 100644 --- a/crates/engine/src/cmd/draw2/cut.rs +++ b/crates/engine/src/cmd/draw2/cut.rs @@ -4,7 +4,7 @@ use super::common::{ }; use super::{ Anchor, AnchorKind, Beside, BezPath, BoolOp, Cap, EngineError, FillRule, Join, NodeId, ParamCurve, PathData, Point, Result, Session, SubPath, - Value, bad, eraser_area, eraser_tip, f64_or, fill_contains, id_param, json, number_or, po, + Value, bad, eraser_area, eraser_tip, f64_or, fill_contains, json, number_or, object_param, po, }; /// Split a filled path into connected pieces (each outer contour with its holes). @@ -170,7 +170,7 @@ pub(in super::super) fn erase_outline(pd: &PathData, step: f64, remove: &dyn Fn( pub(super) fn split(s: &mut Session, p: &Value) -> Result { const C: &str = "path.split"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let si = usize_opt(p, "subpath", C)?.unwrap_or(0); let anchor = usize_opt(p, "anchor", C)?; let seg = usize_opt(p, "segment", C)?; diff --git a/crates/engine/src/cmd/draw2/freehand.rs b/crates/engine/src/cmd/draw2/freehand.rs index 660f52121e..f13b0a0ba4 100644 --- a/crates/engine/src/cmd/draw2/freehand.rs +++ b/crates/engine/src/cmd/draw2/freehand.rs @@ -4,7 +4,8 @@ use super::common::{ }; use super::{ Anchor, Appearance, BTreeSet, BoolOp, Cap, Color, Document, EngineError, FillRule, Join, NewArt, Node, NodeId, NodeKind, Paint, PathData, Point, - Result, Selection, Session, SimplifyOptions, SubPath, Value, bad, id_param, insert_look, json, number_or, po, remove_each, str_param, switch_or, + Result, Selection, Session, SimplifyOptions, SubPath, Value, bad, insert_look, json, number_or, object_param, po, remove_each, str_param, + switch_or, }; // ---------- Pencil / Paintbrush ---------- @@ -158,7 +159,7 @@ pub(super) fn freehand(s: &mut Session, p: &Value) -> Result { let keep_selected = switch_or(p, "keepSelected", true, C)?; let filled = switch_or(p, "fill", false, C)?; if let Some(ext) = p.get("extend").filter(|v| !v.is_null()) { - let id = id_param(ext, "id").ok_or_else(|| bad(C, "extend needs id"))?; + let id = object_param(ext, "id", C)?.ok_or_else(|| bad(C, "extend needs id"))?; let at_start = str_param(ext, "end") == Some("start"); let (pd, _) = path_of(&s.doc()?.doc, id)?; let si = pd.subpaths.iter().rposition(|s| !s.closed && !s.anchors.is_empty()).ok_or_else(|| bad(C, "path has no open end"))?; @@ -179,7 +180,7 @@ pub(super) fn freehand(s: &mut Session, p: &Value) -> Result { // open subpath, that subpath starting at the end the stroke ends on, it ends at its end). let connect = match p.get("connect").filter(|v| !v.is_null()) { Some(cn) => { - let other = id_param(cn, "id").ok_or_else(|| bad(C, "connect needs id"))?; + let other = object_param(cn, "id", C)?.ok_or_else(|| bad(C, "connect needs id"))?; if other == id { return Err(bad(C, "connect needs a path other than the one extended")); } @@ -299,7 +300,7 @@ pub(super) fn freehand(s: &mut Session, p: &Value) -> Result { return Ok(json!({ "id": id.0 })); } if let Some(rd) = p.get("redraw").filter(|v| !v.is_null()) { - let id = id_param(rd, "id").ok_or_else(|| bad(C, "redraw needs id"))?; + let id = object_param(rd, "id", C)?.ok_or_else(|| bad(C, "redraw needs id"))?; let within = number_or(rd, "within", 12.0, C)?; if !within.is_finite() || within < 0.0 { return Err(bad(C, "redraw within must be a finite distance of 0 or more")); diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 7a8a91f0af..fe683bfa9b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -10,8 +10,8 @@ use vectorcraft_geom::{Affine, Point, Vec2}; use super::clipboard::SwatchChoices; use super::{ CommandSpec, Result, Session, always, bad, bool_or, can_clear_history, can_redo, can_undo, checked_ids_param, clock_date, cmd, date_param, - f64_or, f64_req, finite_numbers, has_clipboard, has_doc, has_selection, has_selection_guides_or_artboard_tool, has_selection_or_guides, id_param, - index, ok, point_param, str_param, switch_or, + f64_or, f64_req, finite_numbers, has_clipboard, has_doc, has_selection, has_selection_guides_or_artboard_tool, has_selection_or_guides, index, + object_param, ok, point_param, str_param, switch_or, }; use crate::{Clipboard, EngineError, inspect}; @@ -267,7 +267,7 @@ fn doc_move(s: &mut Session, p: &Value) -> Result { } fn doc_node(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("document.node", "missing id"))?; + let id = object_param(p, "id", "document.node")?.ok_or_else(|| bad("document.node", "missing id"))?; let mut opts = slice_opts(p, "document.node")?; let summary = bool_or(p, "summary", false); if !summary && (opts.depth.is_some() || opts.child_limit.is_some()) { diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 49b1015bf4..593d325460 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -15,7 +15,7 @@ use vectorcraft_geom::{Affine, Rect}; use super::{ CommandSpec, Result, Session, bad, bool_or, check_numbers, check_switches, checked_id, checked_ids_param, cmd, color_value, f64_or, has_doc, - has_selection, id_param, ids_param, index, ok, str_param, switch_or, + has_selection, ids_param, index, object_param, ok, str_param, switch_or, }; use crate::EngineError; @@ -426,7 +426,7 @@ fn paste_remembers_layers(s: &mut Session, p: &Value) -> Result { /// The Layers panel's target circle. fn target(s: &mut Session, p: &Value) -> Result { const C: &str = "layer.target"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let st = s.doc()?; let n = st.doc.node(id).ok_or(EngineError::NoNode(id))?; if !st.doc.is_editable(id) { @@ -594,7 +594,7 @@ fn duplicate_rows(s: &mut Session, p: &Value) -> Result { fn set_current(s: &mut Session, p: &Value) -> Result { const C: &str = "layer.setCurrent"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let st = s.doc_mut()?; let layer = st.doc.layer_containing(id).ok_or(EngineError::NoNode(id))?; st.active_layer = Some(layer); @@ -700,7 +700,7 @@ fn in_isolation(doc: &Document, isolation: Option, id: NodeId) -> bool { /// The selection column: select a row's art. fn select_art(s: &mut Session, p: &Value) -> Result { const C: &str = "layer.selectAll"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let add = switch_or(p, "add", false, C)?; let st = s.doc()?; let n = st.doc.node(id).ok_or(EngineError::NoNode(id))?; @@ -747,9 +747,12 @@ enum Place { /// Drag rows in the Layers panel. fn move_rows(s: &mut Session, p: &Value) -> Result { const C: &str = "layer.move"; - let mut ids = ids_param(p, "ids", C)?.or_else(|| id_param(p, "id").map(|i| vec![i])).ok_or_else(|| bad(C, "missing ids"))?; + let mut ids = match ids_param(p, "ids", C)? { + Some(ids) => ids, + None => vec![object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing ids"))?], + }; ids.truncate(MAX_ROWS); - let target = id_param(p, "target").ok_or_else(|| bad(C, "missing target"))?; + let target = object_param(p, "target", C)?.ok_or_else(|| bad(C, "missing target"))?; let place = match str_param(p, "place").unwrap_or("inside") { "above" => Place::Above, "below" => Place::Below, @@ -831,8 +834,8 @@ fn move_rows(s: &mut Session, p: &Value) -> Result { fn move_node(s: &mut Session, p: &Value) -> Result { const C: &str = "node.move"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; - let parent = id_param(p, "parent"); + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; + let parent = object_param(p, "parent", C)?; let index = p.get("index").and_then(Value::as_u64).map_or(usize::MAX, index); let d = &s.doc()?.doc; let n = d.node(id).ok_or(EngineError::NoNode(id))?; diff --git a/crates/engine/src/cmd/live.rs b/crates/engine/src/cmd/live.rs index 464f241159..618865cede 100644 --- a/crates/engine/src/cmd/live.rs +++ b/crates/engine/src/cmd/live.rs @@ -21,7 +21,7 @@ use super::expand::gradient_sampler; use super::pathops::Beside; use super::{ CommandSpec, EngineError, Result, Session, always, bad, check_numbers, cmd, color_param, f64_or, f64_req, finite_numbers, has_doc, has_multi, - has_selection, id_param, named_ids, number_or, ok, str_param, switch_or, text_param, + has_selection, named_ids, number_or, object_param, ok, str_param, switch_or, text_param, }; mod blend; diff --git a/crates/engine/src/cmd/live/envelope.rs b/crates/engine/src/cmd/live/envelope.rs index 1aceb49b54..3412023280 100644 --- a/crates/engine/src/cmd/live/envelope.rs +++ b/crates/engine/src/cmd/live/envelope.rs @@ -3,7 +3,7 @@ use super::common::{fix_ids, ids_json, is_envelope, roots_param, selected_of, wr use super::mesh::{handle_param, index_param, whole}; use super::{ Affine, Appearance, Arc, Beside, Color, Document, EngineError, EnvelopeKind, EnvelopeMap, EnvelopeOptions, Node, NodeId, NodeKind, Paint, - PathData, Point, PreserveShape, Result, Session, Value, bad, check_numbers, f64_or, f64_req, id_param, json, live, ok, switch_or, text_param, + PathData, Point, PreserveShape, Result, Session, Value, bad, check_numbers, f64_or, f64_req, json, live, object_param, ok, switch_or, text_param, }; // ---------- Envelope ---------- @@ -419,7 +419,7 @@ pub(super) fn env_edit_contents(s: &mut Session, p: &Value) -> Result { pub(super) fn env_set_mesh_point(s: &mut Session, p: &Value) -> Result { const C: &str = "object.envelope.setMeshPoint"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?; let index = index_param(p, "index", C)?; let q = Point::new(f64_req(p, "x", C)?, f64_req(p, "y", C)?); let handle = handle_param(p, C)?; diff --git a/crates/engine/src/cmd/live/mesh.rs b/crates/engine/src/cmd/live/mesh.rs index 70c2d9f348..187d2149f2 100644 --- a/crates/engine/src/cmd/live/mesh.rs +++ b/crates/engine/src/cmd/live/mesh.rs @@ -4,7 +4,7 @@ use super::common::{fix_ids, ids_json, is_mesh, replace_with, roots_param, selec use super::envelope::{outline_of, rows_cols}; use super::{ Color, Document, EngineError, EnvelopeKind, GradientMesh, MeshAppearance, Node, NodeId, NodeKind, Paint, Point, Result, Session, Value, bad, - check_numbers, color_param, f64_or, f64_req, finite_numbers, gradient_sampler, id_param, json, live, ok, str_param, + check_numbers, color_param, f64_or, f64_req, finite_numbers, gradient_sampler, json, live, object_param, ok, str_param, }; // ---------- Gradient mesh ---------- @@ -115,7 +115,7 @@ fn image_mesh(d: &Document, im: &vectorcraft_doc::ImageObject, rows: u32, cols: /// Run `f` on the gradient mesh `p.id` (one undo step `label`); with `envelopes`, a mesh /// envelope's grid too (see [`edit_grid`]). fn with_mesh(s: &mut Session, p: &Value, cmd: &str, label: &str, envelopes: bool, f: impl FnOnce(&mut GradientMesh) -> Result) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad(cmd, "missing `id`"))?; + let id = object_param(p, "id", cmd)?.ok_or_else(|| bad(cmd, "missing `id`"))?; s.edit(label, move |d, _| edit_grid(d, id, cmd, envelopes, f)) } @@ -147,7 +147,7 @@ const MAX_POINTS: usize = 1 << 20; /// points}…]}`. Each mesh comes once, with its points in order and without repeats. fn point_items(p: &Value, cmd: &str) -> Result)>> { let one = |q: &Value| -> Result<(NodeId, BTreeSet)> { - let id = id_param(q, "id").ok_or_else(|| bad(cmd, "missing `id`"))?; + let id = object_param(q, "id", cmd)?.ok_or_else(|| bad(cmd, "missing `id`"))?; let points = match q.get("points") { None | Some(Value::Null) => BTreeSet::from([index_param(q, "index", cmd)?]), Some(Value::Array(v)) if v.len() <= MAX_POINTS => v diff --git a/crates/engine/src/cmd/path.rs b/crates/engine/src/cmd/path.rs index 712ca8a179..518b2062f8 100644 --- a/crates/engine/src/cmd/path.rs +++ b/crates/engine/src/cmd/path.rs @@ -12,7 +12,7 @@ use super::draw2::usize_opt; use super::pathops::Beside; use super::{ CommandSpec, Result, Session, bad, check_numbers, check_switches, cmd, f64_or, f64_req, finite_numbers, has_anchors, has_doc, has_selected_path, - has_selection, id_param, ids_param, index, number_or, ok, str_param, switch_or, targets, + has_selection, ids_param, index, number_or, object_param, ok, str_param, switch_or, targets, }; use crate::EngineError; @@ -168,7 +168,7 @@ pub(super) fn path_mut(d: &mut vectorcraft_doc::Document, id: NodeId) -> Result< } fn append_anchor(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("path.appendAnchor", "missing id"))?; + let id = object_param(p, "id", "path.appendAnchor")?.ok_or_else(|| bad("path.appendAnchor", "missing id"))?; let a = anchor_from_json(p, "path.appendAnchor")?; let subpath = p.get("subpath").and_then(Value::as_u64).map(|u| u as usize); s.edit("Pen", |d, sel| { @@ -204,7 +204,7 @@ fn append_anchor(s: &mut Session, p: &Value) -> Result { /// Close the active open subpath of `id`, optionally setting the incoming handle of the first anchor. fn close(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("path.close", "missing id"))?; + let id = object_param(p, "id", "path.close")?.ok_or_else(|| bad("path.close", "missing id"))?; let subpath = p.get("subpath").and_then(Value::as_u64).map(|u| u as usize); let h_in = match p.get("in").filter(|h| !h.is_null()) { None => None, @@ -297,7 +297,7 @@ pub(crate) fn transform_anchors(s: &mut Session, label: &str, xf: vectorcraft_ge } fn set_handle(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("path.setHandle", "missing id"))?; + let id = object_param(p, "id", "path.setHandle")?.ok_or_else(|| bad("path.setHandle", "missing id"))?; let si = usize_opt(p, "subpath", "path.setHandle")?.unwrap_or(0); let ai = p.get("anchor").and_then(Value::as_u64).map(index).ok_or_else(|| bad("path.setHandle", "missing anchor"))?; let out = str_param(p, "which") != Some("in"); @@ -315,7 +315,7 @@ fn set_handle(s: &mut Session, p: &Value) -> Result { /// `path.moveAnchor`: one anchor to (x, y), its handles keeping their offsets (the Anchor Point /// tool's Shift-drag). fn move_anchor(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("path.moveAnchor", "missing id"))?; + let id = object_param(p, "id", "path.moveAnchor")?.ok_or_else(|| bad("path.moveAnchor", "missing id"))?; let si = usize_opt(p, "subpath", "path.moveAnchor")?.unwrap_or(0); let ai = p.get("anchor").and_then(Value::as_u64).map(index).ok_or_else(|| bad("path.moveAnchor", "missing anchor"))?; let pos = Point::new(f64_req(p, "x", "path.moveAnchor")?, f64_req(p, "y", "path.moveAnchor")?); @@ -335,7 +335,7 @@ fn move_anchor(s: &mut Session, p: &Value) -> Result { } fn set_anchors(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("path.setAnchors", "missing id"))?; + let id = object_param(p, "id", "path.setAnchors")?.ok_or_else(|| bad("path.setAnchors", "missing id"))?; let subs = p.get("subpaths").and_then(Value::as_array).ok_or_else(|| bad("path.setAnchors", "missing subpaths"))?; let data = PathData::new( subs.iter() @@ -1556,7 +1556,7 @@ fn renumber_after_insert(sel: &mut Selection, id: NodeId, si: usize, added: usiz } fn reshape(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("path.reshape", "missing id"))?; + let id = object_param(p, "id", "path.reshape")?.ok_or_else(|| bad("path.reshape", "missing id"))?; let at = Point::new(f64_req(p, "x", "path.reshape")?, f64_req(p, "y", "path.reshape")?); check_numbers(p, &["dx", "dy", "tol", "radius"], "path.reshape")?; let delta = Vec2::new(f64_or(p, "dx", 0.0), f64_or(p, "dy", 0.0)); @@ -1678,7 +1678,7 @@ fn reshape(s: &mut Session, p: &Value) -> Result { } fn insert_anchor(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("path.insertAnchor", "missing id"))?; + let id = object_param(p, "id", "path.insertAnchor")?.ok_or_else(|| bad("path.insertAnchor", "missing id"))?; let si = usize_opt(p, "subpath", "path.insertAnchor")?.unwrap_or(0); let seg = p.get("segment").and_then(Value::as_u64).map(index).ok_or_else(|| bad("path.insertAnchor", "missing segment"))?; let t = number_or(p, "t", 0.5, "path.insertAnchor")?.clamp(0.0, 1.0); diff --git a/crates/engine/src/cmd/slices.rs b/crates/engine/src/cmd/slices.rs index 06ec067934..787d48e441 100644 --- a/crates/engine/src/cmd/slices.rs +++ b/crates/engine/src/cmd/slices.rs @@ -13,7 +13,7 @@ use vectorcraft_doc::{Appearance, CellAlign, CellVAlign, Document, Node, NodeKin use vectorcraft_geom::{Rect, shapes}; use super::{ - CommandSpec, EngineError, NodeId, Result, Session, bad, bool_or, cmd, f64_or, f64_req, has_doc, has_selection, id_param, ids_param, index, + CommandSpec, EngineError, NodeId, Result, Session, bad, bool_or, cmd, f64_or, f64_req, has_doc, has_selection, ids_param, index, object_param, switch_or, targets, }; @@ -679,7 +679,7 @@ fn create(s: &mut Session, p: &Value) -> Result { fn set_rect(s: &mut Session, p: &Value) -> Result { const C: &str = "object.slice.setRect"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?; let r = rect_param(p, C)?; let d = &s.doc()?.doc; if d.slice(id).is_none() { diff --git a/crates/engine/src/cmd/textedit.rs b/crates/engine/src/cmd/textedit.rs index 21ed6d8023..4bcbd6c932 100644 --- a/crates/engine/src/cmd/textedit.rs +++ b/crates/engine/src/cmd/textedit.rs @@ -11,8 +11,8 @@ use super::menucmds::whole_or; use super::stroke::StrokeChange; use super::typecmd::refresh_bounds; use super::{ - CommandSpec, EngineError, Result, Session, bad, bool_or, check_numbers, check_switches, cmd, color_value, has_doc, has_selection, id_param, - index, number_or, point_param, str_param, switch_or, targets, text_param, + CommandSpec, EngineError, Result, Session, bad, bool_or, check_numbers, check_switches, cmd, color_value, has_doc, has_selection, index, + number_or, object_param, point_param, str_param, switch_or, targets, text_param, }; pub fn specs() -> Vec { @@ -272,7 +272,7 @@ fn range_of(p: &Value, len: usize, cmd: &str) -> Result<(usize, usize)> { fn edit_range(s: &mut Session, p: &Value) -> Result { const C: &str = "text.editRange"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?; let t = text_ref(s, id)?; let len = edit::runs_len(&t.runs); let (a, b) = range_of(p, len, C)?; @@ -337,7 +337,7 @@ fn clear_param(s: &Session, p: &Value, id: NodeId, c: &str) -> Result Result { const C: &str = "text.startList"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?; let t = text_ref(s, id)?; let len = edit::runs_len(&t.runs); let at = offset_or(p, "at", len, C)?.min(len); @@ -365,7 +365,7 @@ fn start_list(s: &mut Session, p: &Value) -> Result { } fn get_range(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("text.getRange", "missing `id`"))?; + let id = object_param(p, "id", "text.getRange")?.ok_or_else(|| bad("text.getRange", "missing `id`"))?; if switch_or(p, "story", false, "text.getRange")? { return get_story(s, p, id); } @@ -1254,7 +1254,7 @@ fn drawn_glyph( fn set_glyph_alternate(s: &mut Session, p: &Value) -> Result { const C: &str = "text.setGlyphAlternate"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?; let gid = match p.get("gid") { None => return Err(bad(C, "missing `gid` (a glyph id, or null for the default glyph)")), Some(Value::Null) => None, @@ -1344,7 +1344,7 @@ fn set_glyph_alternate(s: &mut Session, p: &Value) -> Result { fn glyph_alternates(s: &mut Session, p: &Value) -> Result { const C: &str = "text.glyphAlternates"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?; let t = text_ref(s, id)?; let (ch, bytes, face) = glyph_target(&t, p, C)?; let default = face.glyph_for(ch); @@ -1359,7 +1359,7 @@ fn glyph_alternates(s: &mut Session, p: &Value) -> Result { fn touch_type(s: &mut Session, p: &Value) -> Result { const C: &str = "text.touchType"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?; check_numbers(p, &["start"], C)?; let start = byte_param(p, "start").ok_or_else(|| bad(C, "missing `start` (the byte offset of a character)"))?; if !["dx", "dy", "hScale", "vScale", "rotation"].iter().any(|k| p.get(*k).is_some_and(|v| !v.is_null())) { @@ -1585,7 +1585,7 @@ fn touch_type(s: &mut Session, p: &Value) -> Result { fn create_in_path(s: &mut Session, p: &Value) -> Result { const C: &str = "text.createInPath"; - let pid = id_param(p, "path").ok_or_else(|| bad(C, "missing `path` (a path object id)"))?; + let pid = object_param(p, "path", C)?.ok_or_else(|| bad(C, "missing `path` (a path object id)"))?; let on_path = match str_param(p, "mode").unwrap_or("area") { "area" => false, "onPath" | "path" => true, @@ -1975,7 +1975,7 @@ fn fit_headline(s: &mut Session, p: &Value) -> Result { fn discard_empty(s: &mut Session, p: &Value) -> Result { const C: &str = "text.discardEmpty"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?; if edit::runs_len(&text_ref(s, id)?.runs) > 0 { return Ok(json!({"removed": false})); } diff --git a/crates/engine/src/cmd/typecmd.rs b/crates/engine/src/cmd/typecmd.rs index 6d29550fa0..bf5de3a0cb 100644 --- a/crates/engine/src/cmd/typecmd.rs +++ b/crates/engine/src/cmd/typecmd.rs @@ -10,8 +10,8 @@ use vectorcraft_geom::{Affine, PathData, Vec2}; use super::edit::selected_roots; use super::pathops::{length_given, number_given, shape_node}; use super::{ - CommandSpec, EngineError, Result, Session, bad, check_numbers, cmd, color_value, f64_req, has_doc, has_selection, id_param, index, number_or, ok, - str_param, switch_or, targets, text_param, + CommandSpec, EngineError, Result, Session, bad, check_numbers, cmd, color_value, f64_req, has_doc, has_selection, index, number_or, object_param, + ok, str_param, switch_or, targets, text_param, }; pub fn specs() -> Vec { @@ -1222,7 +1222,7 @@ fn reshape_area(s: &mut Session, p: &Value) -> Result { }; let mut items = Vec::with_capacity(raw.len()); for it in raw { - let id = id_param(it, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(it, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let mut refs = super::select::checked_refs(it.get("anchors"), C)?; refs.sort_unstable(); refs.dedup(); @@ -1253,7 +1253,7 @@ fn reshape_area(s: &mut Session, p: &Value) -> Result { fn reshape_path(s: &mut Session, p: &Value) -> Result { const C: &str = "text.reshapePath"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let mut refs = super::select::checked_refs(p.get("anchors"), C)?; refs.sort_unstable(); refs.dedup(); @@ -1284,7 +1284,7 @@ fn reshape_path(s: &mut Session, p: &Value) -> Result { fn set_path_handle(s: &mut Session, p: &Value) -> Result { const C: &str = "text.setPathHandle"; - let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; + let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?; let si = super::draw2::usize_opt(p, "subpath", C)?.unwrap_or(0); let ai = p.get("anchor").and_then(Value::as_u64).map(index).ok_or_else(|| bad(C, "missing anchor"))?; let out = super::object::choice(p, C, "which", &["in", "out"])? != Some("in"); diff --git a/crates/engine/src/cmd/wand.rs b/crates/engine/src/cmd/wand.rs index c79ec0b86e..218871972f 100644 --- a/crates/engine/src/cmd/wand.rs +++ b/crates/engine/src/cmd/wand.rs @@ -8,7 +8,7 @@ use vectorcraft_color::Paint; use vectorcraft_doc::{Document, Node, NodeId, NodeKind}; use super::select::selectable_ids; -use super::{CommandSpec, EngineError, Result, Session, always, bad, bool_or, cmd, has_doc, id_param, str_param}; +use super::{CommandSpec, EngineError, Result, Session, always, bad, bool_or, cmd, has_doc, object_param, str_param}; /// Magic Wand panel settings. Tolerances: colours as 0–255 RGB distance, stroke weight in /// points, opacity in percent. @@ -152,7 +152,7 @@ fn paintable(doc: &Document) -> Vec { } fn magic_wand(s: &mut Session, p: &Value) -> Result { - let id = id_param(p, "id").ok_or_else(|| bad("select.magicWand", "missing id"))?; + let id = object_param(p, "id", "select.magicWand")?.ok_or_else(|| bad("select.magicWand", "missing id"))?; let st = s.doc()?; let reference = st.doc.node(id).ok_or(EngineError::NoNode(id))?.clone(); let w = s.menu.wand.clone(); diff --git a/crates/engine/src/tests/params.rs b/crates/engine/src/tests/params.rs index b0345db4ff..6850715b0e 100644 --- a/crates/engine/src/tests/params.rs +++ b/crates/engine/src/tests/params.rs @@ -400,6 +400,90 @@ fn stroke_set_refuses_an_unknown_key() { } } +/// An object id given as something other than a whole number (`"5"`, `-1`) is refused as such, +/// naming its key, not as a missing id; an id left out is still a missing one. +#[test] +fn a_malformed_object_id_is_not_reported_as_missing() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 40})).unwrap(); + let undo = |s: &Session| s.doc().unwrap().history.undo.len(); + let before = undo(&s); + let points = json!([[0, 0], [10, 10], [20, 0]]); + for bad in [json!("5"), json!(-1)] { + let id = || json!({ "id": bad }); + let with = |extra: serde_json::Value| { + let mut p = id(); + p.as_object_mut().unwrap().extend(extra.as_object().unwrap().clone()); + p + }; + for (cmd, p, key) in [ + ("stroke.widthPoint.set", id(), "id"), + ("stroke.widthPoint.copy", id(), "id"), + ("stroke.widthPoint.remove", id(), "id"), + ("path.curvatureEdit", id(), "id"), + ("path.removeAnchor", id(), "id"), + ("path.convertAnchor", id(), "id"), + ("path.reshapeSegment", id(), "id"), + ("path.split", id(), "id"), + ("path.appendAnchor", id(), "id"), + ("path.close", id(), "id"), + ("path.setHandle", id(), "id"), + ("path.moveAnchor", id(), "id"), + ("path.setAnchors", id(), "id"), + ("path.reshape", id(), "id"), + ("path.insertAnchor", id(), "id"), + ("path.freehand", json!({"points": points, "extend": {"id": bad}}), "id"), + ("path.freehand", json!({"points": points, "redraw": {"id": bad}}), "id"), + ("document.node", id(), "id"), + ("layer.target", id(), "id"), + ("layer.setCurrent", id(), "id"), + ("layer.selectAll", id(), "id"), + ("layer.move", with(json!({"target": 1})), "id"), + ("layer.move", json!({"ids": [1], "target": bad}), "target"), + ("node.move", id(), "id"), + ("node.move", json!({"id": 1, "parent": bad}), "parent"), + ("text.reshapeArea", with(json!({"anchors": []})), "id"), + ("text.reshapeArea", json!({"items": [{"id": bad, "anchors": []}]}), "id"), + ("text.reshapePath", id(), "id"), + ("text.setPathHandle", id(), "id"), + ("text.editRange", id(), "id"), + ("text.startList", id(), "id"), + ("text.getRange", id(), "id"), + ("text.setGlyphAlternate", id(), "id"), + ("text.glyphAlternates", id(), "id"), + ("text.touchType", id(), "id"), + ("text.discardEmpty", id(), "id"), + ("text.createInPath", json!({"path": bad}), "path"), + ("object.envelope.setMeshPoint", id(), "id"), + ("object.mesh.setPointColor", with(json!({"index": 0})), "id"), + ("object.mesh.movePoint", with(json!({"index": 0, "x": 0, "y": 0})), "id"), + ("object.mesh.movePoints", json!({"items": [{"id": bad, "index": 0}]}), "id"), + ("object.mesh.addLine", with(json!({"x": 0, "y": 0})), "id"), + ("object.mesh.deletePoint", with(json!({"index": 0})), "id"), + ("object.slice.setRect", id(), "id"), + ("appearance.transfer", json!({"source": bad, "target": 1}), "source"), + ("appearance.transfer", json!({"source": 1, "target": bad}), "target"), + ("appearance.copyFrom", json!({"source": bad}), "source"), + ("livePaint.strokeEdge", json!({"point": [0, 0], "group": bad}), "group"), + ("livePaint.fill", json!({"point": [0, 0], "group": bad}), "group"), + ("livePaint.info", json!({"group": bad}), "group"), + ("livePaint.info", id(), "id"), + ("select.magicWand", id(), "id"), + ] { + let e = s.execute(cmd, &p).unwrap_err().to_string(); + // `layer.move` checks an `id` that stands in for `ids` before it runs. + let named = e.contains(&format!("`{key}` must be a whole number")) || (cmd == "layer.move" && e.contains("invalid object id")); + assert!(named, "{cmd} {p}: {e}"); + } + } + for cmd in ["path.close", "layer.target", "text.reshapePath", "object.mesh.movePoint", "select.magicWand"] { + let e = s.execute(cmd, &json!({})).unwrap_err().to_string(); + assert!(e.contains("missing"), "{cmd}: {e}"); + } + assert_eq!(undo(&s), before); +} + /// A command that takes the keys of one that isn't checked yet isn't checked either. #[test] fn a_reference_to_an_unchecked_command_is_unchecked() { diff --git a/docs/mcp.md b/docs/mcp.md index 5826c5c8c4..892541c6a8 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -430,7 +430,13 @@ or a new object (`css.export`, `object.isolate`, `layer.locate`, `layer.releaseT counterparts, `image.info`, `links.info`, `links.goTo`, `links.unembed`, `shaper.release`, `shaper.expand`, `transparency.editOpacityMask`, `transparency.viewOpacityMask`, `symbol.edit`, the `object.blend.spine.*` commands and `path.curvature`) refuse an `id` that is not a non-negative integer (`"abc"`, `"5"`, `true`) with an error and -change nothing; an omitted or null `id` keeps the default. A command that leaves out an object selected inside a +change nothing; an omitted or null `id` keeps the default. Commands that need an object id (`document.node`, the +`path.*` anchor and handle commands, `text.reshapePath`, `text.editRange`, the `object.mesh.*` point commands, +`layer.target`, `node.move`, `select.magicWand` and the others like them, and the `source`, `target`, `group` and +`path` keys of `appearance.transfer`, `appearance.copyFrom`, `layer.move`, the `livePaint.*` commands and +`text.createInPath`) refuse a value that is not a non-negative integer with "`id` must be a whole number", naming the +key, and report a key left out or null as missing. The optional `group` of `livePaint.fill` and `parent` of +`node.move` are refused the same way when given; left out, they keep their defaults. A command that leaves out an object selected inside a selected group also leaves it out when `ids` list it together with a group that holds it. Every command that takes `ids` (or `slices`, for the slice commands) refuses a list with an entry that is not a non-negative integer, or a value that is not a list, and changes nothing; it does not act on the other entries. The error for an entry names the key, From 133e2c135695b54396aa102acfe70f7a0c8c22ff Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 13:17:08 -0800 Subject: [PATCH 2/2] MCP: livePaint.info reads id when group is left out The docs said every livePaint key left out is reported as missing. livePaint.info reads `id` when `group` is left out or null, and reports a missing group only when both are left out. --- docs/mcp.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/mcp.md b/docs/mcp.md index 892541c6a8..6287f5b3a7 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -436,7 +436,8 @@ change nothing; an omitted or null `id` keeps the default. Commands that need an `path` keys of `appearance.transfer`, `appearance.copyFrom`, `layer.move`, the `livePaint.*` commands and `text.createInPath`) refuse a value that is not a non-negative integer with "`id` must be a whole number", naming the key, and report a key left out or null as missing. The optional `group` of `livePaint.fill` and `parent` of -`node.move` are refused the same way when given; left out, they keep their defaults. A command that leaves out an object selected inside a +`node.move` are refused the same way when given; left out, they keep their defaults. `livePaint.info` reads `id` when +`group` is left out or null, and reports a missing group only when both are left out. A command that leaves out an object selected inside a selected group also leaves it out when `ids` list it together with a group that holds it. Every command that takes `ids` (or `slices`, for the slice commands) refuses a list with an entry that is not a non-negative integer, or a value that is not a list, and changes nothing; it does not act on the other entries. The error for an entry names the key,