From 626db59d539d62889980d84465312b63b15e2a77 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 10 Nov 2026 13:28:28 -0800 Subject: [PATCH] Expand: object.expand rejects a param of the wrong type object.expand read `object`, `fill` and `stroke` with bool_or, `steps` with f64_or and `gradient` with str_param, so a value of the wrong type fell back to the default: "fill": "yes" expanded the fill, "steps": "16" made 255 strips and "gradient": 3 made strips. They are now read with switch_or, number_or and text_param, and such a value is an error that names the param, before anything changes. Omitting a param or giving null keeps its default. --- crates/engine/src/cmd/expand.rs | 12 ++++----- crates/engine/src/tests/objexpand.rs | 37 ++++++++++++++++++++++++++++ docs/mcp.md | 2 ++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/expand.rs b/crates/engine/src/cmd/expand.rs index 8834e6a74..546b5a825 100644 --- a/crates/engine/src/cmd/expand.rs +++ b/crates/engine/src/cmd/expand.rs @@ -25,7 +25,7 @@ use super::edit::selected_roots; use super::menucmds::{run_raw, squash}; use super::object::as_clipping_path; use super::pathops::{assemble, node_path, outline_strokes, replace_leaves, styled_copy, with_transparency}; -use super::{CommandSpec, EngineError, Result, Session, bad, bool_or, cmd, f64_or, has_selection, str_param}; +use super::{CommandSpec, EngineError, Result, Session, bad, cmd, has_selection, number_or, switch_or, text_param}; const C: &str = "object.expand"; /// Expand Gradient To → Specify … Objects: the default and the most. @@ -101,11 +101,11 @@ enum GradientTo { impl GradientTo { fn parse(p: &Value) -> Result { - let steps = f64_or(p, "steps", DEFAULT_STEPS as f64).round(); + let steps = number_or(p, "steps", DEFAULT_STEPS as f64, C)?.round(); if !(1.0..=MAX_STEPS as f64).contains(&steps) { return Err(bad(C, format!("steps must be 1..{MAX_STEPS}"))); } - match str_param(p, "gradient").unwrap_or("objects") { + match text_param(p, "gradient", C)?.unwrap_or("objects") { "objects" => Ok(Self::Objects(steps as u32)), "mesh" => Ok(Self::Mesh), _ => Err(bad(C, "gradient must be \"objects\" or \"mesh\"")), @@ -236,9 +236,9 @@ fn expand(s: &mut Session, p: &Value) -> Result { } fn expand_steps(s: &mut Session, p: &Value) -> Result { - let object = bool_or(p, "object", true); - let fill = bool_or(p, "fill", true); - let stroke = bool_or(p, "stroke", true); + let object = switch_or(p, "object", true, C)?; + let fill = switch_or(p, "fill", true, C)?; + let stroke = switch_or(p, "stroke", true, C)?; let to = GradientTo::parse(p)?; let from = s.doc()?.history.undo.len(); let mut e = Expandable::of(&s.doc()?.doc, &selected_roots(s)?); diff --git a/crates/engine/src/tests/objexpand.rs b/crates/engine/src/tests/objexpand.rs index b9ccac12d..2930bf172 100644 --- a/crates/engine/src/tests/objexpand.rs +++ b/crates/engine/src/tests/objexpand.rs @@ -1086,6 +1086,43 @@ fn defaults_match_the_dialog_and_info_tells_what_applies() { assert!(matches!(selected(&s).kind, NodeKind::Path { live: Some(_), .. })); } +/// Expand refuses an option of the wrong type with an error naming it, and leaves the art, the +/// selection and the undo history as they were. Null counts as an option left out, which keeps its +/// default. +#[test] +fn expand_refuses_an_option_of_the_wrong_type() { + let mut s = session(); + let id = gradient_rect(&mut s, Rect::new(20.0, 20.0, 180.0, 120.0), white_black(), true); + let state = |s: &Session| { + let st = s.doc().unwrap(); + (st.revision, st.history.undo.len(), st.history.redo.len(), st.selection.objects.clone()) + }; + let before = (state(&s), node(&s, id)); + let switch = |k: &str| format!("`{k}` must be true or false"); + for (p, want) in [ + (json!({"object": "yes"}), switch("object")), + (json!({"object": 0}), switch("object")), + (json!({"fill": "false"}), switch("fill")), + (json!({"stroke": 1}), switch("stroke")), + (json!({"gradient": 3}), "`gradient` must be a string".to_string()), + (json!({"gradient": true}), "`gradient` must be a string".to_string()), + (json!({"steps": "16"}), "`steps` must be a number, not \"16\"".to_string()), + (json!({"steps": [16]}), "`steps` must be a number, not [16]".to_string()), + ] { + let r = s.execute("object.expand", &p); + assert!(matches!(&r, Err(EngineError::BadParams { msg, .. }) if *msg == want), "{p}: {r:?}"); + assert_eq!((state(&s), node(&s, id)), before, "{p}"); + } + // Null options take their defaults: Object, Fill and Stroke on, 255 solid strips. + run(&mut s, "object.expand", json!({"object": null, "fill": null, "stroke": null, "gradient": null, "steps": null})); + let g = selected(&s); + let NodeKind::Group { children, .. } = &g.kind else { panic!("{:?}", g.kind) }; + let (clip, strips) = clip_parts(&children[0]); + assert!(matches!(clip.kind, NodeKind::Path { live: None, .. })); + assert_eq!(strips.len(), 255); + assert!(children[1].appearance.stroke_paint().is_none(), "outlined stroke"); +} + /// A selected open line with a 2 pt black stroke, no fill and a Zig Zag effect (#3089). fn zig_zag_line(s: &mut Session) -> NodeId { let id = NodeId(run(s, "path.create", json!({"d": "M 20 100 L 180 100"}))["id"].as_u64().unwrap()); diff --git a/docs/mcp.md b/docs/mcp.md index 566f2e368..3ce55d7dc 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -3411,6 +3411,8 @@ holds symbol instances that `object` expands, whether only their art holds it, w mesh points as the gradient paints them, with the colors Object › Expand gives them. `object.expand`, `effect.expandAppearance`, `type.createOutlines` and `object.path.outlineStroke` turn a dimension whose art they change into a plain group of that art, in the same undo step (see [dimensions.md](dimensions.md)). +`object.expand` rejects a param of the wrong type, such as `"fill": "yes"`, `"gradient": 3` or `"steps": "16"`, and +changes nothing. Omitting a param or giving null keeps its default. ```json {"name":"run_command","arguments":{"command":"object.expand","params":{"stroke":false,"gradient":"mesh"}}}