diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index bd86b3b6..b098503c 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -321,7 +321,18 @@ 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); 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. + if taken.contains(&sh.name) + && let Some((base, n)) = sh.name.rsplit_once(' ') + && !n.is_empty() + && n.bytes().all(|b| b.is_ascii_digit()) + { + sh.name = super::insert::first_free_name(base, &taken); + } + taken.insert(sh.name.clone()); 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 df15187c..f2a2b9bd 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -152,27 +152,38 @@ fn default_rect(s: &Session, w: f64, h: f64) -> Xfrm { Xfrm::new((size.width - w) / 2.0, (size.height - h) / 2.0, w, h) } +/// The word an automatic shape name starts with ("Rectangle" in "Rectangle 3"). +pub(crate) fn auto_name_base(shape: &Shape) -> String { + match &shape.kind { + ShapeKind::Picture { .. } => "Picture".to_string(), + ShapeKind::Table(_) => "Table".to_string(), + ShapeKind::Chart(_) => "Chart".to_string(), + ShapeKind::Media(m) if m.video => "Video".to_string(), + ShapeKind::Media(_) => "Audio".to_string(), + ShapeKind::Model3d { .. } => "3D Model".to_string(), + _ if shape.text_box => "TextBox".to_string(), + _ => shape + .geom + .preset_name() + .and_then(deckcraft_geom::preset::info) + .map(|i| i.label.split(':').next().unwrap_or(i.label).trim().to_string()) + .unwrap_or_else(|| "Shape".into()), + } +} + +/// "{base} n" with the first `n` from the shape count + 1 that no shape on the slide already uses, +/// so a gap left by a deleted shape never makes a second shape with the same name. +pub(crate) fn first_free_name(base: &str, taken: &std::collections::HashSet) -> String { + (taken.len() + 1..).map(|n| format!("{base} {n}")).find(|c| !taken.contains(c)).unwrap_or_default() +} + pub(crate) fn add_shape(s: &mut Session, cmd: &str, mut shape: Shape, select: bool) -> Result { s.edit(|doc, sel| { shape.id = ShapeId(doc.alloc_id().ok_or_else(|| bad(cmd, NO_ID_SPACE))?); if shape.name.is_empty() { - let base = match &shape.kind { - ShapeKind::Picture { .. } => "Picture".to_string(), - ShapeKind::Table(_) => "Table".to_string(), - ShapeKind::Chart(_) => "Chart".to_string(), - ShapeKind::Media(m) if m.video => "Video".to_string(), - ShapeKind::Media(_) => "Audio".to_string(), - ShapeKind::Model3d { .. } => "3D Model".to_string(), - _ if shape.text_box => "TextBox".to_string(), - _ => shape - .geom - .preset_name() - .and_then(deckcraft_geom::preset::info) - .map(|i| i.label.split(':').next().unwrap_or(i.label).trim().to_string()) - .unwrap_or_else(|| "Shape".into()), - }; - let n = super::current_shapes(doc, sel).map(|v| v.len()).unwrap_or(0) + 1; - shape.name = format!("{base} {n}"); + let base = auto_name_base(&shape); + let taken = super::current_names(doc, sel); + shape.name = first_free_name(&base, &taken); } let id = shape.id; let list = crate::shapes_mut(doc, sel).ok_or_else(|| bad("insert", "no slide"))?; diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index a9d91aff..59513c6b 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -515,6 +515,10 @@ 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() +} + /// The command palette runs a command with `{}`. True when the params doc names a value that /// call would have to supply (`path`, `rows`, `path? | data`), so the palette must not offer it. /// A name ending in `?` is optional. Text after `→` is the result, not an input. diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index a0f200ca..7e1e4242 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -214,6 +214,77 @@ fn clipboard_copy_paste_shapes_and_slides() { assert!(s.doc().unwrap().doc.validate().is_empty()); } +#[test] +fn pasted_shapes_get_the_next_automatic_name() { + let mut s = session(); + let names = |s: &Session| s.doc().unwrap().current_slide().unwrap().shapes.iter().map(|x| x.name.clone()).collect::>(); + let before = names(&s).len(); + s.execute("shape.insert", &json!({"preset": "rect", "rect": [10, 10, 50, 50]})).unwrap(); + let auto = names(&s)[before].clone(); + s.execute("edit.copy", &json!({})).unwrap(); + s.execute("edit.paste", &json!({})).unwrap(); + s.execute("edit.paste", &json!({})).unwrap(); + let all = names(&s); + let base = auto.rsplit_once(' ').unwrap().0; + assert_eq!(all[before + 1], format!("{base} {}", before + 2)); + assert_eq!(all[before + 2], format!("{base} {}", before + 3)); + + // A name the user chose is not an automatic one and stays on the copy. + let id = s.doc().unwrap().selection.shapes[0].0; + s.execute("shape.rename", &json!({"id": id, "name": "Hero"})).unwrap(); + s.execute("edit.copy", &json!({})).unwrap(); + s.execute("edit.paste", &json!({})).unwrap(); + assert_eq!(names(&s).last().unwrap(), "Hero"); +} + +#[test] +fn pasting_after_a_deleted_shape_does_not_repeat_a_name() { + let mut s = session(); + let names = |s: &Session| s.doc().unwrap().current_slide().unwrap().shapes.iter().map(|x| x.name.clone()).collect::>(); + let mut ids = vec![]; + for _ in 0..3 { + s.execute("shape.insert", &json!({"preset": "rect", "rect": [10, 10, 50, 50]})).unwrap(); + ids.push(s.doc().unwrap().selection.shapes[0].0); + } + s.execute("edit.delete", &json!({"ids": [ids[1]]})).unwrap(); + s.execute("slide.go", &json!({"index": 0})).unwrap(); + s.doc_mut().unwrap().selection.shapes = vec![deckcraft_model::ShapeId(ids[2])]; + s.execute("edit.copy", &json!({})).unwrap(); + s.execute("edit.paste", &json!({})).unwrap(); + let all = names(&s); + let mut unique = all.clone(); + unique.sort(); + unique.dedup(); + assert_eq!(unique.len(), all.len(), "{all:?}"); +} + +#[test] +fn pasting_on_another_slide_keeps_names_so_morph_still_pairs_them() { + let mut s = session(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + s.execute("shape.insert", &json!({"preset": "rect", "rect": [0, 0, 20, 20]})).unwrap(); + let first = s.doc().unwrap().selection.shapes[0].0; + s.execute("shape.insert", &json!({"preset": "rect", "rect": [10, 10, 50, 50]})).unwrap(); + s.execute("shape.insert", &json!({"preset": "rect", "rect": [200, 200, 80, 80]})).unwrap(); + s.execute("edit.delete", &json!({"ids": [first]})).unwrap(); + s.execute("edit.selectAll", &json!({})).unwrap(); + s.execute("edit.copy", &json!({})).unwrap(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + s.execute("edit.paste", &json!({})).unwrap(); + let st = s.doc().unwrap(); + let (a, b) = (&st.doc.slides[1], &st.doc.slides[2]); + let an: Vec<_> = a.shapes.iter().map(|x| x.name.clone()).collect(); + let bn: Vec<_> = b.shapes.iter().map(|x| x.name.clone()).collect(); + assert_eq!(an, bn); + let pairs = deckcraft_anim::morph_pairs(a, b); + assert_eq!(pairs.len(), 2); + for (ia, ib) in pairs { + let na = &a.shapes.iter().find(|x| x.id == ia).unwrap().name; + let nb = &b.shapes.iter().find(|x| x.id == ib).unwrap().name; + assert_eq!(na, nb); + } +} + #[test] fn bold_copy_is_html_and_pages_html_pastes_bold() { let mut s = session();