Lasso: refuse a history name that shows nothing or hides formatting

A historyName of zero-width, filler or BOM characters showed an empty History
row, and a bidi control or line separator could make a row read differently
from its name. A history name now has no control or line-break character
anywhere, loses white space and ZWNJ/ZWJ at its ends, and is then held to the
plug-in menu-label rule, now shared as
photocraft_plugins::manifest::printable_name (1-64 characters with a visible
one, no hidden format characters; plug-in names also refuse U+2028/U+2029).

crates/engine/src/cmd/retouch_cmds/patch_history_names_tests.rs

@@ -51,10 +51,41 @@ fn the_lasso_is_named_for_the_tool_that_drew_it() {
 fn a_hostile_history_name_is_refused_without_a_step() {
     let mut s = session();
     let steps = s.active().unwrap().history.past_len();
-    for name in [json!(""), json!("   "), json!("x".repeat(65)), json!("a\nb"), json!(7), json!(["Patch Tool"])] {
+    let invisible = ["\u{200B}", " \u{200B}\u{FEFF} ", "\u{3164}", "\u{FE0F}", "\u{E0020}", "\u{00AD}"];
+    let hidden = ["Patch\u{202E}looT", "\u{FEFF}Patch Tool", "Patch Tool\u{200B}", "\u{2066}Patch", "Patch\u{2028}Tool", "a\u{2029}b"];
+    let ends = ["Patch Tool\n", "\r\nPatch Tool", "\tPatch", "\u{0085}Patch", "Patch\u{2028}"];
+    let others = [json!(""), json!("   "), json!("x".repeat(65)), json!("a\nb"), json!(7), json!(["Patch Tool"])];
+    let others = ends.iter().map(|n| json!(n)).chain(others);
+    for name in invisible.iter().chain(&hidden).map(|n| json!(n)).chain(others) {
         assert!(s.execute("select.lasso", json!({"points": TRIANGLE, "historyName": name})).is_err(), "{name}");
         let polygonal = json!({"points": TRIANGLE, "polygonal": true, "historyName": name});
         assert!(s.execute("select.lasso", polygonal).is_err(), "polygonal {name}");
     }
     assert_eq!(s.active().unwrap().history.past_len(), steps);
 }
+
+#[test]
+fn a_name_joining_its_letters_with_a_zero_width_joiner_is_kept() {
+    let mut s = session();
+    for name in ["\u{0645}\u{200C}\u{06CC}", "\u{1F469}\u{200D}\u{1F4BB}", "Patch Tool \u{2713}"] {
+        s.execute("select.lasso", json!({"points": TRIANGLE, "historyName": name})).unwrap();
+        assert_eq!(last_step(&s).as_deref(), Some(name));
+    }
+}
+
+#[test]
+fn spaces_and_joiners_at_the_ends_are_trimmed_and_only_what_is_left_counts() {
+    let mut s = session();
+    let padded = format!("{}{}", " ".repeat(10), "x".repeat(60));
+    let joined = format!("Patch Tool{}", "\u{200D}".repeat(60));
+    let cases = [
+        ("Patch Tool\u{200D}", "Patch Tool"),
+        (" \u{200C}Patch Tool ", "Patch Tool"),
+        ("Love \u{2764}\u{FE0F}", "Love \u{2764}\u{FE0F}"),
+        (joined.as_str(), "Patch Tool"),
+    ];
+    for (name, stored) in cases.iter().copied().chain([(padded.as_str(), &padded[10..])]) {
+        s.execute("select.lasso", json!({"points": TRIANGLE, "historyName": name})).unwrap();
+        assert_eq!(last_step(&s).as_deref(), Some(stored), "{name:?}");
+    }
+}

crates/engine/src/cmd/selection_cmds.rs

@@ -627,13 +627,27 @@ fn lasso(s: &mut Session, p: &Value) -> Result<Value> {
     // A bad name is refused even when `polygonal` picks the step's name.
     let named = match p.get("historyName") {
         None | Some(Value::Null) => None,
-        Some(Value::String(n)) if !n.trim().is_empty() && n.chars().count() <= 64 && !n.chars().any(char::is_control) => Some(n.trim()),
-        Some(_) => return Err(EngineError::BadParams { cmd: "select.lasso".into(), msg: "historyName must be a short text".into() }),
+        Some(Value::String(n)) if history_name(n).is_some() => history_name(n),
+        Some(_) => {
+            let msg = "historyName must be 1-64 characters with a visible one, and no control, line-break or hidden format characters";
+            return Err(EngineError::BadParams { cmd: "select.lasso".into(), msg: msg.into() });
+        }
     };
     let step = if b(p, "polygonal", false) { "Polygonal Lasso" } else { named.unwrap_or("Lasso") };
     polygon_selection(s, "select.lasso", step, &pts, p)
 }
 
+/// The History step name for `historyName` `n`: refused when it has a control or line-break
+/// character anywhere; otherwise without white space or zero-width (non-)joiners at its ends, so a
+/// name that looks like "Patch Tool" is "Patch Tool", and then held to the plug-in menu-label rule.
+fn history_name(n: &str) -> Option<&str> {
+    if n.chars().any(|c| c.is_control() || matches!(c, '\u{2028}' | '\u{2029}')) {
+        return None;
+    }
+    let t = n.trim_matches(|c: char| c.is_whitespace() || matches!(c, '\u{200C}' | '\u{200D}'));
+    photocraft_plugins::manifest::printable_name(t).then_some(t)
+}
+
 /// Selects the polygon `pts` with the lasso tools' `mode`, `antiAlias` and `feather` params, as
 /// one history step called `label`.
 pub(crate) fn polygon_selection(s: &mut Session, cmd: &str, label: &str, pts: &[(f32, f32)], p: &Value) -> Result<Value> {
@@ -764,7 +778,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "select.lasso",
             "Lasso",
             [],
-            r##"{"points":[[x,y],…],"mode":"replace|add|subtract|intersect"="replace","antiAlias":bool=true,"feather":px=0,"polygonal":bool=false,"historyName"?:text (1..64 characters, no control characters; the History step name when a tool other than the Lasso draws it, e.g. "Patch Tool"; default "Lasso")}"##,
+            r##"{"points":[[x,y],…],"mode":"replace|add|subtract|intersect"="replace","antiAlias":bool=true,"feather":px=0,"polygonal":bool=false,"historyName"?:text (1..64 characters with a visible one, no control, line-break or hidden format characters, spaces and joiners at the ends trimmed; the History step name when a tool other than the Lasso draws it, e.g. "Patch Tool"; default "Lasso")}"##,
             has_doc,
             lasso
         ),

crates/plugins/src/manifest.rs

@@ -143,9 +143,7 @@ impl Manifest {
         if !valid_id(&self.id) {
             return Err(Error::Manifest(format!("invalid id {:?}: use 1-64 characters from A-Z a-z 0-9 . _ -", self.id)));
         }
-        let name = self.name.trim();
-        let printable = name.chars().all(|c| !c.is_control() && !hidden_format(c)) && name.chars().any(visible);
-        if !printable || name.chars().count() > 64 {
+        if !printable_name(self.name.trim()) {
             return Err(Error::Manifest("`name` must be 1-64 printable characters".into()));
         }
         if self.version.len() > 32 || self.description.len() > 1024 || self.author.len() > 128 {
@@ -399,6 +397,12 @@ fn hidden_format(c: char) -> bool {
     !matches!(c, '\u{200C}' | '\u{200D}') && in_ranges(c, FORMAT)
 }
 
+/// Whether `name` (already trimmed) can be shown as a label: 1-64 characters, at least one of them
+/// visible, and no control characters, line or paragraph separators, or hidden format characters.
+pub fn printable_name(name: &str) -> bool {
+    name.chars().count() <= 64 && name.chars().any(visible) && !name.chars().any(|c| c.is_control() || matches!(c, '\u{2028}' | '\u{2029}') || hidden_format(c))
+}
+
 /// A character that draws something on its own.
 fn visible(c: char) -> bool {
     !c.is_whitespace() && !c.is_control() && !in_ranges(c, FORMAT) && !in_ranges(c, BLANK)
@@ -631,6 +635,8 @@ mod tests {
             "\u{3164}\u{FE0F}",
             " \u{3000} ",
             "Tone\u{7}",
+            "Tone\u{2028}Pro",
+            "Tone\u{2029}Pro",
         ] {
             assert!(with_name(bad).is_err(), "{bad:?}");
         }