From 02cdb15cfa1df71e06d6956fce8547fdb8e7e189 Mon Sep 17 00:00:00 2001
From: Nathaniel Ledford <nate@nateledford.com>
Date: Fri, 9 Oct 2026 16:43:39 -0400
Subject: [PATCH] LRC-LIB-KEYWORDS: keyword list files read as meant, or say
 why not

- Classic Mac line ends (a lone carriage return) are lines: a whole
  file used to become one keyword holding them.
- Indenting with spaces is refused at its line: the levels are tabs,
  and space-indented files (hand edits, editors that turn tabs into
  spaces) used to come in flat without a word.
- Control characters in a name, and "|" in a synonym, are refused.
- A keyword repeated in another case keeps its first spelling for the
  keywords inside it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 crates/catalog/src/keywords.rs | 43 +++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/crates/catalog/src/keywords.rs b/crates/catalog/src/keywords.rs
index 9c27db54e..2478724c5 100644
--- a/crates/catalog/src/keywords.rs
+++ b/crates/catalog/src/keywords.rs
@@ -63,21 +63,34 @@ pub const MAX_LEVELS: usize = 64;
 pub fn parse_keyword_list(text: &str) -> Result<Vec<(String, KeywordInfo)>> {
     let bad = |n: usize, why: &str| CatalogError::Invalid(format!("line {n}: {why}"));
     let text = text.strip_prefix('\u{feff}').unwrap_or(text);
+    // Windows, Unix and classic Mac line ends alike
+    let text = text.replace("\r\n", "\n").replace('\r', "\n");
     let mut out: Vec<(String, KeywordInfo)> = Vec::new();
     // the keyword at each level above the line: its path and where it is in `out`
     let mut above: Vec<(String, usize)> = Vec::new();
-    for (i, line) in text.lines().enumerate() {
+    for (i, line) in text.split('\n').enumerate() {
         let n = i + 1;
         if line.trim().is_empty() {
             continue;
         }
         let depth = line.chars().take_while(|c| *c == '\t').count();
-        let body = line.trim_start_matches('\t').trim();
+        let rest = line.trim_start_matches('\t');
+        // levels are tabs: spaces would flatten the hierarchy without a word
+        if rest.starts_with(' ') {
+            return Err(bad(n, "keywords are indented with tabs, one per level, not spaces"));
+        }
+        if rest.chars().any(char::is_control) {
+            return Err(bad(n, "a control character in a keyword"));
+        }
+        let body = rest.trim();
         if let Some(synonym) = body.strip_prefix('{').and_then(|b| b.strip_suffix('}')) {
             let Some(at) = depth.checked_sub(1).and_then(|d| above.get(d)).map(|(_, at)| *at) else {
                 return Err(bad(n, "a synonym (in braces) goes one tab under its keyword"));
             };
             let synonym = synonym.trim();
+            if synonym.contains(SEP) {
+                return Err(bad(n, "“|” separates a keyword's levels: a synonym can't hold one"));
+            }
             if let Some((_, info)) = out.get_mut(at)
                 && !synonym.is_empty()
                 && !info.synonyms.iter().any(|s| same(s, synonym))
@@ -117,7 +130,9 @@ pub fn parse_keyword_list(text: &str) -> Result<Vec<(String, KeywordInfo)>> {
         if left_out && let Some((_, info)) = out.get_mut(at) {
             info.include_on_export = false;
         }
-        above.push((path, at));
+        // as first spelled: the keywords inside a repeat take that spelling
+        let spelled = out.get(at).map_or(path, |(p, _)| p.clone());
+        above.push((spelled, at));
     }
     Ok(out)
 }
@@ -1224,6 +1239,28 @@ mod tests {
         assert!(err(&deep).contains("line 65"), "a file is input: 64 levels at most");
     }
 
+    /// Files from elsewhere read as they were meant or say why not: classic Mac line ends (a lone
+    /// carriage return) are lines; indenting with spaces, which would flatten the hierarchy, is
+    /// refused at its line; so are control characters in a name and "|" in a synonym.
+    #[test]
+    fn a_keyword_list_from_elsewhere_reads_or_says_why_not() {
+        let paths = |text: &str| parse_keyword_list(text).unwrap().into_iter().map(|(p, _)| p).collect::<Vec<_>>();
+        assert_eq!(paths("Events\r\tWeddings\rPlaces\r"), ["Events", "Events|Weddings", "Places"]);
+        let err = |text: &str| parse_keyword_list(text).unwrap_err().to_string();
+        assert!(err("Events\n    Weddings\n").contains("line 2"), "space indentation");
+        assert!(err("Events\n\t Weddings\n").contains("line 2"), "a space after the tabs");
+        assert!(err("Events\nx\u{0}y\n").contains("line 2"), "a control character");
+        assert!(err("Events\n\t{sea|shore}\n").contains("line 2"), "“|” in a synonym");
+    }
+
+    /// A keyword repeated in another case is one keyword, and the keywords inside either take the
+    /// first spelling.
+    #[test]
+    fn a_repeated_keyword_keeps_its_first_spelling() {
+        let read = parse_keyword_list("Events\n\tWeddings\nevents\n\tBirthdays\n").unwrap();
+        assert_eq!(read.iter().map(|(p, _)| p.as_str()).collect::<Vec<_>>(), ["Events", "Events|Weddings", "Events|Birthdays"]);
+    }
+
     /// Importing a list adds the keywords the library doesn't have, with their attributes, and gives
     /// those it has the list's synonyms (their own attributes stay), in one undo step.
     #[test]