diff --git a/ROADMAP.md b/ROADMAP.md index 7f7bf945..91b98d24 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -125,6 +125,7 @@ grade by behaviour against `plan/illustrator/`, not by whether a menu item exist - **Tools:** - **Shaper:** uneven squares and rectangles become live rectangles even when drawing begins mid-edge. Recognition uses uniformly spaced samples and closed-loop corners, fits rectangle edges/angles rather than requiring a nearly perfect bounding-box fill, and includes the pointer-release position. Rectangles/squares and ellipses snap to 45° steps, hexagons to 90°, and triangles point up or down; lines keep their free angle even with Shift. Shaper Groups retain the live originals, merge regions with the gesture-origin fill, punch fills independently from strokes, remove boundaries on exiting scribbles and trim protruding stroke pieces. Source moves/resizes re-evaluate the result in the same undo step; double-click enters construction editing. Native save/reopen, face recoloring, Release and Expand are covered. The construction-mode widget and finer gesture heuristics remain. - **Selection:** Selection, Direct/Group Selection, Magic Wand, Lasso. A Shift-drag marquee toggles what it reaches: objects with the Selection tool, anchors with Direct and Group Selection (#483); the Lasso adds with Shift and subtracts with Alt. Dragging a path segment with Direct Selection bends it when it's curved (its anchors stay) and moves its two anchors when it's straight, one undo step; the fill, Shift or Alt still pick the whole path (Alt-drag copies it). + Agent selection commands validate every object id before changing the selection and report the resulting count and ids (#784). - **Saved selections:** Select ▸ Save Selection… asks for a name and keeps the selection in the document (saved with the file, undoable, up to 25); each one is listed at the bottom of the Select menu and a click selects its objects again; Edit Selection… lists them to rename or delete, applied together on OK. A file's saved selections are capped and checked against its objects when it opens. Not yet: Update Selection and the Layers panel's saved-selection menu. - **Drawing:** Pen, Curvature, anchor tools, Pencil, Paintbrush, Blob Brush, Smooth, Path Eraser, Join. Direction handles dragged with Direct Selection or the Anchor Point tool snap to smart guides, and Shift keeps them at 45° steps from their anchor. Every point a drawing tool places snaps to Smart Guides through one snapper (#506): the Pen's and Curvature tool's anchors, the Pencil's ends, the shape tools' start and dragged corner or radius (Rectangle, Rounded Rectangle, Ellipse, Polygon, Star, Line Segment, Arc, Spiral, the grids), the Type tool's click and frame, a new artboard's corners. - **Shapes:** all shape tools (including Flare) and the line, arc, spiral and grid tools. Space held while dragging out a shape, line, arc, spiral or grid moves it at its current size. While a grid is dragged out, Up/Down change its rows (concentric dividers) and Left/Right its columns (radial dividers). diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 53080ae8..14b350a0 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -304,6 +304,21 @@ pub(crate) fn id_param(p: &Value, key: &str) -> Option { pub(crate) fn ids_param(p: &Value, key: &str) -> Option> { p.get(key).and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_u64).map(NodeId).collect()) } +/// Validate all explicit targets before a command changes the document or selection. +pub(crate) fn checked_ids_param(s: &Session, p: &Value, key: &str, command: &str) -> Result> { + let values = + p.get(key).and_then(Value::as_array).ok_or_else(|| bad(command, format!("{key} must be an array of non-negative integer object ids")))?; + values.iter().map(|value| checked_id(s, value, command)).collect() +} + +pub(crate) fn checked_id(s: &Session, value: &Value, command: &str) -> Result { + let id = value.as_u64().map(NodeId).ok_or_else(|| bad(command, format!("invalid object id {value}: expected a non-negative integer")))?; + if s.doc()?.doc.node(id).is_none() { + return Err(bad(command, format!("no such object id {value}"))); + } + Ok(id) +} + pub(crate) fn point_param(p: &Value, key: &str) -> Option { let a = p.get(key)?.as_array()?; Some(Point::new(a.first()?.as_f64()?, a.get(1)?.as_f64()?)) diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 27d28684..0dbda515 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -25,14 +25,30 @@ pub fn specs() -> Vec { cmd!("select.inverse", "Inverse", ["Select"], None, "{}", has_doc, inverse), cmd!("select.nextAbove", "Next Object Above", ["Select"], Some("Cmd+Alt+]"), "{}", has_selection, |s, _| step(s, 1)), cmd!("select.nextBelow", "Next Object Below", ["Select"], Some("Cmd+Alt+["), "{}", has_selection, |s, _| step(s, -1)), - cmd!("select.set", "Select Objects", [], None, "{ids: [id…]}", has_doc, set), - cmd!("select.add", "Add to Selection", [], None, "{ids: [id…]}", has_doc, add), + cmd!( + "select.set", + "Select Objects", + [], + None, + "{ids: [id…]} all ids must be non-negative integers naming existing objects → {count, ids} (resulting selection)", + has_doc, + set + ), + cmd!( + "select.add", + "Add to Selection", + [], + None, + "{ids: [id…]} all ids must be non-negative integers naming existing objects → {count, ids} (resulting selection)", + has_doc, + add + ), cmd!( "select.toggle", "Toggle Selection", [], None, - "{id} or {ids: [id…]}: each selected one leaves the selection, the others join it", + "{id} or {ids: [id…]}: each selected one leaves the selection, the others join it; all ids must be non-negative integers naming existing objects → {count, ids} (resulting selection)", has_doc, toggle ), @@ -225,29 +241,39 @@ fn step(s: &mut Session, dir: i64) -> Result { } fn set(s: &mut Session, p: &Value) -> Result { - let ids = ids_param(p, "ids").ok_or_else(|| bad("select.set", "missing ids"))?; - s.select(|d, sel| sel.set(ids.iter().copied().filter(|i| d.node(*i).is_some())))?; - ok() + let ids = checked_ids_param(s, p, "ids", "select.set")?; + s.select(|_, sel| sel.set(ids.iter().copied()))?; + selection_result(s) } fn add(s: &mut Session, p: &Value) -> Result { - let ids = ids_param(p, "ids").ok_or_else(|| bad("select.add", "missing ids"))?; + let ids = checked_ids_param(s, p, "ids", "select.add")?; s.select(|_, sel| { for i in ids { sel.add(i); } })?; - ok() + selection_result(s) } fn toggle(s: &mut Session, p: &Value) -> Result { - let ids = ids_param(p, "ids").or_else(|| id_param(p, "id").map(|id| vec![id])).ok_or_else(|| bad("select.toggle", "missing id or ids"))?; + let ids = if p.get("ids").is_some() { + checked_ids_param(s, p, "ids", "select.toggle")? + } else { + let value = p.get("id").ok_or_else(|| bad("select.toggle", "missing id or ids"))?; + vec![checked_id(s, value, "select.toggle")?] + }; s.select(|_, sel| { for id in ids { sel.toggle(id); } })?; - ok() + selection_result(s) +} + +fn selection_result(s: &Session) -> Result { + let selection = &s.doc()?.selection; + Ok(json!({ "count": selection.len(), "ids": selection.objects })) } fn key(s: &mut Session, p: &Value) -> Result { diff --git a/crates/engine/tests/selection_commands.rs b/crates/engine/tests/selection_commands.rs new file mode 100644 index 00000000..95753367 --- /dev/null +++ b/crates/engine/tests/selection_commands.rs @@ -0,0 +1,75 @@ +//! Selection command results and validation for agent callers (#784). +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use serde_json::{Value, json}; +use vectorcraft_doc::NodeId; +use vectorcraft_engine::{EngineError, Session}; + +fn session() -> (Session, NodeId, NodeId) { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let mut rectangle = + |x| NodeId(s.execute("shape.rectangle", &json!({"x": x, "y": 0, "width": 10, "height": 10})).unwrap()["id"].as_u64().unwrap()); + let a = rectangle(0); + let b = rectangle(20); + (s, a, b) +} + +#[test] +fn selection_commands_reject_invalid_ids_without_changing_state() { + for command in ["select.set", "select.add", "select.toggle"] { + let (mut s, a, b) = session(); + s.execute("select.set", &json!({"ids": [b.0, a.0]})).unwrap(); + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 0]], "mode": "add"})).unwrap(); + s.execute("select.key", &json!({"id": b.0})).unwrap(); + for invalid in [json!(999), json!(-1), json!(2.5), json!("2"), Value::Null, json!(true), json!(u64::MAX)] { + let selection = s.active().unwrap().selection.clone(); + let revision = s.active().unwrap().revision; + let journal = s.journal.clone(); + let error = s.execute(command, &json!({"ids": [b.0, invalid]})).unwrap_err(); + assert!(matches!(error, EngineError::BadParams { .. }), "{command}: {error}"); + assert!(error.to_string().contains(&invalid.to_string()), "the error must identify {invalid}: {error}"); + assert_eq!(s.active().unwrap().selection, selection, "{command}: {invalid}"); + assert_eq!(s.active().unwrap().revision, revision); + assert_eq!(s.journal, journal); + } + } +} + +#[test] +fn selection_commands_require_an_id_array() { + for command in ["select.set", "select.add", "select.toggle"] { + let (mut s, _, _) = session(); + for params in [json!({}), json!({"ids": null}), json!({"ids": 2}), json!({"ids": "2"}), json!({"ids": true})] { + let selection = s.active().unwrap().selection.clone(); + assert!(matches!(s.execute(command, ¶ms), Err(EngineError::BadParams { .. })), "{command}: {params}"); + assert_eq!(s.active().unwrap().selection, selection); + } + } +} + +#[test] +fn selection_commands_report_the_resulting_selection() { + let (mut s, a, b) = session(); + assert_eq!(s.execute("select.set", &json!({"ids": [b.0, a.0, b.0]})).unwrap(), json!({"count": 2, "ids": [b.0, a.0]})); + assert_eq!(s.execute("select.set", &json!({"ids": []})).unwrap(), json!({"count": 0, "ids": []})); + assert_eq!(s.execute("select.add", &json!({"ids": [a.0, a.0]})).unwrap(), json!({"count": 1, "ids": [a.0]})); + assert_eq!(s.execute("select.add", &json!({"ids": [b.0]})).unwrap(), json!({"count": 2, "ids": [a.0, b.0]})); + assert_eq!(s.execute("select.toggle", &json!({"id": a.0})).unwrap(), json!({"count": 1, "ids": [b.0]})); + assert_eq!(s.execute("select.toggle", &json!({"ids": [b.0, a.0]})).unwrap(), json!({"count": 1, "ids": [a.0]})); + for command in ["select.add", "select.toggle"] { + assert_eq!(s.execute(command, &json!({"ids": []})).unwrap(), json!({"count": 1, "ids": [a.0]})); + } +} + +#[test] +fn single_id_toggle_validates_before_mutating() { + let (mut s, a, _) = session(); + for invalid in [json!(999), json!(-1), json!(2.5), json!("2"), Value::Null, json!(true)] { + let selection = s.active().unwrap().selection.clone(); + assert!(matches!(s.execute("select.toggle", &json!({"id": invalid})), Err(EngineError::BadParams { .. }))); + assert_eq!(s.active().unwrap().selection, selection); + } + // A malformed array must not silently fall back to a valid single id. + assert!(s.execute("select.toggle", &json!({"ids": "bad", "id": a.0})).is_err()); +} diff --git a/docs/mcp.md b/docs/mcp.md index 0b947d11..1de61e6e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -612,6 +612,15 @@ the one highlighted layer or group row, else the one selected group, else the cu so art added later is clipped too); called again it releases the mask. It returns `{clip}`, and the Layers panel underlines clipping-path names. +## Object selection + +`select.set {ids}` replaces the selection, `select.add {ids}` adds to it, and +`select.toggle {id}` or `select.toggle {ids}` toggles each target. +Every id must be a non-negative integer naming an existing object. +A malformed or unknown id is an error naming that value; the entire selection is left unchanged. +All three commands return `{count, ids}` for the resulting selection, in selection order. +An empty array clears the selection for `select.set` and leaves it alone for Add and Toggle. + ## Saved selections Select → Save Selection… keeps the selected objects under a name, in the document: `select.save {name?}` (default