LRC-LIB-KEYWORDS: keyword sets keep their slots, and can be renamed

For Edit Keyword Set (Lightroom Classic's), `keyword.saveSet`:
- keeps each slot in its place: an empty one ("") is an empty slot, so
  the keywords after it keep their ⌥ keys (empty ones were dropped and
  the rest moved up); trailing empty slots go, nine at most;
- holds each keyword once, whatever its case: a second one leaves its
  slot empty;
- with `replace`, renames that set in its place among the sets and
  keeps it current; a name another set has is refused.
⌥ on an empty slot does nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/keywords.rs

@@ -89,6 +89,22 @@ pub fn note_recent(s: &mut Session, added: &[String]) {
     let _ = s.save_prefs();
 }
 
+/// A set's slots as typed: each cleaned in its place (an empty one is an empty slot, so the others
+/// keep their ⌥ keys), each keyword once whatever its case (a second one leaves its slot empty),
+/// nine at most, no empty ones at the end.
+pub fn slots(typed: &[String]) -> Vec<String> {
+    let mut out: Vec<String> = Vec::new();
+    for k in typed.iter().take(9) {
+        let k = clean(k);
+        let twice = !k.is_empty() && out.iter().any(|x| lightcraft_catalog::keywords::same(x, &k));
+        out.push(if twice { String::new() } else { k });
+    }
+    while out.last().is_some_and(String::is_empty) {
+        out.pop();
+    }
+    out
+}
+
 /// The nine keywords ⌥1–⌥9 apply: the current set's, or the recent ones.
 pub fn current_keywords(s: &Session) -> Vec<String> {
     let set = s.keyword_set.as_deref().and_then(|n| s.keyword_sets.iter().find(|x| x.name.eq_ignore_ascii_case(n)));
@@ -129,7 +145,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Save Keyword Set",
             [],
             None,
-            "{name, keywords?: [up to 9] (default: the current nine)} — replaces a set of that name and makes it current",
+            "{name, keywords?: [up to 9, \"\" = an empty slot] (default: the current nine), replace?: the set it renames} — replaces a set of that name (or `replace`, in its place) and makes it current",
             always,
             |s, p| {
                 let name = str_param(p, "name")
@@ -137,16 +153,28 @@ pub fn specs() -> Vec<CommandSpec> {
                     .filter(|n| !n.is_empty() && !n.eq_ignore_ascii_case(RECENT))
                     .ok_or_else(|| bad("keyword.saveSet", "missing or reserved `name`"))?
                     .to_string();
-                let mut keywords: Vec<String> = if p.get("keywords").is_some() {
-                    strs(p, "keywords").iter().map(|k| clean(k)).filter(|k| !k.is_empty()).collect()
-                } else {
-                    current_keywords(s)
-                };
-                keywords.truncate(9);
+                let keywords = if p.get("keywords").is_some() { slots(&strs(p, "keywords")) } else { current_keywords(s) };
                 let set = KeywordSet { name: name.clone(), keywords };
-                match s.keyword_sets.iter_mut().find(|x| x.name.eq_ignore_ascii_case(&name)) {
-                    Some(x) => *x = set,
-                    None => s.keyword_sets.push(set),
+                let same_name = |x: &KeywordSet, n: &str| x.name.to_lowercase() == n.to_lowercase();
+                match str_param(p, "replace").map(str::trim) {
+                    // rename: in its place, onto a name no other set has
+                    Some(old) => {
+                        let at = s
+                            .keyword_sets
+                            .iter()
+                            .position(|x| same_name(x, old))
+                            .ok_or_else(|| bad("keyword.saveSet", format!("no keyword set `{old}`")))?;
+                        if s.keyword_sets.iter().enumerate().any(|(i, x)| i != at && same_name(x, &name)) {
+                            return Err(bad("keyword.saveSet", format!("there is a keyword set “{name}” already")));
+                        }
+                        if let Some(x) = s.keyword_sets.get_mut(at) {
+                            *x = set;
+                        }
+                    }
+                    None => match s.keyword_sets.iter_mut().find(|x| same_name(x, &name)) {
+                        Some(x) => *x = set,
+                        None => s.keyword_sets.push(set),
+                    },
                 }
                 s.keyword_set = Some(name);
                 s.save_prefs()?;
@@ -179,7 +207,8 @@ pub fn specs() -> Vec<CommandSpec> {
                     .and_then(Value::as_u64)
                     .filter(|i| (1..=9).contains(i))
                     .ok_or_else(|| bad("keyword.toggleFromSet", "`index` must be 1..9"))?;
-                let Some(k) = current_keywords(s).get(i as usize - 1).cloned() else {
+                // (an empty slot does nothing)
+                let Some(k) = current_keywords(s).get(i as usize - 1).cloned().filter(|k| !k.is_empty()) else {
                     return Ok(json!({"changed": 0}));
                 };
                 let ids = s.targets(p);

crates/engine/src/tests_keyword_list.rs

@@ -183,3 +183,49 @@ fn a_set_keyword_toggles_whatever_its_case() {
     s.execute("keyword.toggleFromSet", &json!({"index": 1, "ids": ids})).unwrap();
     assert!(ids.iter().all(|id| keywords_of(&s, *id).is_empty()), "taken off both");
 }
+
+fn set_named(s: &mut Session, name: &str) -> serde_json::Value {
+    let sets = s.execute("keyword.sets", &json!({})).unwrap();
+    sets["sets"].as_array().unwrap().iter().find(|x| x["name"] == name).cloned().unwrap_or(serde_json::Value::Null)
+}
+
+/// Edit Keyword Set: a set's nine slots keep their places, so each keyword stays on its ⌥ key; an
+/// empty slot is allowed and does nothing.
+#[test]
+fn a_keyword_sets_slots_keep_their_places() {
+    let mut s = Session::with_demo();
+    let id = s.visible_cloned()[0].0;
+    s.execute("keyword.saveSet", &json!({"name": "Weddings", "keywords": [" ceremony ", "", "reception", ""]})).unwrap();
+    assert_eq!(set_named(&mut s, "Weddings")["keywords"], json!(["ceremony", "", "reception"]), "trailing empty slots go");
+    let before = keywords_of(&s, id);
+    let r = s.execute("keyword.toggleFromSet", &json!({"index": 2, "ids": [id]})).unwrap();
+    assert_eq!((r["changed"].as_u64(), keywords_of(&s, id)), (Some(0), before), "⌥2 is empty: nothing");
+    s.execute("keyword.toggleFromSet", &json!({"index": 3, "ids": [id]})).unwrap();
+    assert!(keywords_of(&s, id).contains(&"reception".to_string()), "⌥3 is reception");
+}
+
+/// A set holds each keyword once, whatever its case: a second one leaves its slot empty.
+#[test]
+fn a_keyword_set_holds_each_keyword_once() {
+    let mut s = Session::with_demo();
+    s.execute("keyword.saveSet", &json!({"name": "Travel", "keywords": ["Lisbon", "harbour", "LISBON"]})).unwrap();
+    assert_eq!(set_named(&mut s, "Travel")["keywords"], json!(["Lisbon", "harbour"]));
+}
+
+/// Renaming a set keeps its place among the sets and keeps it current; a name another set has is
+/// refused.
+#[test]
+fn renaming_a_keyword_set_keeps_its_place() {
+    let mut s = Session::with_demo();
+    s.execute("keyword.saveSet", &json!({"name": "Weddings", "keywords": ["ceremony"]})).unwrap();
+    s.execute("keyword.saveSet", &json!({"name": "Travel", "keywords": ["harbour"]})).unwrap();
+    s.execute("keyword.useSet", &json!({"name": "Weddings"})).unwrap();
+    let r = s.execute("keyword.saveSet", &json!({"name": "Ceremonies", "replace": "weddings", "keywords": ["ceremony", "rings"]})).unwrap();
+    let names: Vec<String> = r["sets"].as_array().unwrap().iter().map(|x| x["name"].as_str().unwrap().to_string()).collect();
+    assert_eq!(names, ["Recent Keywords", "Ceremonies", "Travel"]);
+    assert_eq!(r["current"], "Ceremonies");
+    assert_eq!(r["keywords"], json!(["ceremony", "rings"]));
+    let err = s.execute("keyword.saveSet", &json!({"name": "travel", "replace": "Ceremonies", "keywords": []})).unwrap_err().to_string();
+    assert!(err.contains("already"), "{err}");
+    assert!(s.execute("keyword.saveSet", &json!({"name": "Rings", "replace": "Lisbon", "keywords": []})).is_err(), "no such set");
+}