From ee6158360607a1d7424c926fc38f61e0bbcd6d17 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 16 Oct 2026 15:42:09 -0700 Subject: [PATCH] Artboards: commands given an index that names no artboard fail instead of editing another one artboard.setProps and artboard.fitToSelection read an `index` that is negative, fractional, a string or null as 0 and edited the first artboard, and artboard.setProps edited the first artboard when the call named no `index`. artboard.duplicate, artboard.copy and artboard.cut took the Artboard tool's artboard for such an `index`. artboard.reorder failed for it with "missing index". These commands now read `index` as artboard.delete, artboard.move and artboard.fitToArt do, and fail with "no such artboard" when it names no artboard; the document and the clipboard stay as they were. artboard.setProps without `index` fails with "missing index". Without `index`, artboard.fitToSelection still fits the first artboard and the copy commands still take the Artboard tool's artboard. Docs: docs/mcp.md. --- crates/engine/src/cmd/layer.rs | 4 ++-- crates/engine/src/cmd/panelcmds.rs | 23 +++++++++++++++++------ crates/engine/src/tests_menucmds.rs | 25 +++++++++++++++++++++++++ crates/engine/src/tests_panelcmds.rs | 21 +++++++++++++++++++++ docs/mcp.md | 4 ++++ 5 files changed, 69 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 9d398d6a1..8ff970171 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -834,7 +834,7 @@ fn artboard_delete(s: &mut Session, p: &Value) -> Result { } fn artboard_set(s: &mut Session, p: &Value) -> Result { - let i = p.get("index").and_then(Value::as_u64).unwrap_or(0) as usize; + let i = super::panelcmds::artboard_index(&s.doc()?.doc, p, "artboard.setProps")?; let was = s.doc()?.doc.artboards.get(i).map(|a| a.rect).ok_or_else(|| EngineError::Other("no such artboard".into()))?; let now = rect_from(p, was); let resized = (now.width() - was.width()).abs() > 1e-6 || (now.height() - was.height()).abs() > 1e-6; @@ -933,8 +933,8 @@ fn artboard_fit_art(s: &mut Session, p: &Value) -> Result { } fn artboard_fit_sel(s: &mut Session, p: &Value) -> Result { - let i = p.get("index").and_then(Value::as_u64).unwrap_or(0) as usize; let st = s.doc()?; + let i = if p.get("index").is_none() { 0 } else { super::panelcmds::artboard_index(&st.doc, p, "artboard.fitToSelection")? }; let b = st.doc.bounds_of(&st.selection.objects, true).ok_or_else(|| EngineError::Other("no selection bounds".into()))?; s.execute("artboard.setProps", &json!({"index": i, "x": b.x0, "y": b.y0, "width": b.width(), "height": b.height()})) } diff --git a/crates/engine/src/cmd/panelcmds.rs b/crates/engine/src/cmd/panelcmds.rs index 968444277..9b55ff8b1 100644 --- a/crates/engine/src/cmd/panelcmds.rs +++ b/crates/engine/src/cmd/panelcmds.rs @@ -54,7 +54,7 @@ pub fn specs() -> Vec { fn artboard_reorder(s: &mut Session, p: &Value) -> Result { const C: &str = "artboard.reorder"; - let i = p.get("index").and_then(Value::as_u64).ok_or_else(|| bad(C, "missing index"))? as usize; + let i = artboard_index(&s.doc()?.doc, p, C)?; let to = p.get("to").and_then(Value::as_u64).ok_or_else(|| bad(C, "missing to"))? as usize; s.edit("Reorder Artboards", |d, _| { if i >= d.artboards.len() { @@ -70,10 +70,21 @@ fn artboard_reorder(s: &mut Session, p: &Value) -> Result { /// The artboard (`index`) and whether its art comes along (`art`) for the artboard copy /// commands: by default the Artboard tool's artboard and its Move/Copy Artwork with Artboard option. -fn board_and_art(s: &Session, p: &Value) -> (usize, bool) { +/// An `index` that names no artboard is an error. +fn board_and_art(s: &Session, p: &Value, cmd: &str) -> Result<(usize, bool)> { let t = s.tool_options_of("artboard"); - let i = p.get("index").and_then(Value::as_u64).or_else(|| t["active"].as_u64()).unwrap_or(0) as usize; - (i, bool_or(p, "art", t["moveArt"].as_bool().unwrap_or(true))) + let i = match p.get("index") { + Some(_) => artboard_index(&s.doc()?.doc, p, cmd)?, + None => t["active"].as_u64().unwrap_or(0) as usize, + }; + Ok((i, bool_or(p, "art", t["moveArt"].as_bool().unwrap_or(true)))) +} + +/// The one artboard `cmd` acts on: `index`, an artboard of `d`, with the errors of +/// [`artboard_indices`] (`missing index`, `no such artboard`). +pub(crate) fn artboard_index(d: &vectorcraft_doc::Document, p: &Value, cmd: &str) -> Result { + let i = p.get("index").ok_or_else(|| bad(cmd, "missing index"))?; + artboard_indices(d, &json!({ "index": i }), cmd)?.into_iter().next().ok_or_else(|| EngineError::Other("no such artboard".into())) } /// The art that goes with artboard `i` when `art`: the objects fully inside it (locked and hidden @@ -106,7 +117,7 @@ pub(crate) fn beside_artboards(d: &vectorcraft_doc::Document, rect: Rect) -> Vec } fn artboard_duplicate(s: &mut Session, p: &Value) -> Result { - let (i, art) = board_and_art(s, p); + let (i, art) = board_and_art(s, p, "artboard.duplicate")?; let art = artboard_art(s, i, art)?; let sc = Scaling { patterns: super::object::transform_patterns(s, p, &art)?, ..Scaling::default() }; let (index, ids) = s.edit("Duplicate Artboard", |d, _| { @@ -119,7 +130,7 @@ fn artboard_duplicate(s: &mut Session, p: &Value) -> Result { /// Artboard Copy / Cut: the artboard and its art onto the clipboard (pasting brings both); a cut /// then deletes them. pub(crate) fn artboard_copy(s: &mut Session, p: &Value, cut: bool) -> Result { - let (i, art) = board_and_art(s, p); + let (i, art) = board_and_art(s, p, if cut { "artboard.cut" } else { "artboard.copy" })?; let art = artboard_art(s, i, art)?; let st = s.doc()?; if cut && st.doc.artboards.len() <= 1 { diff --git a/crates/engine/src/tests_menucmds.rs b/crates/engine/src/tests_menucmds.rs index 22642ec0c..e05e5db99 100644 --- a/crates/engine/src/tests_menucmds.rs +++ b/crates/engine/src/tests_menucmds.rs @@ -736,6 +736,31 @@ fn fit_to_artwork_bounds_fits_each_artboard_to_its_own_art() { assert_eq!(boards(&s), [Rect::new(100.0, 100.0, 1050.0, 280.0), was[1], was[2]], "the first artboard by default"); } +/// Artboard Options and Fit to Selected Art fail with `no such artboard` on an `index` that names +/// no artboard (past the last, negative, fractional, a string or null), and the artboards stay as +/// they were. Artboard Options needs an `index`; Fit to Selected Art without one fits the first +/// artboard. +#[test] +fn artboard_options_and_fit_to_selected_art_refuse_an_index_that_names_no_artboard() { + let mut s = session(); + s.execute("artboard.new", &json!({"x": 1000, "y": 0, "width": 400, "height": 300})).unwrap(); + rect(&mut s, 100.0, 100.0, 50.0, 50.0); + s.execute("select.all", &json!({})).unwrap(); + s.execute("paint.setStroke", &json!({"none": true})).unwrap(); + let boards = |s: &Session| s.doc().unwrap().doc.artboards.iter().map(|a| a.rect).collect::>(); + let (was, n) = (boards(&s), undo_len(&s)); + for index in [json!(99), json!(-1), json!(1.5), json!("1"), Value::Null] { + for (cmd, p) in [("artboard.setProps", json!({"index": index, "width": 300})), ("artboard.fitToSelection", json!({"index": index}))] { + assert_eq!(s.execute(cmd, &p).unwrap_err().to_string(), "no such artboard", "{cmd} {p}"); + } + } + let e = s.execute("artboard.setProps", &json!({"width": 300})).unwrap_err().to_string(); + assert_eq!(e, "invalid parameters for `artboard.setProps`: missing index"); + assert_eq!((boards(&s), undo_len(&s)), (was.clone(), n), "the artboards and the undo steps as they were"); + s.execute("artboard.fitToSelection", &json!({})).unwrap(); + assert_eq!(boards(&s), [Rect::new(100.0, 100.0, 150.0, 150.0), was[1]], "the first artboard by default"); +} + // ---------- Edit Colors ---------- #[test] diff --git a/crates/engine/src/tests_panelcmds.rs b/crates/engine/src/tests_panelcmds.rs index 8c62ffbd1..47066421a 100644 --- a/crates/engine/src/tests_panelcmds.rs +++ b/crates/engine/src/tests_panelcmds.rs @@ -130,6 +130,27 @@ fn duplicate_artboard_copies_its_art() { assert_eq!(ids(&s.execute("artboard.duplicate", &json!({"index": 0, "art": true})).unwrap()["ids"]).len(), 1); } +/// Duplicate, Copy and Cut Artboard and Move Artboard Up/Down fail with `no such artboard` on an +/// `index` that names no artboard (past the last, negative, fractional, a string or null), and the +/// artboards, the art and the clipboard stay as they were. +#[test] +fn artboard_copies_refuse_an_index_that_names_no_artboard() { + let mut s = session(); + rect(&mut s); + s.execute("artboard.new", &json!({})).unwrap(); + s.set_tool_options(Some("artboard"), json!({"active": 1}).as_object().unwrap()); + let (was, n) = (boards(&s), s.doc().unwrap().history.undo.len()); + for index in [json!(99), json!(-1), json!(1.5), json!("1"), serde_json::Value::Null] { + for cmd in ["artboard.duplicate", "artboard.copy", "artboard.cut", "artboard.reorder"] { + let p = if cmd == "artboard.reorder" { json!({"index": index, "to": 0}) } else { json!({"index": index}) }; + assert_eq!(s.execute(cmd, &p).unwrap_err().to_string(), "no such artboard", "{cmd} {p}"); + } + } + let st = s.doc().unwrap(); + assert_eq!((boards(&s), st.doc.layers[0].children().unwrap().len(), st.history.undo.len()), (was, 1, n)); + assert!(s.clipboard.nodes.is_empty() && s.clipboard.artboard.is_none(), "nothing copied"); +} + /// With the Artboard tool, Copy and Paste copy its artboard with the art on it (#446): Paste places /// it right of the last artboard, Paste in Place where it was, also in another document; each paste /// is one undo step, and the tool takes the pasted artboard. diff --git a/docs/mcp.md b/docs/mcp.md index 465a4f704..f77f4b2b7 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -2287,6 +2287,10 @@ pastes only the art), in this document or another; the result's `artboard` is th does the same in one step → `{index, ids}`, and `artboard.move {copy: true, moveArt: true}` is the Artboard tool's Alt-drag. These two commands also move the pattern tiles of the copies when `patterns` is true (default: prefs `transformPatternTiles`); `edit.paste` leaves the tiles of a pasted artboard's art where they were. +`artboard.copy`, `artboard.cut` and `artboard.duplicate` take the Artboard tool's artboard when the call names no +`index`. Every artboard command that takes an `index` fails with `no such artboard` when the `index` is not the +0-based index of an artboard (past the last one, negative, fractional, a string or null), and the document and the +clipboard stay as they were. Scale Artwork with Artboard (#602): `artboard.setProps {index, x?, y?, width?, height?, scaleArt: true}` resizes an artboard and scales the art fully inside it, and its artboard guides, from the old rectangle onto the new one, each side