From 6b4839f5fc1337a940872a75df28f79002163f7b Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 15:55:15 -0400 Subject: [PATCH] LRC-LIB-KEYWORDS: keyword sets keep their slots, and can be renamed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- crates/engine/src/cmd/keywords.rs | 51 +++++++++++++++++++------ crates/engine/src/tests_keyword_list.rs | 46 ++++++++++++++++++++++ 2 files changed, 86 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/keywords.rs b/crates/engine/src/cmd/keywords.rs index a3862b3b4..f167c5122 100644 --- a/crates/engine/src/cmd/keywords.rs +++ b/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 { + let mut out: Vec = 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 { 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 { "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 { .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 = 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 { .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); diff --git a/crates/engine/src/tests_keyword_list.rs b/crates/engine/src/tests_keyword_list.rs index 3084fc122..cfff1f4f6 100644 --- a/crates/engine/src/tests_keyword_list.rs +++ b/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 = 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"); +}