CP4.2: review — pack licences and licence file names are plain text; missing_dictionary documents that every language is named; past 256 pack names the hint names a generic dictionary

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FypgCX8LsYYE8fsj4byBm2

crates/proof/src/hunspell.rs

@@ -174,7 +174,13 @@ pub fn candidates(names: &[&str]) -> Vec<(PathBuf, PathBuf)> {
 }
 
 /// [`candidates`] over the given folders.
-pub(crate) fn ordered(names: &[&str], dirs: &[PathBuf], region_packs: &[PathBuf], system: &[PathBuf], other_packs: &[PathBuf]) -> Vec<(PathBuf, PathBuf)> {
+pub(crate) fn ordered(
+    names: &[&str],
+    dirs: &[PathBuf],
+    region_packs: &[PathBuf],
+    system: &[PathBuf],
+    other_packs: &[PathBuf],
+) -> Vec<(PathBuf, PathBuf)> {
     let norm = |n: &str| n.to_ascii_lowercase().replace('_', "-");
     let first = names.first().map(|n| norm(n));
     let (region, rest): (Vec<&str>, Vec<&str>) = names.iter().copied().partition(|n| Some(norm(n)) == first);

crates/proof/src/lib.rs

@@ -580,10 +580,12 @@ pub fn has_dictionary(lang: Option<&str>) -> bool {
     ProofLang::from_tag(lang).is_proofed()
 }
 
-/// The Hunspell dictionary name (`pl_PL`, `it_IT`) a proofing language needs but doesn't have
-/// installed, for a status-bar hint (#1167). `None` when the language's dictionary is available,
-/// or when it needs none (English has a built-in word list; unproofed languages aren't offered
-/// one). Reads the dictionary once (it is cached afterwards).
+/// The Hunspell dictionary name (`pl_PL`, `it_IT`, `nl_NL`) a proofing language needs but doesn't
+/// have installed, for a status-bar hint (#1167). `None` when the language's dictionary is
+/// available, when it needs none (English has a built-in word list), or for a tag that is no
+/// language (`x-none`, `zxx`). Any other language is named by its tag, even one no Hunspell
+/// dictionary exists for (`ja`, `zh`, `th`): the caller decides whether to show the hint. Reads
+/// the dictionary once (it is cached afterwards).
 pub fn missing_dictionary(lang: Option<&str>) -> Option<&'static str> {
     let lang = ProofLang::from_tag(lang);
     dictionary_needed(lang, dictionary_loaded(lang)?)
@@ -614,31 +616,38 @@ fn dictionary_needed(lang: ProofLang, installed: bool) -> Option<&'static str> {
         return None;
     }
     if let Some(l) = lang.spelling_locale() {
-        return pack_lang::names(l).first().and_then(|n| interned(n));
+        return pack_lang::names(l).first().map(|n| interned(n));
     }
     language(lang).map(|l| l.slot.name())
 }
 
 /// `name` as a `&'static str` (the API hands out Hunspell names that way), for at most
-/// [`MAX_INTERNED`] names; `None` after that.
-fn interned(name: &str) -> Option<&'static str> {
+/// [`MAX_INTERNED`] names; after that, [`UNINTERNED`], so the hint still shows.
+fn interned(name: &str) -> &'static str {
     // ponytail: leaks each distinct pack dictionary name once, capped; a `Cow` API if more matter.
     static NAMES: std::sync::Mutex<Vec<&'static str>> = std::sync::Mutex::new(Vec::new());
-    let mut names = NAMES.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
+    interned_in(&mut NAMES.lock().unwrap_or_else(std::sync::PoisonError::into_inner), name)
+}
+
+/// [`interned`] with the names kept so far.
+fn interned_in(names: &mut Vec<&'static str>, name: &str) -> &'static str {
     if let Some(n) = names.iter().find(|n| **n == name) {
-        return Some(n);
+        return n;
     }
     if names.len() >= MAX_INTERNED {
-        return None;
+        return UNINTERNED;
     }
     let n: &'static str = Box::leak(name.to_string().into_boxed_str());
     names.push(n);
-    Some(n)
+    n
 }
 
 /// Most pack dictionary names [`interned`] keeps.
 const MAX_INTERNED: usize = 256;
 
+/// The name [`interned`] gives once it keeps no more.
+const UNINTERNED: &str = "a Hunspell dictionary";
+
 /// Is `word` spelled correctly in the run's proofing language? A language without a dictionary
 /// can't be checked, so every word counts as correct (no red underlines for it).
 pub fn is_correct_for(word: &str, lang: Option<&str>) -> bool {
@@ -939,7 +948,8 @@ fn grammar_in(text: &str, lang: ProofLang, capital_first: bool) -> Vec<Issue> {
         ProofLang::En => grammar_en(text, capital_first),
         ProofLang::EnGb | ProofLang::EnCa => grammar_en(text, capital_first).into_iter().map(|i| Issue { lang, ..i }).collect(),
         // `pt-pt` shares Portuguese's rules, its issues in its own language.
-        _ => language(lang.built_in()).map_or_else(Vec::new, |l| (l.grammar)(text, lang, capital_first).into_iter().map(|i| Issue { lang, ..i }).collect()),
+        _ => language(lang.built_in())
+            .map_or_else(Vec::new, |l| (l.grammar)(text, lang, capital_first).into_iter().map(|i| Issue { lang, ..i }).collect()),
     }
 }
 
@@ -1703,7 +1713,8 @@ mod tests {
             std::fs::write(system.join(format!("{name}.dic")), "1\nx\n").unwrap();
             system.join(format!("{name}.dic"))
         };
-        let pack = |locale: &str| write_pack(&packs_dir, locale, &[("spelling.aff", b"SET UTF-8\n"), ("spelling.dic", b"1\nx\n")]).join("spelling.dic");
+        let pack =
+            |locale: &str| write_pack(&packs_dir, locale, &[("spelling.aff", b"SET UTF-8\n"), ("spelling.dic", b"1\nx\n")]).join("spelling.dic");
         let tag = |t: &str| ProofLang::from_tag(Some(t));
         packs::set_packs_dir(Some(packs_dir.clone()));
 
@@ -1772,6 +1783,18 @@ mod tests {
         let _ = std::fs::remove_dir_all(&root);
     }
 
+    /// Second review of #485: past [`MAX_INTERNED`] names the hint still names a dictionary.
+    #[test]
+    fn interned_names_fall_back_to_a_generic_one() {
+        let mut names = Vec::new();
+        for i in 0..MAX_INTERNED {
+            assert_eq!(interned_in(&mut names, &format!("xx_{i}")), format!("xx_{i}"));
+        }
+        assert_eq!(interned_in(&mut names, "xx_0"), "xx_0", "a kept name is still given");
+        assert_eq!(interned_in(&mut names, "yy_YY"), UNINTERNED);
+        assert_eq!(names.len(), MAX_INTERNED);
+    }
+
     /// Review of #485: German and Bulgarian, which have hyphenation of their own but no
     /// dictionary slot, are spelled from their Language pack (or a Hunspell dictionary) too.
     #[test]

crates/proof/src/packs.rs

@@ -161,7 +161,7 @@ pub fn licence_texts() -> Vec<(String, String)> {
     let mut left: u64 = MAX_LICENCE_TOTAL;
     for (p, _) in &scan().packs {
         for (file, text) in licence_files(&p.path, &mut left) {
-            out.push((format!("{} ({}): {file}", p.name, p.locale), text));
+            out.push((format!("{} ({}): {}", p.name, p.locale, plain_text(&file)), text));
         }
         if left == 0 {
             log::warn!("Language pack licences: {MAX_LICENCE_TOTAL} bytes in all; the rest aren't shown");
@@ -287,12 +287,12 @@ fn parse_pack_toml(text: &str) -> Result<PackInfo, String> {
     if !is_locale(&locale) {
         return Err(format!("pack.toml: {locale:?} isn't a lowercase language tag"));
     }
-    let licence = string("licence").or_else(|| string("license")).filter(|l| !l.trim().is_empty()).ok_or("pack.toml: no licence")?;
+    let licence = string("licence").or_else(|| string("license")).map(|l| plain_text(&l)).filter(|l| !l.is_empty()).ok_or("pack.toml: no licence")?;
     let name = string("name").map(|n| plain_text(&n)).filter(|n| !n.is_empty());
-    Ok(PackInfo { locale, name, licence: licence.chars().take(200).collect() })
+    Ok(PackInfo { locale, name, licence })
 }
 
-/// Text from a pack shown to people (its name): without control, bidirectional or other format
+/// Text from a pack shown to people (its name, licence and licence file names): without control, bidirectional or other format
 /// characters (U+202E would reverse what follows), trimmed, at most 200 characters.
 pub(crate) fn plain_text(s: &str) -> String {
     s.chars().filter(|&c| !c.is_control() && !is_format(c)).take(200).collect::<String>().trim().to_string()
@@ -607,6 +607,10 @@ mod tests {
         assert_eq!(parse_pack_toml(t).unwrap().name.as_deref(), Some("Nederlands"));
         let t = "locale = \"nl\"\nlicence = \"MIT\"\nname = \"\\u202e\\u200f\"\n";
         assert_eq!(parse_pack_toml(t).unwrap().name, None, "nothing left: the locale is shown");
+        let t = "locale = \"nl\"\nlicence = \"MIT\\u202e LPG\"\n";
+        assert_eq!(parse_pack_toml(t).unwrap().licence, "MIT LPG");
+        let t = "locale = \"nl\"\nlicence = \"\\u200f\"\n";
+        assert!(parse_pack_toml(t).is_err(), "no licence left");
     }
 
     #[test]