LRC-LIB-KEYWORDS: the default parent belongs to its keyword in the library

"Put New Keywords Inside This Keyword" was a session preference that
followed renames by hand: undo and redo never moved it back, so after
undoing a rename it named a keyword that no longer existed and the
next new keyword silently made that parent again. It is now a mark on
the keyword in the catalog's keyword list (`new_keywords_inside`, one
keyword at most): it moves with its keyword (into a merge too), goes
with it, comes back with undo and redo, and is kept with the library.
Setting it is one undo step.

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

crates/catalog/src/keywords.rs

@@ -74,6 +74,10 @@ pub struct KeywordInfo {
     /// The keyword names a person.
     #[serde(default)]
     pub person: bool,
+    /// New keywords go inside this one (Put New Keywords Inside This Keyword): one keyword at
+    /// most, see [`Catalog::default_keyword_parent`].
+    #[serde(default, skip_serializing_if = "std::ops::Not::not")]
+    pub new_keywords_inside: bool,
 }
 
 fn yes() -> bool {
@@ -82,7 +86,14 @@ fn yes() -> bool {
 
 impl Default for KeywordInfo {
     fn default() -> Self {
-        KeywordInfo { synonyms: Vec::new(), include_on_export: true, export_containing: true, export_synonyms: true, person: false }
+        KeywordInfo {
+            synonyms: Vec::new(),
+            include_on_export: true,
+            export_containing: true,
+            export_synonyms: true,
+            person: false,
+            new_keywords_inside: false,
+        }
     }
 }
 
@@ -215,6 +226,7 @@ impl Catalog {
                 let path = reparent(&l.path, f, to);
                 match list.get_mut(&path.to_lowercase()) {
                     Some(there) => {
+                        there.info.new_keywords_inside |= l.info.new_keywords_inside;
                         for s in l.info.synonyms {
                             if !there.info.synonyms.iter().any(|x| same(x, &s)) {
                                 there.info.synonyms.push(s);
@@ -323,6 +335,31 @@ impl Catalog {
         self.keyword_path(path).is_some()
     }
 
+    /// The keyword new keywords go inside (Put New Keywords Inside This Keyword), if any.
+    pub fn default_keyword_parent(&self) -> Option<String> {
+        self.keyword_list.values().find(|l| l.info.new_keywords_inside).map(|l| l.path.clone())
+    }
+
+    /// Make `keyword` the one new keywords go inside (`None`: none) — one batch. It is listed if it
+    /// wasn't, keeping the attributes it has.
+    pub fn set_default_parent_ops(&self, keyword: Option<&str>) -> Result<Op> {
+        let path = match keyword {
+            Some(k) => Some(self.keyword_path(k).ok_or_else(|| CatalogError::Invalid(format!("no keyword “{}”", clean(k))))?),
+            None => None,
+        };
+        Ok(Op::Batch {
+            ops: self.list_ops(|list| {
+                for l in list.values_mut() {
+                    l.info.new_keywords_inside = false;
+                }
+                if let Some(path) = path {
+                    let l = list.entry(path.to_lowercase()).or_insert_with(|| ListedKeyword { path, info: KeywordInfo::default() });
+                    l.info.new_keywords_inside = true;
+                }
+            }),
+        })
+    }
+
     /// The keyword as written in the library (whatever the case of `path`): its listing, or the
     /// levels a photo's keyword spells it with. `None` when there is no such keyword.
     pub fn keyword_path(&self, path: &str) -> Option<String> {
@@ -859,6 +896,34 @@ mod tests {
         assert_eq!(merged(["a", "a|b"]), ["x|c", "x|d"]);
     }
 
+    /// One keyword at most is where new keywords go (Put New Keywords Inside This Keyword). Making
+    /// another one so is one undo step; the mark moves with its keyword (also into a merge) and
+    /// goes with it.
+    #[test]
+    fn one_keyword_is_where_new_keywords_go() {
+        let (mut c, _) = lib(&[&["Events|Weddings"], &["Travel"]]);
+        let op = c.set_default_parent_ops(Some("events")).unwrap();
+        let undo = c.apply(op).unwrap();
+        assert_eq!(c.default_keyword_parent().as_deref(), Some("Events"));
+        let op = c.set_default_parent_ops(Some("travel")).unwrap();
+        c.apply(op).unwrap();
+        assert_eq!(c.default_keyword_parent().as_deref(), Some("Travel"), "only one");
+        assert!(c.set_default_parent_ops(Some("lisbon")).is_err());
+        let op = c.rename_keyword_ops("travel", "Trips").unwrap();
+        c.apply(op).unwrap();
+        assert_eq!(c.default_keyword_parent().as_deref(), Some("Trips"));
+        let op = c.merge_keywords_ops(&["trips".into()], "Events").unwrap();
+        c.apply(op).unwrap();
+        assert_eq!(c.default_keyword_parent().as_deref(), Some("Events"), "into the merge");
+        let op = c.delete_keyword_ops("events").unwrap();
+        c.apply(op).unwrap();
+        assert_eq!(c.default_keyword_parent(), None);
+        let op = c.set_default_parent_ops(None).unwrap();
+        assert_eq!(op, Op::Batch { ops: vec![] }, "none to clear");
+        c.apply(undo).unwrap();
+        assert_eq!(c.default_keyword_parent(), None, "undo of the first: none was");
+    }
+
     #[test]
     fn suggestions_rank_co_occurrence_and_prefixes() {
         let (c, _) = lib(&[&["dog", "park"], &["dog", "park", "ball"], &["dog", "beach"], &["cat"], &["cat"], &["cat"], &["parade"], &["eagle"]]);

crates/engine/src/cmd/keywords.rs

@@ -15,8 +15,8 @@ fn strs(p: &Value, key: &str) -> Vec<String> {
     }
 }
 
-/// Commit a keyword batch; returns how many changes it made. The keyword filter and the default
-/// parent follow a renamed keyword and are cleared when their keyword is deleted.
+/// Commit a keyword batch; returns how many changes it made. The keyword filter follows a renamed
+/// keyword and is cleared when its keyword is deleted.
 fn commit_keywords(s: &mut Session, label: &str, op: lightcraft_catalog::Op, follow: impl Fn(&str) -> Option<String>) -> Result<Value> {
     let n = match &op {
         lightcraft_catalog::Op::Batch { ops } => ops.len(),
@@ -28,13 +28,6 @@ fn commit_keywords(s: &mut Session, label: &str, op: lightcraft_catalog::Op, fol
     if let Some(k) = s.filter.keyword.clone() {
         s.filter.keyword = follow(&k);
     }
-    if let Some(k) = s.keyword_parent.clone() {
-        let followed = follow(&k).filter(|k| s.catalog.has_keyword(k));
-        if followed.as_deref() != Some(k.as_str()) {
-            s.keyword_parent = followed;
-            let _ = s.save_prefs();
-        }
-    }
     Ok(json!({"changed": n}))
 }
 
@@ -50,6 +43,7 @@ fn info_from(p: &Value, base: KeywordInfo) -> KeywordInfo {
         export_containing: bool_or(p, "exportContaining", base.export_containing),
         export_synonyms: bool_or(p, "exportSynonyms", base.export_synonyms),
         person: bool_or(p, "person", base.person),
+        new_keywords_inside: base.new_keywords_inside,
     }
 }
 
@@ -254,7 +248,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 let parent = match p.get("parent") {
                     Some(Value::String(k)) => Some(s.catalog.keyword_path(k).unwrap_or_else(|| clean(k))),
                     Some(_) => None,
-                    None => s.keyword_parent.clone(),
+                    None => s.catalog.default_keyword_parent(),
                 };
                 let path = clean(&match parent {
                     Some(parent) => format!("{parent}|{name}"),
@@ -340,12 +334,13 @@ pub fn specs() -> Vec<CommandSpec> {
             "{keyword: keyword | null} — the parent keyword.create puts new keywords inside (null: the top level)",
             always,
             |s, p| {
-                s.keyword_parent = match p.get("keyword") {
+                let keyword = match p.get("keyword") {
                     Some(Value::Null) | None => None,
                     Some(_) => Some(named_keyword(s, p, "keyword.setDefaultParent")?),
                 };
-                let _ = s.save_prefs();
-                Ok(json!({"keyword": s.keyword_parent}))
+                let op = s.catalog.set_default_parent_ops(keyword.as_deref()).map_err(|e| bad("keyword.setDefaultParent", e.to_string()))?;
+                commit_keywords(s, "Put New Keywords Inside", op, |k| Some(k.to_string()))?;
+                Ok(json!({"keyword": s.catalog.default_keyword_parent()}))
             }
         ),
         cmd!(

crates/engine/src/lib.rs

@@ -247,8 +247,6 @@ pub struct Session {
     pub keyword_sets: Vec<cmd::keywords::KeywordSet>,
     pub keyword_set: Option<String>,
     pub recent_keywords: Vec<String>,
-    /// "Put new keywords inside this keyword": the parent `keyword.create` uses unless told.
-    pub keyword_parent: Option<String>,
     /// Before/After: the "before" settings chosen per photo (this session; default: the photo's
     /// import state). See `cmd/before.rs`.
     pub before: std::collections::HashMap<PhotoId, Arc<DevelopSettings>>,
@@ -334,7 +332,6 @@ impl Session {
             keyword_sets: Vec::new(),
             keyword_set: None,
             recent_keywords: Vec::new(),
-            keyword_parent: None,
             before: Default::default(),
             import_probes: Default::default(),
             preview_build: None,

crates/engine/src/library.rs

@@ -149,8 +149,6 @@ struct PrefsFile {
     keyword_sets: Vec<crate::cmd::keywords::KeywordSet>,
     keyword_set: Option<String>,
     recent_keywords: Vec<String>,
-    /// The parent new keywords go inside.
-    keyword_parent: Option<String>,
     /// Develop defaults for imported photos.
     import: crate::import::ImportDefaults,
     /// Thumbnail disk cache budget (MB, 0 = default).
@@ -340,7 +338,6 @@ impl Session {
         self.keyword_sets = prefs.keyword_sets;
         self.keyword_set = prefs.keyword_set;
         self.recent_keywords = prefs.recent_keywords;
-        self.keyword_parent = prefs.keyword_parent;
         self.import_defaults = prefs.import;
         self.cache_mb = prefs.cache_mb;
         self.forget_local_days = prefs.forget_local_days.unwrap_or(lightcraft_catalog::DEFAULT_FORGET_DAYS);
@@ -570,7 +567,6 @@ impl Session {
             keyword_sets: self.keyword_sets.clone(),
             keyword_set: self.keyword_set.clone(),
             recent_keywords: self.recent_keywords.clone(),
-            keyword_parent: self.keyword_parent.clone(),
             import: self.import_defaults.clone(),
             cache_mb: self.cache_mb,
             smart_previews_dir: self.smart_previews_dir.as_ref().map(|d| d.to_string_lossy().to_string()),

crates/engine/src/tests_keyword_list.rs

@@ -55,8 +55,8 @@ fn creating_a_keyword_with_attributes_and_the_selection() {
 }
 
 /// "Put new keywords inside this keyword": new keywords go inside the default parent unless the
-/// command says otherwise (`parent: null` = the top level). It follows the parent's rename, goes
-/// when the parent is deleted, and is kept with the library.
+/// command says otherwise (`parent: null` = the top level). It belongs to its keyword in the
+/// library: it follows a rename, goes with a delete, and undo and redo bring it back with them.
 #[test]
 fn new_keywords_go_inside_the_default_parent() {
     let dir = temp_dir("default-parent");
@@ -64,17 +64,26 @@ fn new_keywords_go_inside_the_default_parent() {
     s.open_library(&dir, true).unwrap();
     s.execute("keyword.create", &json!({"name": "Events"})).unwrap();
     s.execute("keyword.setDefaultParent", &json!({"keyword": "events"})).unwrap();
-    assert_eq!(s.keyword_parent.as_deref(), Some("Events"));
+    assert_eq!(s.catalog.default_keyword_parent().as_deref(), Some("Events"));
     assert_eq!(s.execute("keyword.create", &json!({"name": "Birthdays"})).unwrap()["keyword"], "Events|Birthdays");
     assert_eq!(s.execute("keyword.create", &json!({"name": "Travel", "parent": null})).unwrap()["keyword"], "Travel");
     s.execute("keyword.rename", &json!({"from": "Events", "to": "Occasions"})).unwrap();
-    assert_eq!(s.keyword_parent.as_deref(), Some("Occasions"), "follows the rename");
+    assert_eq!(s.catalog.default_keyword_parent().as_deref(), Some("Occasions"), "follows the rename");
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert_eq!(s.catalog.default_keyword_parent().as_deref(), Some("Events"), "undo takes it back");
+    s.execute("edit.redo", &json!({})).unwrap();
     drop(s);
     let mut s = Session::new();
     s.open_library(&dir, false).unwrap();
-    assert_eq!(s.keyword_parent.as_deref(), Some("Occasions"), "kept with the library");
+    assert_eq!(s.catalog.default_keyword_parent().as_deref(), Some("Occasions"), "kept with the library");
     s.execute("keyword.delete", &json!({"keyword": "occasions"})).unwrap();
-    assert_eq!(s.keyword_parent, None, "gone with its keyword");
+    assert_eq!(s.catalog.default_keyword_parent(), None, "gone with its keyword");
+    assert_eq!(s.execute("keyword.create", &json!({"name": "Graduations"})).unwrap()["keyword"], "Graduations", "at the top level");
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert_eq!(s.catalog.default_keyword_parent().as_deref(), Some("Occasions"), "undoing the delete brings it back");
+    s.execute("keyword.setDefaultParent", &json!({"keyword": null})).unwrap();
+    assert_eq!(s.catalog.default_keyword_parent(), None);
     assert!(s.execute("keyword.setDefaultParent", &json!({"keyword": "lisbon"})).is_err(), "no such keyword");
     let _ = std::fs::remove_dir_all(&dir);
 }

crates/ui-egui/src/panels/keyword_list.rs

@@ -215,7 +215,7 @@ fn row(app: &mut LightcraftApp, ui: &mut egui::Ui, r: &Row, selection: &[PhotoId
     let name = if measure(&r.name) <= room { r.name.clone() } else { crate::widgets::elide_head(&r.name, room, measure) };
     let label = ui.painter().text(pos2(x + 34.0, cy), Align2::LEFT_CENTER, name, font.clone(), if picked { t.text } else { t.text_label });
     // where new keywords go (Put New Keywords Inside This Keyword): a dot after the name
-    if app.session.keyword_parent.as_deref().is_some_and(|k| same(k, &r.path)) {
+    if app.session.catalog.default_keyword_parent().as_deref().is_some_and(|k| same(k, &r.path)) {
         let dot = Rect::from_center_size(pos2(label.right() + 7.0, cy), vec2(6.0, 6.0));
         ui.painter().circle_filled(dot.center(), 3.0, t.accent);
         register(ui.ctx(), format!("keywordDefault:{}", r.path), dot);
@@ -288,7 +288,7 @@ fn menu(app: &mut LightcraftApp, ui: &mut egui::Ui, path: &str, selection: &[Pho
     if item(ui, "edit", crate::i18n::tr("Edit Keyword Tag…"), true) {
         app.ui.dialog = Some(edit_dialog(app, path));
     }
-    let is_default = app.session.keyword_parent.as_deref().is_some_and(|k| same(k, path));
+    let is_default = app.session.catalog.default_keyword_parent().as_deref().is_some_and(|k| same(k, path));
     let label = format!("{}{}", if is_default { "✓ " } else { "" }, crate::i18n::tr("Put New Keywords Inside This Keyword"));
     if item(ui, "defaultParent", &label, true) {
         let _ = app.run("keyword.setDefaultParent", json!({"keyword": if is_default { serde_json::Value::Null } else { json!(path) }}));
@@ -316,7 +316,8 @@ fn menu(app: &mut LightcraftApp, ui: &mut egui::Ui, path: &str, selection: &[Pho
 
 /// Create Keyword Tag: inside the keyword picked in the list, else the default parent.
 pub(crate) fn create_dialog(app: &LightcraftApp) -> Dialog {
-    let parent = app.ui.keyword_list_selected.clone().filter(|k| app.session.catalog.has_keyword(k)).or_else(|| app.session.keyword_parent.clone());
+    let parent =
+        app.ui.keyword_list_selected.clone().filter(|k| app.session.catalog.has_keyword(k)).or_else(|| app.session.catalog.default_keyword_parent());
     let d = KeywordInfo::default();
     Dialog::KeywordTag {
         editing: None,

crates/ui-egui/src/tests_keyword_list.rs

@@ -195,7 +195,7 @@ fn a_keywords_menu_offers_its_actions() {
         assert!(has(&h, &format!("keywordMenu:{item}")), "{item}");
     }
     ask(&mut h, "ui.clickWidget", json!({"id": "keywordMenu:defaultParent"}));
-    assert_eq!(h.app.session.keyword_parent.as_deref(), Some("Events"));
+    assert_eq!(h.app.session.catalog.default_keyword_parent().as_deref(), Some("Events"));
     assert!(has(&h, "keywordDefault:Events"), "the list marks where new keywords go");
     // Create inside it
     ask(&mut h, "ui.clickWidget", json!({"id": "keywordRow:Events", "button": "right"}));
@@ -207,7 +207,7 @@ fn a_keywords_menu_offers_its_actions() {
     ask(&mut h, "ui.clickWidget", json!({"id": "keywordRow:Events", "button": "right"}));
     ask(&mut h, "ui.clickWidget", json!({"id": "keywordMenu:purge"}));
     assert!(!h.app.session.catalog.has_keyword("Events"));
-    assert_eq!(h.app.session.keyword_parent, None, "gone with it");
+    assert_eq!(h.app.session.catalog.default_keyword_parent(), None, "gone with it");
 }
 
 fn center(h: &Headless, id: &str) -> egui::Pos2 {