Brushes: New Brush and Bristle Brush Options take a bristle brush name of at most 31 characters

Squashed from #3312 onto main.

crates/engine/src/cmd/brushsym.rs

@@ -10,7 +10,7 @@ use std::collections::BTreeSet;
 use std::sync::Arc;
 
 use serde_json::{Value, json};
-use vectorcraft_brush::{self as brush, Brush};
+use vectorcraft_brush::{self as brush, Bristle, Brush, BrushKind};
 use vectorcraft_color::{Color, Paint};
 use vectorcraft_doc::{AppearanceItem, Document, FillLayer, Node, NodeId, NodeKind, Symbol, SymbolExportType, SymbolNineSlice, SymbolRegistration};
 use vectorcraft_geom::{Affine, Point, Vec2};
@@ -72,7 +72,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "New Brush…",
             [],
             None,
-            "{type: calligraphic|scatter|art|pattern|bristle, name? (trimmed; blank: New <Type> Brush), params?: {…definition fields}, ids?|id?} new brush; art/scatter/pattern brushes take the selected art (pattern: side tile) unless params give it → {name}",
+            "{type: calligraphic|scatter|art|pattern|bristle, name? (trimmed; blank: New <Type> Brush; a bristle brush name, with the number a taken name gets, has at most 31 characters), params?: {…definition fields}, ids?|id?} new brush; art/scatter/pattern brushes take the selected art (pattern: side tile) unless params give it → {name}",
             has_doc,
             brush_new
         ),
@@ -117,7 +117,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Brush Options…",
             [],
             None,
-            "{name, params?: {…fields to change (brush.get shows them; values outside their ranges are clamped). calligraphic: angle −180..180°, roundness 0..100%, size 0..1296 pt, variation: [angle°, roundness%, size pt], modes: [fixed|random|pressure for angle, roundness, size] (pressure goes from value − variation at the lightest pen pressure to value + variation at the heaviest). scatter: size [a, b] 1..10000%, spacing [a, b] 1..10000%, scatter [a, b] −1000..1000%, rotation [a, b] −180..180°, modes: [fixed|random|pressure for size, spacing, scatter, rotation] (fixed: a; random: between a and b; pressure: a at the lightest pen pressure to b at the heaviest), rotation_relative_to_path. art: width 1..1000%, scale: {mode: proportional|stretch|betweenGuides, start, end: 0..1 of the art length}, direction: leftToRight|rightToLeft|topToBottom|bottomToTop, flip_along, flip_across, overlap. pattern: scale 1..10000%, spacing 0..10000%, fit: stretch|addSpace|approximate, flip_along, flip_across. scatter, art and pattern: colorization: {method: none|tints|tintsAndShades|hueShift, key: colour (hueShift: the art colour that becomes the stroke colour; without a key: the solid color the art's fills and strokes use most, black when they use none)}; colorization and the art brush's scale change only the fields given}, newName? (trimmed; blank: the name is kept)} edit a brush definition (strokes using it update) → {name}",
+            "{name, params?: {…fields to change (brush.get shows them; values outside their ranges are clamped). calligraphic: angle −180..180°, roundness 0..100%, size 0..1296 pt, variation: [angle°, roundness%, size pt], modes: [fixed|random|pressure for angle, roundness, size] (pressure goes from value − variation at the lightest pen pressure to value + variation at the heaviest). scatter: size [a, b] 1..10000%, spacing [a, b] 1..10000%, scatter [a, b] −1000..1000%, rotation [a, b] −180..180°, modes: [fixed|random|pressure for size, spacing, scatter, rotation] (fixed: a; random: between a and b; pressure: a at the lightest pen pressure to b at the heaviest), rotation_relative_to_path. art: width 1..1000%, scale: {mode: proportional|stretch|betweenGuides, start, end: 0..1 of the art length}, direction: leftToRight|rightToLeft|topToBottom|bottomToTop, flip_along, flip_across, overlap. pattern: scale 1..10000%, spacing 0..10000%, fit: stretch|addSpace|approximate, flip_along, flip_across. scatter, art and pattern: colorization: {method: none|tints|tintsAndShades|hueShift, key: colour (hueShift: the art colour that becomes the stroke colour; without a key: the solid color the art's fills and strokes use most, black when they use none)}; colorization and the art brush's scale change only the fields given}, newName? (trimmed; blank: the name is kept; a new bristle brush name has at most 31 characters, and a longer name the brush has is kept)} edit a brush definition (strokes using it update) → {name}",
             has_doc,
             brush_options
         ),
@@ -501,6 +501,9 @@ fn brush_new(s: &mut Session, p: &Value) -> Result<Value> {
     let given = str_param(p, "name").map(str::trim).filter(|n| !n.is_empty());
     let base_name = given.map_or_else(|| format!("New {} Brush", ty[..1].to_uppercase() + &ty[1..]), str::to_string);
     let name = brush::unique_name(&lib, &base_name);
+    if ty == "bristle" {
+        bristle_name_fits(C, &name)?;
+    }
     let mut v = json!({"name": name, "type": ty});
     if let Some(params) = p.get("params") {
         merge(&mut v, params);
@@ -521,6 +524,15 @@ fn brush_new(s: &mut Session, p: &Value) -> Result<Value> {
     Ok(json!({ "name": name }))
 }
 
+/// Refuses `name` for a bristle brush when it has more than [`Bristle::MAX_NAME_CHARS`] characters.
+fn bristle_name_fits(cmd: &str, name: &str) -> Result<()> {
+    let (count, max) = (name.chars().count(), Bristle::MAX_NAME_CHARS);
+    if count > max {
+        return Err(bad(cmd, format!("a bristle brush name can have at most {max} characters: `{name}` has {count}")));
+    }
+    Ok(())
+}
+
 /// A brush definition from command params, its values kept within their ranges ([`Brush::from_json`]).
 fn parse_brush(cmd: &str, v: Value) -> Result<Brush> {
     Brush::from_json(v).map_err(|e| bad(cmd, format!("invalid brush: {e}")))
@@ -659,6 +671,10 @@ fn brush_options(s: &mut Session, p: &Value) -> Result<Value> {
         if lib.iter().any(|b| &b.name == nn) {
             return Err(bad(C, format!("a brush named `{nn}` exists")));
         }
+        // A `newName` that only trims the white space around the saved name is taken at any length.
+        if matches!(lib[i].kind, BrushKind::Bristle(_)) && nn != name.trim() {
+            bristle_name_fits(C, nn)?;
+        }
         v["name"] = json!(nn);
     } else {
         v["name"] = json!(name);

crates/engine/src/tests_brushsym.rs

@@ -352,6 +352,54 @@ fn new_duplicate_and_renamed_brushes_trim_their_names() {
     assert_eq!(names[names.len() - 6..], ["Nib", " Old ", "New Calligraphic Brush", "Quill", "Nib copy", "Old copy"]);
 }
 
+/// New Brush and Bristle Brush Options take a bristle brush name of at most 31 characters, counted
+/// in Unicode characters (31 é's fit). A longer name that a document already holds is kept, and the
+/// other brush types take a name of any length.
+#[test]
+fn bristle_brush_names_have_at_most_31_characters() {
+    let mut s = session();
+    let library = |s: &Session| vectorcraft_brush::library(&s.doc().unwrap().doc);
+    let refused = |s: &mut Session, cmd: &str, p: Value| s.execute(cmd, &p).unwrap_err().to_string();
+    let (long, fits) = ("b".repeat(32), "é".repeat(31));
+    let e = refused(&mut s, "brush.new", json!({"type": "bristle", "name": long}));
+    assert!(e.contains("a bristle brush name can have at most 31 characters: `bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb` has 32"), "{e}");
+    assert!(!library(&s).iter().any(|b| b.name == long));
+    assert_eq!(s.execute("brush.new", &json!({"type": "bristle", "name": fits})).unwrap()["name"], fits.as_str());
+    // A taken name is numbered, and the numbered name is the one counted.
+    let e = refused(&mut s, "brush.new", json!({"type": "bristle", "name": fits}));
+    assert!(e.contains(&format!("`{fits} 2` has 33")), "{e}");
+    let e = refused(&mut s, "brush.options", json!({"name": "Bristle Round", "newName": long}));
+    assert!(e.contains("at most 31 characters"), "{e}");
+    let short = "c".repeat(31);
+    assert_eq!(s.execute("brush.options", &json!({"name": "Bristle Round", "newName": short})).unwrap()["name"], short.as_str());
+    // Calligraphic brushes take a longer name.
+    assert_eq!(s.execute("brush.new", &json!({"type": "calligraphic", "name": long})).unwrap()["name"], long.as_str());
+    let longer = format!("{long}x");
+    assert_eq!(s.execute("brush.options", &json!({"name": long, "newName": longer})).unwrap()["name"], longer.as_str());
+    // Bristle brushes that already have longer names, one of them with white space around it, as a
+    // saved file can hold.
+    let (old, spaced) = ("d".repeat(40), format!(" {} ", "e".repeat(40)));
+    let mut lib = library(&s);
+    lib.iter_mut().filter(|b| b.name == short).for_each(|b| b.name = old.clone());
+    lib.iter_mut().filter(|b| b.name == fits).for_each(|b| b.name = spaced.clone());
+    vectorcraft_brush::store(std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc), &lib);
+    let l = line(&mut s);
+    s.execute("brush.apply", &json!({"name": old, "ids": [l.0]})).unwrap();
+    // Brush Options without a new name, with the same name, or with the name only trimmed keep them.
+    s.execute("brush.options", &json!({"name": old, "params": {"size": 9}})).unwrap();
+    assert_eq!(s.execute("brush.options", &json!({"name": old, "newName": old, "params": {"density": 20}})).unwrap()["name"], old.as_str());
+    assert_eq!(s.execute("brush.options", &json!({"name": spaced, "newName": spaced})).unwrap()["name"], spaced.trim());
+    assert_eq!(s.execute("brush.get", &json!({"name": old})).unwrap()["size"], json!(9.0));
+    assert_eq!(s.execute("brush.duplicate", &json!({"name": old})).unwrap()["name"], format!("{old} copy"));
+    // Saved and opened again, the brush and the stroke painted with it keep the name.
+    let back = vectorcraft_format::load(&vectorcraft_format::save(&s.doc().unwrap().doc, false)).unwrap();
+    assert!(vectorcraft_brush::find(&back, &old).is_some_and(|b| b.kind.type_id() == "bristle"));
+    assert_eq!(back.node(l).unwrap().appearance.stroke().unwrap().brush.as_deref(), Some(old.as_str()));
+    let mut s2 = Session::new();
+    s2.add_document(back, None);
+    assert_eq!(s2.execute("brush.options", &json!({"name": old, "params": {"size": 4}})).unwrap()["name"], old.as_str());
+}
+
 /// The Brushes panel's multi-selection: `names` deletes or duplicates several brushes in one undo
 /// step, and an unknown name changes nothing.
 #[test]