Fix #3965: an unresolved step reference given as id is refused under the command's name; effect.apply keeps its effect-naming id

This commit is contained in:
Taha-Mahmoodi
2026-10-24 17:57:21 +07:00
committed by bflatastic
parent c40113b755
commit 97f84723ef
5 changed files with 30 additions and 8 deletions
+2 -2
View File
@@ -376,9 +376,9 @@ pub(crate) fn given_objects(s: &Session, p: &Value, command: &str) -> Result<Opt
}
match p.get("id").filter(|v| !v.is_null()) {
None => 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)?])),
}
+7 -2
View File
@@ -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
@@ -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");
+3 -2
View File
@@ -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"}`).
+1
View File
@@ -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 |