From 352fec7725408e80bdee168aa02ac4d7e9af9f5a Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Fri, 30 Oct 2026 15:53:18 -0400 Subject: [PATCH] Insert and paste count group members when picking the next automatic name Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd.rs | 14 ++++++++++++-- crates/engine/src/cmd/edit.rs | 9 ++++++--- crates/engine/src/cmd/insert.rs | 4 ++-- crates/engine/src/tests.rs | 27 +++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 59513c6b..6f20c414 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -515,8 +515,18 @@ pub(crate) fn current_shapes<'a>(doc: &'a deckcraft_model::Presentation, sel: &c } } -pub(crate) fn current_names(doc: &deckcraft_model::Presentation, sel: &crate::Selection) -> std::collections::HashSet { - current_shapes(doc, sel).map(|v| v.iter().map(|s| s.name.clone()).collect()).unwrap_or_default() +/// Every name on the slide, group members included, and the number of shapes (which exceeds the +/// number of names when two shapes share one). +pub(crate) fn current_names(doc: &deckcraft_model::Presentation, sel: &crate::Selection) -> (std::collections::HashSet, usize) { + let mut names = std::collections::HashSet::new(); + let mut count = 0; + if let Some(v) = current_shapes(doc, sel) { + deckcraft_model::walk(v, &mut |sh, _| { + names.insert(sh.name.clone()); + count += 1; + }); + } + (names, count) } /// The command palette runs a command with `{}`. True when the params doc names a value that diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index ff69575f..f084c622 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -321,7 +321,7 @@ fn paste(s: &mut Session, p: &Value) -> Result { let mut shapes = clip.shapes.clone(); reid("edit.paste", doc, &mut shapes)?; crate::media_refs::visit_mut::shapes(&mut shapes, &mut remap_media(&media)); - let mut taken = super::current_names(doc, sel); + let (mut taken, mut count) = super::current_names(doc, sel); for sh in &mut shapes { // Only a name already on the slide is changed (two shapes with one name are what the // issue is about); otherwise it stays, as Morph pairs shapes across slides by name. @@ -330,9 +330,12 @@ fn paste(s: &mut Session, p: &Value) -> Result { && !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit()) { - sh.name = super::insert::first_free_name(base, &taken, taken.len()); + sh.name = super::insert::first_free_name(base, &taken, count); } - taken.insert(sh.name.clone()); + deckcraft_model::walk(std::slice::from_ref(sh), &mut |m, _| { + taken.insert(m.name.clone()); + count += 1; + }); if let Some(x) = sh.xfrm.as_mut() { x.x += off; x.y += off; diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 92f44711..5d9c9344 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -198,8 +198,8 @@ pub(crate) fn add_shape(s: &mut Session, cmd: &str, mut shape: Shape, select: bo shape.id = ShapeId(doc.alloc_id().ok_or_else(|| bad(cmd, NO_ID_SPACE))?); if shape.name.is_empty() { let base = auto_name_base(&shape); - let taken = super::current_names(doc, sel); - shape.name = first_free_name(&base, &taken, taken.len()); + let (taken, count) = super::current_names(doc, sel); + shape.name = first_free_name(&base, &taken, count); } let id = shape.id; let list = crate::shapes_mut(doc, sel).ok_or_else(|| bad("insert", "no slide"))?; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index b2298100..da4dfab4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -311,6 +311,33 @@ fn duplicating_a_group_does_not_repeat_the_names_of_its_members() { assert_eq!(unique.len(), all.len(), "{all:?}"); } +/// A shape inserted or pasted after a group was duplicated takes none of its members' names. +#[test] +fn inserting_after_duplicating_a_group_does_not_repeat_a_member_name() { + let mut s = session(); + let mut ids = vec![]; + for x in [10, 70] { + s.execute("shape.insert", &json!({"preset": "rect", "rect": [x, 10, 50, 50]})).unwrap(); + ids.push(s.doc().unwrap().selection.shapes[0].0); + } + s.execute("arrange.group", &json!({"ids": ids})).unwrap(); + let group = s.doc().unwrap().selection.shapes[0].0; + s.execute("edit.duplicate", &json!({"ids": [group]})).unwrap(); + // Fewer top-level shapes than shapes: counting only the top level reaches a member's number. + for x in [130, 190, 250] { + s.execute("shape.insert", &json!({"preset": "rect", "rect": [x, 10, 50, 50]})).unwrap(); + } + s.execute("edit.copy", &json!({})).unwrap(); + s.execute("edit.paste", &json!({})).unwrap(); + let doc = s.doc().unwrap(); + let mut all = vec![]; + deckcraft_model::walk(&doc.current_slide().unwrap().shapes, &mut |sh, _| all.push(sh.name.clone())); + let mut unique = all.clone(); + unique.sort(); + unique.dedup(); + assert_eq!(unique.len(), all.len(), "{all:?}"); +} + /// Group members are renamed like any other copied shape. #[test] fn duplicating_a_group_renames_its_automatic_children() {