From f9e78eb144312c68df6594e039f7806ecf063e02 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 31 Oct 2026 08:44:05 -0700 Subject: [PATCH] Appearance: effect.apply reports the active row's object, and an empty aligned target list is an error effect.apply now reports the item of the first selected object, whose row the active item is. With nothing selected, an explicit aligned item returns the command's error instead of an internal error. --- crates/engine/src/cmd/appearance.rs | 2 +- crates/engine/src/cmd/effectcmd.rs | 5 +++-- crates/engine/src/tests/appearance.rs | 10 ++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 66d6ab232..33379e752 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -262,7 +262,7 @@ impl ItemTarget { /// targets don't share, and any other target, stands for itself in every target. pub(crate) fn per_target(self, s: &Session, ids: &[NodeId]) -> Vec> { let ItemTarget::Item { index, explicit, aligned: true } = self else { return vec![Some(self); ids.len()] }; - let Some(st) = s.active() else { return vec![Some(self); ids.len()] }; + let Some(st) = s.active().filter(|_| ids.len() >= 2) else { return vec![Some(self); ids.len()] }; let k = self.anchor(s, ids); let mut order = ids.to_vec(); order.swap(0, k); diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index 7ae1b72cc..25a5ff004 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -219,8 +219,9 @@ pub(crate) fn apply(s: &mut Session, p: &Value) -> Result { index = fx.len() - 1; true })?; - // The item the effect landed on in the first object (the active item applies where it fits). - let first = ids.first().and_then(|id| s.doc().ok()?.doc.node(*id)); + // The item the effect landed on in the object whose row the item is (the first selected object + // for the active item, which applies where it fits). + let first = ids.get(item.anchor(s, &ids)).and_then(|id| s.doc().ok()?.doc.node(*id)); let landed = first.and_then(|n| item.effects_item(&n.appearance, C).ok().flatten()); Ok(json!({ "ids": ids_json(&ids), "index": index, "item": landed })) } diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index d18de9e30..e36871188 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -501,6 +501,16 @@ fn edits_change_the_active_row_of_the_first_selected_object() { assert!(matches!(fill_paint(&na, 0), Paint::Gradient(_)), "so does a's equal fill"); assert_eq!(fill_paint(&nb, 0).color().unwrap().to_hex(), "#ff0000"); assert!(!matches!(stroke(&nb, 2).paint, Paint::Gradient(_)) && !matches!(stroke(&na, 1).paint, Paint::Gradient(_))); + // The effect goes on the equal rows, and the reply names the active row of b. + run(&mut s, "appearance.setActiveItem", json!({"index": 2})); // b's stroke, a's row 1 + let r = run(&mut s, "effect.apply", json!({"effect": "distort.twist"})); + assert_eq!(r["item"], 2); + let (na, nb) = (node(&s, a), node(&s, b)); + assert_eq!((stroke(&nb, 2).effects.len(), stroke(&na, 1).effects.len()), (1, 1)); + // With nothing selected an aligned item is an error, not a crash. + run(&mut s, "select.set", json!({"ids": []})); + let e = s.execute("effect.remove", &json!({"index": 0, "item": 0, "aligned": true})).unwrap_err().to_string(); + assert!(!e.contains("internal error"), "{e}"); } /// How many steps there are to undo and to redo, and whether the document has unsaved changes.