XCLIP: ON/OFF checks without copying boundaries and sets each reference once (CodeRabbit on #2756)

This commit is contained in:
Tanish Manku
2026-11-04 13:02:38 -07:00
parent 371e8b9e02
commit f554de28b5
2 changed files with 19 additions and 7 deletions
+15 -7
View File
@@ -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" => {
+4
View File
@@ -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]