From 98efc6e956c97efd7c6b6bbe206a0d39b64ff940 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 18:14:17 -0800 Subject: [PATCH 1/2] MCP: guide.add and the artboard selections refuse an artboard that is not a whole number guide.add and select.allOnArtboard (`artboard`), select.font (`scope.artboards`) and addon.select.byArtboards (`artboards`) now read artboard indices with the shared artboard helpers. A value that is not a whole number of 0 or more, such as "1", -1, 1.5 or true, fails with an error naming the key, instead of being reported as a missing artboard or read as artboard 0, and the document and selection stay as they were. A whole number past the last artboard is still `no such artboard`, and an omitted or null `artboard` keeps its default. --- crates/engine/src/cmd/addons/select.rs | 12 ++--- crates/engine/src/cmd/addons/select/tests.rs | 29 ++++++++++++ crates/engine/src/cmd/docmenu.rs | 12 ++--- crates/engine/src/cmd/fonts.rs | 45 ++++++++++++++++--- crates/engine/src/cmd/panelcmds.rs | 2 +- crates/engine/src/cmd/select.rs | 5 ++- crates/engine/src/tests/menucmds.rs | 47 ++++++++++++++++++++ docs/mcp.md | 18 +++++--- docs/target-app-parity.md | 1 + 9 files changed, 143 insertions(+), 28 deletions(-) diff --git a/crates/engine/src/cmd/addons/select.rs b/crates/engine/src/cmd/addons/select.rs index b2ab59d851..c9ae29a303 100644 --- a/crates/engine/src/cmd/addons/select.rs +++ b/crates/engine/src/cmd/addons/select.rs @@ -14,8 +14,9 @@ use vectorcraft_doc::{LockedArtboards, Node, NodeId, NodeKind}; use vectorcraft_geom::Point; use super::super::links::{Status, statuses}; +use super::super::panelcmds::listed_artboards; use super::super::select::{is_selectable, on_artboard, selectable}; -use super::super::{CommandSpec, bad, checked_ids_param, cmd, has_doc, point_indices, switch_or, text_param}; +use super::super::{CommandSpec, bad, checked_ids_param, cmd, has_doc, switch_or, text_param}; use crate::{DocState, Result, Session}; /// How Select by Artboards relates an object to the chosen artboards (`relation`). @@ -33,7 +34,7 @@ pub fn specs() -> Vec { "Select by Artboards", ["Object", "Plug-ins", "Selection Tools"], None, - "{artboards: [index…] (0-based, at least one), relation?: \"overlap\" (default: on at least one of them, as select.allOnArtboard takes it: the bounds overlap the artboard or the centre is on it) | \"inside\" (the bounds lie wholly within one of them, edges included) | \"outside\" (on none of them)} select those objects among the ones Select All takes (in isolation mode, the isolated container's); hidden and locked objects, objects on hidden, locked or template layers and art held by a locked artboard are left out. Bounds are geometric, a symbol instance's measured by its art → {count}", + "{artboards: [index…] (0-based whole numbers, at least one), relation?: \"overlap\" (default: on at least one of them, as select.allOnArtboard takes it: the bounds overlap the artboard or the centre is on it) | \"inside\" (the bounds lie wholly within one of them, edges included) | \"outside\" (on none of them)} select those objects among the ones Select All takes (in isolation mode, the isolated container's); hidden and locked objects, objects on hidden, locked or template layers and art held by a locked artboard are left out. Bounds are geometric, a symbol instance's measured by its art → {count}", has_doc, by_artboards ), @@ -58,13 +59,12 @@ fn by_artboards(s: &mut Session, p: &Value) -> Result { return Err(bad(C, "`relation` must be overlap, inside or outside")); } let listed = p.get("artboards").ok_or_else(|| bad(C, "missing `artboards`"))?; - let indices = point_indices(listed).ok_or_else(|| bad(C, "`artboards` must be a list of artboard indices"))?; - if indices.is_empty() { + if listed.as_array().is_some_and(Vec::is_empty) { return Err(bad(C, "choose at least one artboard")); } let st = s.doc()?; - let rects = - indices.iter().map(|i| st.doc.artboards.get(*i).map(|a| a.rect)).collect::>>().ok_or_else(|| bad(C, "no such artboard"))?; + let rects: Vec<_> = + listed_artboards(&st.doc, listed, "artboards", C)?.into_iter().filter_map(|i| st.doc.artboards.get(i)).map(|a| a.rect).collect(); let symbols = st.doc.symbol_boxes(); let ids: Vec = selectable(&st.doc, st.isolation) .into_iter() diff --git a/crates/engine/src/cmd/addons/select/tests.rs b/crates/engine/src/cmd/addons/select/tests.rs index c97b278258..09106a3652 100644 --- a/crates/engine/src/cmd/addons/select/tests.rs +++ b/crates/engine/src/cmd/addons/select/tests.rs @@ -108,6 +108,35 @@ fn bad_params_are_errors_and_leave_the_selection() { } } +/// A malformed `artboards` entry is refused as one, naming the parameter, before any entry is +/// looked up; a value that is not a list, an empty list, a left-out list and a whole number past +/// the last artboard each have their own error. +#[test] +fn artboard_errors_say_what_is_wrong() { + let mut s = three_boards(); + let a = rect(&mut s, 10.0, 10.0, 20.0, 20.0); + let entries = "invalid parameters for `addon.select.byArtboards`: `artboards` must contain whole numbers"; + let not_a_list = "invalid parameters for `addon.select.byArtboards`: `artboards` must be a list of artboard indices"; + for (p, want) in [ + (json!({"artboards": ["1"]}), entries), + (json!({"artboards": [-1]}), entries), + (json!({"artboards": [1.5]}), entries), + (json!({"artboards": [true]}), entries), + (json!({"artboards": [9, "1"]}), entries), + (json!({"artboards": "0"}), not_a_list), + (json!({"artboards": null}), not_a_list), + (json!({"artboards": []}), "invalid parameters for `addon.select.byArtboards`: choose at least one artboard"), + (json!({}), "invalid parameters for `addon.select.byArtboards`: missing `artboards`"), + (json!({"artboards": [9]}), "no such artboard"), + (json!({"artboards": [0, 9]}), "no such artboard"), + ] { + s.execute("select.set", &json!({"ids": [a.0]})).unwrap(); + assert_eq!(s.execute(C, &p).unwrap_err().to_string(), want, "{p}"); + assert_eq!(s.doc().unwrap().selection.objects, vec![a], "{p}"); + } + assert_eq!(pick(&mut s, json!({"artboards": [0, 0]})), vec![a]); +} + #[test] fn a_symbol_instance_is_measured_by_its_art() { // The symbol's art, (100, 60) to (140, 80), drawn 50 pt up and left: (50, 10) to (90, 30), on diff --git a/crates/engine/src/cmd/docmenu.rs b/crates/engine/src/cmd/docmenu.rs index ae48ba02e6..3b1e8a7666 100644 --- a/crates/engine/src/cmd/docmenu.rs +++ b/crates/engine/src/cmd/docmenu.rs @@ -10,6 +10,7 @@ use vectorcraft_doc::{CharStyle, Guide, Node, NodeId, NodeKind, SavedSelection, use vectorcraft_geom::Vec2; use super::edit::selected_roots; +use super::panelcmds::artboard_at; use super::pathops::Beside; use super::select::{first_selectable, is_selectable, selectable_ids, walk_selectable}; use super::{ @@ -201,7 +202,7 @@ pub fn specs() -> Vec { "Add Guide", [], None, - "{vertical: bool, pos: pt (x for vertical, y for horizontal), artboard?: index (an artboard guide: it runs across that artboard only and moves, is copied and is deleted with it; default a canvas guide, across the whole canvas), layer?: id (the layer it is on, shown, hidden, locked and deleted with it; default the current layer)} → {index}", + "{vertical: bool, pos: pt (x for vertical, y for horizontal), artboard?: index (0-based whole number; an artboard guide: it runs across that artboard only and moves, is copied and is deleted with it; default, or null, a canvas guide, across the whole canvas), layer?: id (the layer it is on, shown, hidden, locked and deleted with it; default the current layer)} → {index}", has_doc, guide_add ), @@ -767,15 +768,8 @@ fn clear_guides(s: &mut Session, _: &Value) -> Result { fn guide_add(s: &mut Session, p: &Value) -> Result { let pos = f64_req(p, "pos", "guide.add")?; let vertical = switch_or(p, "vertical", false, "guide.add")?; - let artboard = match p.get("artboard").filter(|v| !v.is_null()) { - Some(v) => { - let boards = &s.doc()?.doc.artboards; - let ab = v.as_u64().and_then(|i| boards.get(usize::try_from(i).ok()?)).ok_or_else(|| EngineError::Other("no such artboard".into()))?; - Some(ab.id) - } - None => None, - }; let st = s.doc()?; + let artboard = artboard_at(&st.doc, p, "artboard", "guide.add")?.and_then(|i| st.doc.artboards.get(i)).map(|a| a.id); let layer = match id_param(p, "layer") { Some(l) if st.doc.node(l).is_some_and(Node::is_layer) => Some(l), Some(_) => return Err(bad("guide.add", "layer must name a layer or sublayer")), diff --git a/crates/engine/src/cmd/fonts.rs b/crates/engine/src/cmd/fonts.rs index 7c8f664cf3..28662f013c 100644 --- a/crates/engine/src/cmd/fonts.rs +++ b/crates/engine/src/cmd/fonts.rs @@ -10,9 +10,10 @@ use vectorcraft_color::Paint; use vectorcraft_doc::{AppearanceItem, Document, Node, NodeId, NodeKind, TextKind, TextObject, TextRun}; use vectorcraft_geom::Rect; +use super::panelcmds::listed_artboards; use super::select::on_artboard; use super::typecmd::refresh_bounds; -use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, has_doc, index, str_param}; +use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, has_doc, str_param}; pub fn specs() -> Vec { vec![ @@ -39,7 +40,7 @@ pub fn specs() -> Vec { "Select Text by Font", [], None, - "{family, style?, scope?: \"document\" (default) | {artboards: [index, …] (0-based)} | {layers: [id, …] (layers or sublayers)}} select the text objects using the font, the instances of the symbols whose art uses it, the text whose inline graphics draw such a symbol, the objects painted with a pattern whose art uses it (by a visible fill or stroke whose opacity is above 0, a stroke wider than 0) and the objects whose opacity mask uses it (in type that is not hidden or locked in that art), that are not hidden or locked, in a hidden or locked group or layer, or on a locked artboard; a pattern swatch that nothing paints, or a layer's mask, selects nothing; with scope, only those on one of the artboards (as All on Active Artboard takes them: their bounds overlap it or their center is on it) or in one of the layers → {count}", + "{family, style?, scope?: \"document\" (default) | {artboards: [index, …] (0-based whole numbers)} | {layers: [id, …] (layers or sublayers)}} select the text objects using the font, the instances of the symbols whose art uses it, the text whose inline graphics draw such a symbol, the objects painted with a pattern whose art uses it (by a visible fill or stroke whose opacity is above 0, a stroke wider than 0) and the objects whose opacity mask uses it (in type that is not hidden or locked in that art), that are not hidden or locked, in a hidden or locked group or layer, or on a locked artboard; a pattern swatch that nothing paints, or a layer's mask, selects nothing; with scope, only those on one of the artboards (as All on Active Artboard takes them: their bounds overlap it or their center is on it) or in one of the layers → {count}", has_doc, select_font ), @@ -1080,9 +1081,9 @@ impl FontScope { Some(_) => Err(bad(C, format!("`scope.{key}` must be a non-empty array"))), } }; - if let Some(a) = list("artboards")? { - let rect = |v: &Value| v.as_u64().and_then(|i| d.artboards.get(index(i))).map(|b| b.rect); - return a.iter().map(|v| rect(v).ok_or_else(|| bad(C, format!("no such artboard {v}")))).collect::>().map(Self::Artboards); + if let (Some(v), Some(_)) = (scope.get("artboards"), list("artboards")?) { + let indices = listed_artboards(d, v, "scope.artboards", C)?; + return Ok(Self::Artboards(indices.into_iter().filter_map(|i| d.artboards.get(i)).map(|b| b.rect).collect())); } if let Some(a) = list("layers")? { let layer = |v: &Value| v.as_u64().map(NodeId).filter(|id| d.node(*id).is_some_and(Node::is_layer)); @@ -1292,6 +1293,40 @@ mod tests { } } + /// Select Text by Font refuses a `scope.artboards` entry that is not a whole number, naming the + /// parameter, before it looks at the artboards, and leaves the selection as it was; a whole + /// number past the last artboard is `no such artboard`. + #[test] + fn select_font_refuses_artboards_that_are_not_whole_numbers() { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + s.execute("artboard.new", &json!({"x": 300, "y": 0, "width": 200, "height": 200})).unwrap(); + let text = |s: &mut Session, x: i32| { + NodeId(s.execute("text.create", &json!({"x": x, "y": 50, "text": "Serif", "font": "Source Serif 4"})).unwrap()["id"].as_u64().unwrap()) + }; + let first = text(&mut s, 10); + let second = text(&mut s, 350); + let run = + |s: &mut Session, artboards: Value| s.execute("select.font", &json!({"family": "Source Serif 4", "scope": {"artboards": artboards}})); + let malformed = "invalid parameters for `select.font`: `scope.artboards` must contain whole numbers"; + for (artboards, want) in [ + (json!(["1"]), malformed), + (json!([-1]), malformed), + (json!([1.5]), malformed), + (json!([true]), malformed), + (json!([null]), malformed), + (json!([9, "1"]), malformed), + (json!([9]), "no such artboard"), + (json!([1, 9]), "no such artboard"), + ] { + s.execute("select.set", &json!({"ids": [first.0]})).unwrap(); + assert_eq!(run(&mut s, artboards.clone()).unwrap_err().to_string(), want, "{artboards}"); + assert_eq!(s.doc().unwrap().selection.objects, [first], "{artboards}"); + } + run(&mut s, json!([1, 1])).unwrap(); + assert_eq!(s.doc().unwrap().selection.objects, [second]); + } + /// Select Text by Font with a layer scope takes the text in the layers, their sublayers /// included, and still leaves out hidden and locked text. #[test] diff --git a/crates/engine/src/cmd/panelcmds.rs b/crates/engine/src/cmd/panelcmds.rs index fa0492c2db..e10875e742 100644 --- a/crates/engine/src/cmd/panelcmds.rs +++ b/crates/engine/src/cmd/panelcmds.rs @@ -255,7 +255,7 @@ pub(crate) fn artboard_indices(d: &vectorcraft_doc::Document, p: &Value, cmd: &s /// list of artboard indices", and a list with an entry that is not a whole number is "`{key}` must /// contain whole numbers"; a whole number past the last artboard is `no such artboard`. Every entry /// is checked for its form before any is checked against the artboards. -fn listed_artboards(d: &vectorcraft_doc::Document, v: &Value, key: &str, cmd: &str) -> Result> { +pub(crate) fn listed_artboards(d: &vectorcraft_doc::Document, v: &Value, key: &str, cmd: &str) -> Result> { let entries = v.as_array().ok_or_else(|| bad(cmd, format!("`{key}` must be a list of artboard indices")))?; if entries.is_empty() { return Err(bad(cmd, format!("`{key}` must be a non-empty list of artboard indices"))); diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index ac9ca365f4..1675fcc87c 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -8,6 +8,7 @@ use vectorcraft_doc::{Document, LockedArtboards, Node, NodeId, NodeKind}; use vectorcraft_tools::builder::is_live_paint; use super::object::can_align; +use super::panelcmds::artboard_at; use super::wand::{fill_of, stroke_of}; use super::{ CommandSpec, Result, Session, bad, bool_or, checked_id, checked_ids_param, cmd, color_value, has_doc, has_selection, index, ok, str_param, @@ -31,7 +32,7 @@ pub fn specs() -> Vec { "All on Active Artboard", ["Select"], Some("Cmd+Alt+A"), - "{artboard?: index (0-based; the app passes the active one; default: the first)}", + "{artboard?: index (0-based whole number; the app passes the active one; default, or null: the first)}", has_doc, all_on_artboard ), @@ -360,7 +361,7 @@ fn all(s: &mut Session, _: &Value) -> Result { fn all_on_artboard(s: &mut Session, p: &Value) -> Result { let st = s.doc()?; - let i = p.get("artboard").and_then(Value::as_u64).map_or(0, index); + let i = artboard_at(&st.doc, p, "artboard", "select.allOnArtboard")?.unwrap_or(0); let r = st.doc.artboards.get(i).map(|a| a.rect).ok_or_else(|| bad("select.allOnArtboard", "no such artboard"))?; let symbols = st.doc.symbol_boxes(); let ids: Vec = diff --git a/crates/engine/src/tests/menucmds.rs b/crates/engine/src/tests/menucmds.rs index 1151f83910..493b4d58e2 100644 --- a/crates/engine/src/tests/menucmds.rs +++ b/crates/engine/src/tests/menucmds.rs @@ -3232,6 +3232,32 @@ fn select_all_on_artboard_refuses_an_artboard_past_u32() { assert!(selected(&s).is_empty()); } +/// All on Active Artboard refuses an `artboard` that is not a whole number, naming the parameter, +/// and leaves the selection as it was; a whole number past the last artboard is `no such +/// artboard`. Left out or null, it takes the first artboard. +#[test] +fn select_all_on_artboard_refuses_an_artboard_that_is_not_a_whole_number() { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 100, "height": 100, "artboards": 2, "artboardLayout": {"layout": "row", "spacing": 0}})).unwrap(); + let (first, second) = (rect(&mut s, 10.0, 10.0, 20.0, 20.0), rect(&mut s, 110.0, 10.0, 20.0, 20.0)); + for bad in [json!("1"), json!(-1), json!(1.5), json!(true), json!([1]), json!({})] { + s.execute("select.none", &json!({})).unwrap(); + let err = s.execute("select.allOnArtboard", &json!({"artboard": bad})).unwrap_err().to_string(); + assert_eq!(err, "invalid parameters for `select.allOnArtboard`: `artboard` must be a whole number", "{bad}"); + assert!(selected(&s).is_empty(), "{bad}"); + } + s.execute("select.none", &json!({})).unwrap(); + assert_eq!(s.execute("select.allOnArtboard", &json!({"artboard": 9})).unwrap_err().to_string(), "no such artboard"); + assert!(selected(&s).is_empty()); + for p in [json!({}), json!({"artboard": null}), json!({"artboard": 0})] { + s.execute("select.none", &json!({})).unwrap(); + s.execute("select.allOnArtboard", &p).unwrap(); + assert_eq!(selected(&s), vec![first], "{p}"); + } + s.execute("select.allOnArtboard", &json!({"artboard": 1})).unwrap(); + assert_eq!(selected(&s), vec![second]); +} + #[test] fn save_and_recall_selection() { let mut s = session(); @@ -4538,6 +4564,27 @@ fn artboard_guides_go_with_their_artboard() { assert_eq!((guides(&s).len(), selected_guides(&s)), (3, vec![0, 1])); } +/// `guide.add` refuses an `artboard` that is not a whole number, naming the parameter, and adds no +/// guide; a whole number past the last artboard is `no such artboard`. Null makes a canvas guide, +/// and a whole number an artboard guide. +#[test] +fn guide_add_refuses_an_artboard_that_is_not_a_whole_number() { + let mut s = session(); + s.execute("artboard.new", &json!({"x": 900, "y": 0, "width": 400, "height": 300})).unwrap(); + let add = |s: &mut Session, artboard: Value| s.execute("guide.add", &json!({"vertical": true, "pos": 10, "artboard": artboard})); + for bad in [json!("1"), json!(-1), json!(1.5), json!(true), json!([1]), json!({})] { + let err = add(&mut s, bad.clone()).unwrap_err().to_string(); + assert_eq!(err, "invalid parameters for `guide.add`: `artboard` must be a whole number", "{bad}"); + } + assert_eq!(add(&mut s, json!(9)).unwrap_err().to_string(), "no such artboard"); + assert!(s.doc().unwrap().doc.ruler_guides().is_empty()); + add(&mut s, Value::Null).unwrap(); + add(&mut s, json!(1)).unwrap(); + let d = &s.doc().unwrap().doc; + let boards: Vec<_> = d.ruler_guides().iter().map(|g| g.artboard).collect(); + assert_eq!(boards, [None, Some(d.artboards[1].id)]); +} + /// #451: a guide dragged out of a ruler snaps (to the art's corners, side midpoints and anchors, /// the artboards' edges and centres) and is made in one undo step where it is released over the /// canvas; with the Artboard tool it is an artboard guide of the active artboard. diff --git a/docs/mcp.md b/docs/mcp.md index d4e03892e9..20e688c006 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -3077,7 +3077,9 @@ number. `guide.add {vertical, pos, artboard?}` makes one (the x of a vertical guide, the y of a horizontal one, in points) → `{index}`. With `artboard` (an artboard's index) it is an artboard guide: it runs across that artboard only, moves with it (`artboard.move`, a pure move in `artboard.setProps`, `artboard.rearrange`), scales with it (`artboard.setProps {scaleArt}`), is copied with it (`artboard.duplicate`, `artboard.move {copy}`) and is -deleted with it (`artboard.delete`); without, a canvas guide runs across the whole canvas. `guide.list` → +deleted with it (`artboard.delete`); without, or with null, a canvas guide runs across the whole canvas. An `artboard` +that is not a whole number of 0 or more fails with "`artboard` must be a whole number", and an index past the last +artboard with `no such artboard`; neither adds a guide. `guide.list` → `[{index, id, vertical, pos, selected, artboard?, layer?, hidden, locked, shown, editable}…]` (`artboard` only for artboard guides; `id` is the guide's node and `layer` the layer it is on; `hidden` and `locked` are its own state, `shown` and `editable` also take its layer into account). `layer.setProps {ids: [id…], visible?, locked?}` hides or @@ -5860,8 +5862,12 @@ below 3 or above 1000 counts as 3 or 1000, and a fractional or negative `sides` Without `artboard`, or with an index the document doesn't have, it aligns them to the artboard under the center of the selection, or to the first artboard when no artboard is under the center. With `to: "selection"` or `"key"`, `object.align` does not use `artboard`. `select.allOnArtboard {artboard?}` (Select › All on Active Artboard) selects -the objects on artboard `artboard`, or on the first artboard when the call gives none; an index the document doesn't -have is an error. +the objects on artboard `artboard`, or on the first artboard when the call gives none or null. A value that is not a +whole number of 0 or more fails with "`artboard` must be a whole number", and an index the document doesn't have with +`no such artboard`; either leaves the selection as it was. `select.font {family, style?, scope?}` (Select Text +by Font) takes `scope: {artboards: [index…]}` the same way: an entry that is not a whole number fails with +"`scope.artboards` must contain whole numbers" before any entry is looked up, and an index the document doesn't have +fails with `no such artboard`. `object.distribute {to: "artboard", artboard?}` and `object.distributeSpacing {to: "artboard", artboard?}` (the Distribute Objects and Distribute Spacing buttons with Align to Artboard) use the same artboard as `object.align`. @@ -11749,8 +11755,10 @@ overlapping the artboard or the centre on it), `inside` (the bounds wholly withi straddling two touching artboards is inside neither) or `outside` (on none of them). It looks at the objects Select All takes, or the isolated container's in isolation mode, so hidden and locked objects, objects on hidden, locked or template layers and art held by a locked artboard are left out. Bounds are geometric, a symbol instance's measured by its art. It -returns the `count` selected. An empty list, an index with no artboard, a value that isn't an index, or another -relation is an error, and the selection is left as it was. +returns the `count` selected. An empty list, an index with no artboard (`no such artboard`), a value that isn't a list +("`artboards` must be a list of artboard indices"), an entry that isn't a whole number of 0 or more ("`artboards` must +contain whole numbers", checked before any entry is looked up), or another relation is an error, and the selection is +left as it was. ```json {"name":"run_command","arguments":{"command":"addon.select.byArtboards","params":{"artboards":[2,6,11],"relation":"inside"}}} diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index cad5851e98..1e6e7a0f6b 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1155,6 +1155,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur `path.reshapeSegment`, `object.blend.spine.reshapeSegment`, `path.convertAnchor`, `path.curvatureEdit`, `path.insertAnchor`, `path.moveAnchors`, `path.reshape`, `path.close` and the Pencil, Curvature, Knife, Eraser, Path Eraser, Smooth, Join and Blob Brush commands refuse a number, point or handle given as text, such as "10", and leave the document unchanged; `path.convertAnchor` refuses an `x` without a `y`, or a `y` without an `x`. `path.setHandle`, `path.moveAnchor`, `text.setPathHandle` (`anchor`), `path.insertAnchor` (`segment`), `artboard.reorder`, `library.moveGroup`, `graphicStyle.move` (`to`), `path.appendAnchor`, `path.close`, `path.reverse` (`subpath`) and the ruler origin commands (`artboard`) refuse a value that is not a whole number of 0 or more, such as `"3"` or `2.5`, with `` `anchor` must be a whole number ``, naming the key, instead of reporting it as missing or reading it as the default subpath or artboard; a null `artboard` is artboard 0 (`tests::params::a_malformed_index_is_not_missing_or_its_default`). The artboard commands that take `index`, `indices` or a replayed `selectedArtboards`, the artboard Align and Distribute commands (`artboard`, `activeArtboard`), `edit.newDocumentFromState` (`index`) and Paste in Place, in Front and in Back (`artboard`, `artboards`) report a value that is not a whole number as such, naming the key, and keep `no such artboard`, `no history state N` and the paste range messages for a whole number that names nothing; a null artboard `index` is `missing index`, and an `indices` value that is not a list, an empty list and a malformed entry each have their own message (`tests::panelcmds::artboard_lists_name_the_fault_and_the_key`). + `guide.add` and `select.allOnArtboard` (`artboard`), `select.font` (`scope.artboards`) and `addon.select.byArtboards` (`artboards`) refuse an artboard that is not a whole number of 0 or more, such as `"1"`, `-1`, `1.5` or `true`, naming the key, instead of reporting it as a missing artboard or reading it as artboard 0, and leave the document and selection unchanged; a whole number past the last artboard is still `no such artboard`, and a null or left-out `artboard` keeps its default (`tests::menucmds::guide_add_refuses_an_artboard_that_is_not_a_whole_number`). `path.close` and `path.setHandle` (`independent`), `path.moveAnchors` (`patterns`), the Pencil (`closed`, `keepSelected`, `fill`), Curvature (`closed` and a point's `corner`), `path.curvatureEdit` (`corner`), `path.reshapeSegment` (`perpendicular`) and the Blob Brush (`keepSelected`, `mergeOnlySelection`, `merge`) refuse a value other than true, false or null, such as "true", and leave the document unchanged. Direct Selection's marquee selects the frame anchors of area type that lie inside it, and a click on a frame anchor selects that one; Shift toggles them. The selected frame anchors show filled. A drag of one of them moves all of them, the arrow keys nudge them, and the type reflows at its size (`text.reshapeArea`, which takes `items` to move the anchors of several frames in one step), as [Create text](http://web.archive.org/web/20220127090642/https://helpx.adobe.com/illustrator/using/creating-text.html) and [Selecting objects](http://web.archive.org/web/20211230015528/https://helpx.adobe.com/illustrator/using/selecting-objects.html) describe. When the selection also holds other objects, path anchors or ruler guides, or when another change to the document or the selection follows the pick, the type is selected whole. A press on a selected object's centre point (Attributes panel › Show Center) selects the object whole, and a drag from it moves the object as one Move, held by that point (#4891; test `the_centre_point_selects_and_moves_the_object_whole`). From 65ec6970ab3e11003cabfbfd97d35a15a46a7a14 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 20:07:36 -0800 Subject: [PATCH 2/2] MCP: select.same.appearanceAttribute refuses an item that is not a whole number An `item` given as text, a negative or fractional number, a boolean or a list was read as left out, so the command matched the Appearance panel's active item instead. It now fails with "`item` must be a whole number" and leaves the selection as it was; a left-out or null `item` still uses the active item. --- crates/engine/src/cmd/select.rs | 6 +++--- crates/engine/src/tests/styles.rs | 20 ++++++++++++++++++++ docs/mcp.md | 5 ++++- docs/target-app-parity.md | 2 +- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 1675fcc87c..7754ce9346 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -11,7 +11,7 @@ use super::object::can_align; use super::panelcmds::artboard_at; use super::wand::{fill_of, stroke_of}; use super::{ - CommandSpec, Result, Session, bad, bool_or, checked_id, checked_ids_param, cmd, color_value, has_doc, has_selection, index, ok, str_param, + CommandSpec, Result, Session, bad, bool_or, checked_id, checked_ids_param, cmd, color_value, has_doc, has_selection, index_opt, ok, str_param, switch_or, text_param, }; use crate::DocState; @@ -222,7 +222,7 @@ pub fn specs() -> Vec { "Appearance Attribute", ["Select", "Same"], None, - "{item?: appearance item index} the objects sharing an appearance attribute of the first selected object: its fill or stroke `item` (default: the Appearance panel's active item), else its first effect, else its topmost fill (compound paths as select.same.fillColor)", + "{item?: appearance item index (0-based whole number; default, or null: the Appearance panel's active item)} the objects sharing an appearance attribute of the first selected object: its fill or stroke `item` (default: the Appearance panel's active item), else its first effect, else its topmost fill (compound paths as select.same.fillColor)", has_selection, same_attribute ), @@ -1103,10 +1103,10 @@ enum Attribute { fn same_attribute(s: &mut Session, p: &Value) -> Result { let cmd = "select.same.appearanceAttribute"; + let item = index_opt(p, "item", cmd)?.or(s.appearance_item()); let st = s.doc()?; let (refn, _) = same_reference(st, cmd)?; let ap = &refn.appearance; - let item = p.get("item").and_then(Value::as_u64).map(index).or(s.appearance_item()); let attr = match (item.and_then(|i| ap.items.get(i)), ap.effects.first()) { (Some(it), _) => Attribute::Item(it.clone()), (None, Some(e)) => Attribute::Effect(e.clone()), diff --git a/crates/engine/src/tests/styles.rs b/crates/engine/src/tests/styles.rs index 6f77b58971..e460aeb890 100644 --- a/crates/engine/src/tests/styles.rs +++ b/crates/engine/src/tests/styles.rs @@ -325,6 +325,26 @@ fn select_same_appearance_attribute() { run(&mut s, "select.set", json!({"ids": [a.0]})); run(&mut s, "select.same.appearanceAttribute", json!({"item": (1u64 << 32) + 1})); assert_eq!(s.doc().unwrap().selection.objects, vec![a, b]); + // Null is the default: the shared effect. + run(&mut s, "select.set", json!({"ids": [a.0]})); + run(&mut s, "select.same.appearanceAttribute", json!({"item": null})); + assert_eq!(s.doc().unwrap().selection.objects, vec![a, b]); +} + +/// Select > Same > Appearance Attribute refuses an `item` that is not a whole number and leaves +/// the selection as it was. +#[test] +fn select_same_appearance_attribute_refuses_an_item_that_is_not_a_whole_number() { + let mut s = session(); + let a = rect(&mut s, 10.0); + run(&mut s, "effect.apply", json!({"effect": "distort.roughen"})); + rect(&mut s, 100.0); + run(&mut s, "select.set", json!({"ids": [a.0]})); + for item in [json!("1"), json!(-1), json!(1.5), json!(true), json!([1])] { + let e = s.execute("select.same.appearanceAttribute", &json!({"item": item})).unwrap_err().to_string(); + assert_eq!(e, "invalid parameters for `select.same.appearanceAttribute`: `item` must be a whole number", "{item}"); + assert_eq!(s.doc().unwrap().selection.objects, vec![a], "{item}"); + } } /// With a member path of a compound path selected first, Select > Same > Graphic Style and diff --git a/docs/mcp.md b/docs/mcp.md index b001f720d4..ec350e3a97 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -2785,7 +2785,10 @@ group selected on its own, or, for `symbolInstance`, an object that is not a sym leaves the selection empty. A selected object that is hidden or locked, inside a hidden or locked group or layer, on a template layer, on a locked artboard while nothing is isolated, or outside the isolated group is passed over. New art, such as a symbol instance just placed, is not passed over for being on a locked artboard while it stays selected. When -the selection holds no other object, they return the error "nothing selected". +the selection holds no other object, they return the error "nothing selected". `appearanceAttribute {item?}` takes +the fill or stroke at appearance item `item`, or the Appearance panel's active item when the call gives none or null; a +value that is not a whole number of 0 or more fails with "`item` must be a whole number" and leaves the selection as it +was. `select.object.allOnSameLayers` selects the objects on the layers and sublayers of the selected objects that can be selected. A selected object that is hidden or locked, inside a hidden or locked group or layer, on a template layer, on diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 58096f67b1..d2e7c9fadc 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1158,7 +1158,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur `path.reshapeSegment`, `object.blend.spine.reshapeSegment`, `path.convertAnchor`, `path.curvatureEdit`, `path.insertAnchor`, `path.moveAnchors`, `path.reshape`, `path.close` and the Pencil, Curvature, Knife, Eraser, Path Eraser, Smooth, Join and Blob Brush commands refuse a number, point or handle given as text, such as "10", and leave the document unchanged; `path.convertAnchor` refuses an `x` without a `y`, or a `y` without an `x`. `path.setHandle`, `path.moveAnchor`, `text.setPathHandle` (`anchor`), `path.insertAnchor` (`segment`), `artboard.reorder`, `library.moveGroup`, `graphicStyle.move` (`to`), `path.appendAnchor`, `path.close`, `path.reverse` (`subpath`) and the ruler origin commands (`artboard`) refuse a value that is not a whole number of 0 or more, such as `"3"` or `2.5`, with `` `anchor` must be a whole number ``, naming the key, instead of reporting it as missing or reading it as the default subpath or artboard; a null `artboard` is artboard 0 (`tests::params::a_malformed_index_is_not_missing_or_its_default`). The artboard commands that take `index`, `indices` or a replayed `selectedArtboards`, the artboard Align and Distribute commands (`artboard`, `activeArtboard`), `edit.newDocumentFromState` (`index`) and Paste in Place, in Front and in Back (`artboard`, `artboards`) report a value that is not a whole number as such, naming the key, and keep `no such artboard`, `no history state N` and the paste range messages for a whole number that names nothing; a null artboard `index` is `missing index`, and an `indices` value that is not a list, an empty list and a malformed entry each have their own message (`tests::panelcmds::artboard_lists_name_the_fault_and_the_key`). - `guide.add` and `select.allOnArtboard` (`artboard`), `select.font` (`scope.artboards`) and `addon.select.byArtboards` (`artboards`) refuse an artboard that is not a whole number of 0 or more, such as `"1"`, `-1`, `1.5` or `true`, naming the key, instead of reporting it as a missing artboard or reading it as artboard 0, and leave the document and selection unchanged; a whole number past the last artboard is still `no such artboard`, and a null or left-out `artboard` keeps its default (`tests::menucmds::guide_add_refuses_an_artboard_that_is_not_a_whole_number`). + `guide.add` and `select.allOnArtboard` (`artboard`), `select.font` (`scope.artboards`) and `addon.select.byArtboards` (`artboards`) refuse an artboard that is not a whole number of 0 or more, such as `"1"`, `-1`, `1.5` or `true`, naming the key, instead of reporting it as a missing artboard or reading it as artboard 0, and leave the document and selection unchanged; a whole number past the last artboard is still `no such artboard`, and a null or left-out `artboard` keeps its default (`tests::menucmds::guide_add_refuses_an_artboard_that_is_not_a_whole_number`). `select.same.appearanceAttribute` refuses such an `item` the same way instead of using the Appearance panel's active item (`tests::styles::select_same_appearance_attribute_refuses_an_item_that_is_not_a_whole_number`). `path.close` and `path.setHandle` (`independent`), `path.moveAnchors` (`patterns`), the Pencil (`closed`, `keepSelected`, `fill`), Curvature (`closed` and a point's `corner`), `path.curvatureEdit` (`corner`), `path.reshapeSegment` (`perpendicular`) and the Blob Brush (`keepSelected`, `mergeOnlySelection`, `merge`) refuse a value other than true, false or null, such as "true", and leave the document unchanged. Direct Selection's marquee selects the frame anchors of area type that lie inside it, and a click on a frame anchor selects that one; Shift toggles them. The selected frame anchors show filled. A drag of one of them moves all of them, the arrow keys nudge them, and the type reflows at its size (`text.reshapeArea`, which takes `items` to move the anchors of several frames in one step), as [Create text](http://web.archive.org/web/20220127090642/https://helpx.adobe.com/illustrator/using/creating-text.html) and [Selecting objects](http://web.archive.org/web/20211230015528/https://helpx.adobe.com/illustrator/using/selecting-objects.html) describe. When the selection also holds other objects, path anchors or ruler guides, or when another change to the document or the selection follows the pick, the type is selected whole. A press on a selected object's centre point (Attributes panel › Show Center) selects the object whole, and a drag from it moves the object as one Move, held by that point (#4891; test `the_centre_point_selects_and_moves_the_object_whole`).