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 2cfa3a0d..2e4fa2e8 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -488,8 +488,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 a5a15a1f..c1016c29 100644 --- a/crates/engine/src/panels_behaviour_tests.rs +++ b/crates/engine/src/panels_behaviour_tests.rs @@ -403,3 +403,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"); +}