From 245837ff27ca0b5b3e5a7c012849918cba40851c Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Wed, 14 Oct 2026 22:52:15 +0300 Subject: [PATCH] doc: name counters stop at u64::MAX instead of overflowing Document::next_name summed max + 1 over numbers parsed from existing layer names, and bump_name did the same for the duplicate-rename path: a document (or peer, or node.set) holding a layer named "Frame 18446744073709551615" overflowed both (panic in debug, wrap to a wrong name in release). next_name now collects the numbers it sees and, when the counter is spent, picks the lowest free number (mirrors new_style_id); bump_name only bumps when the number can step, else falls to its existing " 2" branch. Tests: next_name_never_overflows (doc) and duplicating_a_layer_named_past_the_counter_does_not_overflow (engine panel behaviour). --- crates/doc/src/lib.rs | 9 ++++++++- crates/doc/src/tests.rs | 10 ++++++++++ crates/engine/src/cmd/edit.rs | 3 ++- crates/engine/src/panels_behaviour_tests.rs | 11 +++++++++++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 3ae8bbe8..513800e3 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -903,15 +903,22 @@ impl Document { /// A unique name like "Frame 3" for a new node of a kind. pub fn next_name(&self, base: &str) -> String { let mut max = 0u64; + let mut taken = std::collections::BTreeSet::new(); for n in self.nodes() { if let Some(rest) = n.name.strip_prefix(base) && let Some(num) = rest.strip_prefix(' ') && let Ok(k) = num.parse::() { max = max.max(k); + taken.insert(k); } } - format!("{base} {}", max + 1) + let next = match max.checked_add(1) { + Some(next) => next, + // The counter is spent: the lowest free number instead of wrapping onto a name. + None => (1u64..).find(|k| !taken.contains(k)).unwrap_or(1), + }; + format!("{base} {next}") } pub fn style(&self, id: &str) -> Option<&Style> { diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index ef2f37b9..d54bb29b 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -335,6 +335,16 @@ fn next_name_increments() { assert_eq!(d.next_name("Ellipse"), "Ellipse 1"); } +#[test] +fn next_name_never_overflows() { + let mut d = Document::new("x"); + let p = page(&d); + d.create(p, NodeKind::Frame, "Frame 18446744073709551615", 0.0, 0.0, 1.0, 1.0).unwrap(); + assert_eq!(d.next_name("Frame"), "Frame 1", "the counter can't step past u64::MAX"); + d.create(p, NodeKind::Frame, "Frame 1", 0.0, 0.0, 1.0, 1.0).unwrap(); + assert_eq!(d.next_name("Frame"), "Frame 2"); +} + #[test] fn custom_easings() { use crate::proto::Easing; diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 57ba9fb0..4f5e91eb 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -446,8 +446,9 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { fn bump_name(n: &str) -> String { if let Some((base, num)) = n.rsplit_once(' ') && let Ok(k) = num.parse::() + && let Some(next) = k.checked_add(1) { - return format!("{base} {}", k + 1); + return format!("{base} {next}"); } format!("{n} 2") } diff --git a/crates/engine/src/panels_behaviour_tests.rs b/crates/engine/src/panels_behaviour_tests.rs index f2802461..b1097398 100644 --- a/crates/engine/src/panels_behaviour_tests.rs +++ b/crates/engine/src/panels_behaviour_tests.rs @@ -386,3 +386,14 @@ fn assets_panel_lists_files_and_drops_by_the_documented_rules() { assert_eq!(kids(&s, row).first(), Some(&into), "an asset dropped in an auto layout frame joins the flow where it is dropped"); assert_eq!(s.doc().unwrap().page_of(into), Some(page)); } + +#[test] +fn duplicating_a_layer_named_past_the_counter_does_not_overflow() { + let mut s = Session::new(); + ex(&mut s, "pref.renameDuplicates", json!({"on": true})); + let r = rect(&mut s, "r", 30.0, 40.0, None); + ex(&mut s, "node.set", json!({"id": r, "props": {"name": "Layer 18446744073709551615"}})); + let dup = id_of(&ex(&mut s, "edit.duplicate", json!({"ids": [r]}))["ids"][0]).unwrap(); + assert_eq!(name(&s, dup), "Layer 18446744073709551615 2"); + assert!(s.doc().unwrap().validate().is_empty(), "the document still checks out"); +}