From f554de28b5380ef69a6d47c3330cb76fbac86404 Mon Sep 17 00:00:00 2001
From: Tanish Manku <tmanku.dev@gmail.com>
Date: Wed, 4 Nov 2026 13:02:38 -0700
Subject: [PATCH] XCLIP: ON/OFF checks without copying boundaries and sets each
 reference once (CodeRabbit on #2756)

---
 crates/engine/src/cmd/blocks/xclip.rs | 22 +++++++++++++++-------
 crates/engine/tests/it/xclip.rs       |  4 ++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/crates/engine/src/cmd/blocks/xclip.rs b/crates/engine/src/cmd/blocks/xclip.rs
index b9a638827..067932127 100644
--- a/crates/engine/src/cmd/blocks/xclip.rs
+++ b/crates/engine/src/cmd/blocks/xclip.rs
@@ -117,14 +117,22 @@ fn run(s: &mut Session, p: &Value) -> Result<Value> {
         "on" | "off" => {
             let on = option == "on";
             // Every reference is checked before any changes: an error leaves the drawing as it was.
-            let mut clips = Vec::with_capacity(hs.len());
-            for h in &hs {
-                let mut c = clip_of(s, *h).ok_or_else(|| bad("xclip", "no clipping boundary on the selected reference"))?;
-                c.enabled = on;
-                clips.push((*h, c));
+            let d = s.doc()?;
+            if !hs.iter().all(|h| matches!(d.entity(*h).map(|e| &e.kind), Some(EntityKind::Insert(i)) if i.clip.is_some())) {
+                return Err(bad("xclip", "no clipping boundary on the selected reference"));
             }
-            for (h, c) in clips {
-                set_clip(s, h, Some(c))?;
+            // Only the flag changes, once per reference (a selection may name one many times).
+            let mut seen = std::collections::HashSet::new();
+            for h in hs.iter().filter(|h| seen.insert(**h)) {
+                s.doc_mut()?
+                    .modify_entity(*h, |e| {
+                        if let EntityKind::Insert(i) = &mut e.kind
+                            && let Some(c) = &mut i.clip
+                        {
+                            c.enabled = on;
+                        }
+                    })
+                    .map_err(|e| bad("xclip", e.to_string()))?;
             }
         }
         "delete" | "d" => {
diff --git a/crates/engine/tests/it/xclip.rs b/crates/engine/tests/it/xclip.rs
index c49f20377..10b551716 100644
--- a/crates/engine/tests/it/xclip.rs
+++ b/crates/engine/tests/it/xclip.rs
@@ -88,6 +88,10 @@ fn off_on_a_selection_with_an_unclipped_reference_changes_nothing() {
     assert!(s.execute("xclip", &json!({ "handles": [r1, r2], "option": "off" })).is_err());
     assert!(clip(&s, &r1).unwrap().enabled);
     assert!(clip(&s, &r2).is_none());
+    // A reference named many times is turned off once, its boundary kept.
+    s.execute("xclip", &json!({ "handles": vec![r1.clone(); 1000], "option": "off" })).unwrap();
+    let c = clip(&s, &r1).unwrap();
+    assert!(!c.enabled && c.points.len() == 2);
 }
 
 #[test]