diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index fada7e98..c2aab92e 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -810,15 +810,17 @@ impl Document { self.layers.push(Arc::new(Node::layer(id, &name, LayerColor::Preset((n % LAYER_COLORS.len()) as u8)))); id } + /// The default name of a new layer or sublayer, "Layer N". Layers and sublayers share the + /// numbering: it starts after the number of layers there are and skips any layer's name. pub fn next_layer_name(&self) -> String { - let mut i = self.layers.len() + 1; - loop { - let name = format!("Layer {i}"); - if !self.layers.iter().any(|l| l.name.as_deref() == Some(&name)) { - return name; + let (mut count, mut taken) = (0, std::collections::HashSet::new()); + self.walk(|n| { + if n.is_layer() { + count += 1; + taken.extend(n.name.as_deref()); } - i += 1; - } + }); + (count + 1..).map(|i| format!("Layer {i}")).find(|name| !taken.contains(name.as_str())).unwrap_or_default() } /// Visit every node depth first in paint order (bottom to top). diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 7139d008..9fc9353d 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -228,24 +228,10 @@ fn all_layers(d: &Document) -> Vec<&Node> { out } -/// The next unused `Layer N` name, counting sublayers too. -pub(crate) fn next_layer_name(d: &Document) -> String { - let layers = all_layers(d); - let taken: HashSet<&str> = layers.iter().filter_map(|l| l.name.as_deref()).collect(); - let mut i = layers.len() + 1; - loop { - let name = format!("Layer {i}"); - if !taken.contains(name.as_str()) { - return name; - } - i += 1; - } -} - /// A new layer named `name` (default: the next `Layer N`) in the next layer colour. pub(crate) fn make_layer(d: &mut Document, name: Option<&str>) -> Node { let n = all_layers(d).len(); - let name = name.map(str::to_string).unwrap_or_else(|| next_layer_name(d)); + let name = name.map(str::to_string).unwrap_or_else(|| d.next_layer_name()); let id = d.alloc_id(); Node::layer(id, &name, LayerColor::Preset((n % LAYER_COLORS.len()) as u8)) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 132ec54c..7f8adc4f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -570,3 +570,20 @@ fn a_panicking_command_rolls_back_instead_of_crashing() { assert!(s.doc().unwrap().doc.node(b).is_some()); assert_eq!(s.journal.last().map(|j| j.0.as_str()), Some("shape.rectangle")); } + +/// New layers and sublayers share one "Layer N" numbering, whatever art the document holds, and +/// never repeat a name. +#[test] +fn new_layers_and_sublayers_are_numbered_together() { + let name = |s: &Session, id: &Value| s.doc().unwrap().doc.node(NodeId(id["id"].as_u64().unwrap())).unwrap().display_name().to_string(); + let mut s = session(); + let sub = s.execute("layer.newSublayer", &json!({})).unwrap(); + assert_eq!(name(&s, &sub), "Layer 2", "not its parent's name"); + for x in [0.0, 60.0, 120.0, 180.0, 240.0] { + rect(&mut s, x, 0.0, 50.0, 50.0); + } + let layer = s.execute("layer.new", &json!({})).unwrap(); + assert_eq!(name(&s, &layer), "Layer 3", "after the sublayer, not a repeat of it"); + let sub = s.execute("layer.newSublayer", &json!({})).unwrap(); + assert_eq!(name(&s, &sub), "Layer 4", "the art doesn't count"); +}