diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index c09279e2a..9c5d38158 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -376,9 +376,9 @@ pub(crate) fn given_objects(s: &Session, p: &Value, command: &str) -> Result Ok(None), - // A step reference a batch couldn't resolve, as `targets` reports it. + // A step reference a batch couldn't resolve, as `targets` reports it, under the command. Some(Value::String(r)) if r.starts_with('$') => { - Err(EngineError::Other(format!("`id` is the step reference {r:?}, which only `run` and `command_batch` resolve"))) + Err(bad(command, format!("`id` is the step reference {r:?}, which only `run` and `command_batch` resolve"))) } Some(v) => Ok(Some(vec![checked_id(s, v, command)?])), } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index eb8a6fbc8..aa6fe3e54 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1762,8 +1762,13 @@ impl Session { self.check_params(id, params)?; // Objects given as `id` (where it stands for `ids` of one) or as `ids` beside it are checked // before anything else: a malformed one, or one naming no object, is an error under this - // command's id, never the selection in its place. - if cmd::params::id_stands_for_ids(spec.params) && self.active().is_some() { + // command's id, never the selection in its place. `effect.apply {id: "stylize.dropShadow"}` + // without `effect` names the effect (its older key), not an object. + let effect_named = id == "effect.apply" + && params.get("effect").is_none() + && params.get("ids").is_none_or(Value::is_null) + && params.get("id").and_then(Value::as_str).is_some_and(|e| !e.starts_with('$')); + if cmd::params::id_stands_for_ids(spec.params) && !effect_named && self.active().is_some() { cmd::given_objects(self, params, id)?; } // Clear's menu needs a selection; an agent supplying explicit targets only needs a diff --git a/crates/engine/src/tests_ids_without_selection.rs b/crates/engine/src/tests_ids_without_selection.rs index 1d1f4b325..1660a7630 100644 --- a/crates/engine/src/tests_ids_without_selection.rs +++ b/crates/engine/src/tests_ids_without_selection.rs @@ -136,7 +136,8 @@ fn id_does_what_ids_of_one_does_with_nothing_selected() { /// Every command that takes `id` as `ids` of one ([`cmd::params::id_stands_for_ids`]), with an /// object selected: an `id` that is malformed (a string, negative, a fraction, an object) or names /// no object is refused under the command's id, as the same value in `ids` is, and the document -/// and the selection stay as they were. It never stands for the selection instead. +/// and the selection stay as they were. It never stands for the selection instead. (A string `id` +/// given to `effect.apply` without `effect` names the effect, which "bad" isn't: refused too.) #[test] fn a_bad_id_is_refused_as_the_same_ids_are_and_changes_nothing() { let (_, [.., d], _) = doc_with_type(); @@ -158,7 +159,8 @@ fn a_bad_id_is_refused_as_the_same_ids_are_and_changes_nothing() { (r, st.doc == before && st.selection.objects == vec![NodeId(d)]) }; let (with_id, with_ids) = (run("id", bad.clone()), run("ids", json!([bad.clone()]))); - if with_id.0.is_ok() || !with_id.1 || with_id != with_ids { + let effect_named = spec.id == "effect.apply" && bad.is_string(); + if with_id.0.is_ok() || !with_id.1 || (with_id != with_ids && !effect_named) { failures.push(format!( "{} {bad}: with id {:.160} (unchanged: {}); with ids {:.160}", spec.id, @@ -168,6 +170,19 @@ fn a_bad_id_is_refused_as_the_same_ids_are_and_changes_nothing() { )); } } + // A step reference no batch resolved is refused under this command's id too. + let (mut s, ..) = doc_with_type(); + s.execute("select.set", &json!({ "ids": [d] })).unwrap(); + let before = s.doc().unwrap().doc.clone(); + let mut p = params_for(spec.id, d); + p["id"] = json!("$missing"); + match s.execute(spec.id, &p) { + Err(EngineError::BadParams { cmd, msg }) if cmd == spec.id && msg.contains("step reference") => {} + r => failures.push(format!("{} \"$missing\": {r:?}", spec.id)), + } + if s.doc().unwrap().doc != before { + failures.push(format!("{} \"$missing\": the document changed", spec.id)); + } } assert!(failures.is_empty(), "{} fail:\n{}", failures.len(), failures.join("\n")); assert!(checked >= 100, "{checked} commands"); diff --git a/docs/mcp.md b/docs/mcp.md index 2e2f5138b..fccffb8db 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -350,8 +350,9 @@ document is 612 × 792 (US Letter). New objects become the selection. Most comma explicit `ids`, which stand in for the selection, so they work with nothing selected. Where a command's params say `ids?|id?` (or `id?, ids?: … (instead of id)`), `id` is `ids` of one object: `{"id": 7}` does what `{"ids": [7]}` does, with or without a selection, and an `id` or `ids` that is malformed (not a non-negative integer) or names no -object is refused under the command's name, never replaced by the selection. 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. +object is refused under the command's name, never replaced by the selection (`effect.apply {"id": "warp.arc"}` +without `effect` names the effect). 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. Paint values (`fill`, `stroke`) accept `"#rrggbb"`, `"none"`, `[r,g,b]` (0..1), `{"c","m","y","k"}`, `{"gray"}`, or a full `paint.setFill` params object (`{"gradient": …}`, `{"swatch": "name"}`). diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 70d41e315..f154f8f83 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1092,6 +1092,7 @@ applies to fills). Tests: `color::gradient::tests` (perceptual midpoint, expansi | Date | Change | Summary | |---|---|---| +| 2026-10-24 | minor | Commands: a command whose params take `ids?\|id?` (or `id?, ids?: … (instead of id)`) takes `id` as `ids` of one with nothing selected too, instead of refusing it as "nothing selected" (29 such commands needed a selection); Invert, Complement, Gradient Vector and Make Pixel Perfect act on the object given as `id`; an `id` or `ids` that is malformed or names no object is refused under the command's name instead of the selection standing in for it (`plugin.run`'s required `id`, the plug-in's, is not an object) (`tests_ids_without_selection::id_does_what_ids_of_one_does_with_nothing_selected`, `a_bad_id_is_refused_as_the_same_ids_are_and_changes_nothing`); scores unchanged | | 2026-10-24 | minor | Commands: a command whose params take `ids?|id?` (or `id?, ids?: … (instead of id)`) takes `id` as `ids` of one with nothing selected too, instead of refusing it as "nothing selected" (29 such commands needed a selection); Invert, Complement, Gradient Vector and Make Pixel Perfect act on the object given as `id`; an `id` or `ids` that is malformed or names no object is refused under the command's name instead of the selection standing in for it (`plugin.run`'s required `id`, the plug-in's, is not an object) (`tests_ids_without_selection::id_does_what_ids_of_one_does_with_nothing_selected`, `a_bad_id_is_refused_as_the_same_ids_are_and_changes_nothing`); scores unchanged | | 2026-10-24 | minor | Commands: a command whose params take `ids?|id?` (or `id?, ids?: … (instead of id)`) takes `id` as `ids` of one with nothing selected too, instead of refusing it as "nothing selected" (29 such commands needed a selection); Invert, Complement, Gradient Vector and Make Pixel Perfect act on the object given as `id` (`tests_ids_without_selection::id_does_what_ids_of_one_does_with_nothing_selected`); scores unchanged | | 2026-10-24 | minor | Type advanced: ruby (ふりがな) is kept, where it is laid out, by Type › Create Outlines and in EPS, EMF/WMF and legacy `.ai` exports (it was dropped there), see [type-parity.md](type-parity.md); scores and hours unchanged |