From 401ea058d1644346c4b8f41ca25b4c5e6754fdf1 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:51:13 +0200 Subject: [PATCH] Proofing: say when a dictionary is installed but unreadable, with file and reason (storytold/wordcraft#2830) (storytold/wordcraft#2900) * Proofing: say when a dictionary is installed but unreadable, with file and reason (storytold/wordcraft#2830) * proof test: use SHIFT_JIS as the unsupported encoding (ISO 8859-1 is decoded since storytold/wordcraft#2904) --------- Co-authored-by: Brandon Thomas --- crates/proof/src/hr/mod.rs | 3 ++- crates/proof/src/hunspell.rs | 44 ++++++++++++++++++++++++++++++++++++ crates/proof/src/id/mod.rs | 3 ++- crates/proof/src/it/mod.rs | 3 ++- crates/proof/src/lib.rs | 33 +++++++++++++++++++++++++++ crates/proof/src/pl/mod.rs | 3 ++- crates/proof/src/pt/mod.rs | 3 ++- crates/proof/src/tr/mod.rs | 9 ++++++-- 8 files changed, 94 insertions(+), 7 deletions(-) diff --git a/crates/proof/src/hr/mod.rs b/crates/proof/src/hr/mod.rs index 5bbb55bc..23df9e93 100644 --- a/crates/proof/src/hr/mod.rs +++ b/crates/proof/src/hr/mod.rs @@ -36,11 +36,12 @@ pub fn dictionary() -> Arc { fn load_installed() -> AffixDict { let Some((aff, dic)) = crate::hunspell::find(NAMES) else { + crate::hunspell::clear_load_error(NAMES[0]); log::info!("Croatian spelling: no Hunspell hr_HR dictionary installed"); return AffixDict::default(); }; - crate::hunspell::load(&aff, &dic).unwrap_or_else(|e| { + crate::hunspell::note_load(NAMES[0], &dic, crate::hunspell::load(&aff, &dic)).unwrap_or_else(|e| { log::warn!("Croatian spelling: {e}"); AffixDict::default() }) diff --git a/crates/proof/src/hunspell.rs b/crates/proof/src/hunspell.rs index 330dfdc5..6f0cab1d 100644 --- a/crates/proof/src/hunspell.rs +++ b/crates/proof/src/hunspell.rs @@ -20,10 +20,36 @@ const MAX_FILE: u64 = 64 << 20; /// An extra directory searched first (a setting), set by the front end. static EXTRA_DIR: RwLock> = RwLock::new(None); +/// Why an installed dictionary failed to load, by `.dic` name (`pt_BR`): the file and the reason. +/// Without it a file that is there but unreadable looks the same as no file (#2830). +static LOAD_ERRORS: RwLock> = RwLock::new(Vec::new()); + +/// Remember (or, for `Ok`, forget) why the dictionary `key` found at `dic` failed to load, and +/// hand the result back unchanged. +pub fn note_load(key: &str, dic: &Path, result: Result) -> Result { + let mut errors = LOAD_ERRORS.write().unwrap_or_else(|e| e.into_inner()); + errors.retain(|(k, _, _)| k != key); + if let Err(e) = &result { + errors.push((key.to_string(), dic.to_path_buf(), e.clone())); + } + result +} + +/// Forget the load error of the dictionary `key` (none found, or it is about to be searched for again). +pub fn clear_load_error(key: &str) { + LOAD_ERRORS.write().unwrap_or_else(|e| e.into_inner()).retain(|(k, _, _)| k != key); +} + +/// The `.dic` file and the reason of the last failed load of the dictionary `key`, if any. +pub fn load_error(key: &str) -> Option<(PathBuf, String)> { + LOAD_ERRORS.read().unwrap_or_else(|e| e.into_inner()).iter().find(|(k, _, _)| k == key).map(|(_, p, e)| (p.clone(), e.clone())) +} + /// Search `dir` before the system locations (a "dictionaries folder" setting); `None` clears it. /// Takes effect for dictionaries not loaded yet. pub fn set_dictionary_dir(dir: Option) { *EXTRA_DIR.write().unwrap_or_else(|e| e.into_inner()) = dir; + LOAD_ERRORS.write().unwrap_or_else(|e| e.into_inner()).clear(); // they belong to the old search path } /// The LibreOffice extension directories a Hunspell dictionary name may live in: LibreOffice's @@ -191,6 +217,24 @@ mod tests { assert!(decode(b"SET UTF-8\n", b"\xff\xfe").is_err()); } + /// #2830: a dictionary that is there but can't be read leaves its reason behind; a good + /// load or no file clears it. + #[test] + fn load_error_is_remembered_until_the_next_good_load() { + let key = "test_unreadable_2830"; + assert_eq!(load_error(key), None); + let dic = Path::new("/dicts/test.dic"); + assert!(note_load(key, dic, decode(b"SET SHIFT_JIS\n", b"")).is_err()); + let (file, reason) = load_error(key).expect("error kept"); + assert_eq!(file, dic); + assert_eq!(reason, "unsupported encoding SHIFT_JIS"); + assert!(note_load(key, dic, decode(b"SET UTF-8\n", b"")).is_ok()); + assert_eq!(load_error(key), None); + let _ = note_load::<()>(key, dic, Err("too large".into())); + clear_load_error(key); + assert_eq!(load_error(key), None); + } + #[test] fn decodes_latin1() { // `orçamento esperávamos` in ISO 8859-1: ç = 0xE7, á = 0xE1. diff --git a/crates/proof/src/id/mod.rs b/crates/proof/src/id/mod.rs index 92f6570c..5b9be698 100644 --- a/crates/proof/src/id/mod.rs +++ b/crates/proof/src/id/mod.rs @@ -41,10 +41,11 @@ pub fn dictionary() -> Arc { fn load_installed() -> AffixDict { let Some((aff, dic)) = crate::hunspell::find(NAMES) else { + crate::hunspell::clear_load_error(NAMES[0]); log::info!("Indonesian spelling: no Hunspell id_ID dictionary installed"); return AffixDict::default(); }; - crate::hunspell::load(&aff, &dic).unwrap_or_else(|e| { + crate::hunspell::note_load(NAMES[0], &dic, crate::hunspell::load(&aff, &dic)).unwrap_or_else(|e| { log::warn!("Indonesian spelling: {e}"); AffixDict::default() }) diff --git a/crates/proof/src/it/mod.rs b/crates/proof/src/it/mod.rs index c8e33f14..77284784 100644 --- a/crates/proof/src/it/mod.rs +++ b/crates/proof/src/it/mod.rs @@ -45,10 +45,11 @@ pub fn dictionary() -> Arc { fn load_installed() -> AffixDict { let Some((aff, dic)) = crate::hunspell::find(NAMES) else { + crate::hunspell::clear_load_error(NAMES[0]); log::info!("Italian spelling: no Hunspell it_IT dictionary installed"); return AffixDict::default(); }; - crate::hunspell::load(&aff, &dic).unwrap_or_else(|e| { + crate::hunspell::note_load(NAMES[0], &dic, crate::hunspell::load(&aff, &dic)).unwrap_or_else(|e| { log::warn!("Italian spelling: {e}"); AffixDict::default() }) diff --git a/crates/proof/src/lib.rs b/crates/proof/src/lib.rs index 28dfb582..5afa42d7 100644 --- a/crates/proof/src/lib.rs +++ b/crates/proof/src/lib.rs @@ -167,6 +167,22 @@ pub fn installed_dictionaries() -> Vec<(ProofLang, Option)> ] } +/// Why the dictionary found for `lang` failed to load (unsupported encoding, too large, a parse +/// error), if it was read and failed. Doesn't read anything itself: a dictionary nobody has +/// checked yet reports `None` (#2830). +pub fn dictionary_load_error(lang: ProofLang) -> Option { + let key = match lang { + ProofLang::Pl => "pl_PL", + ProofLang::Pt => "pt_BR", + ProofLang::It => "it_IT", + ProofLang::Hr => "hr_HR", + ProofLang::Tr => "tr_TR", + ProofLang::Id => "id_ID", + _ => return None, + }; + hunspell::load_error(key).map(|(_, reason)| reason) +} + /// A number that changes whenever a word is added to or removed from the user dictionary, or the /// dictionaries folder changes. pub fn dictionary_generation() -> u64 { @@ -384,6 +400,23 @@ pub fn missing_dictionary(lang: Option<&str>) -> Option<&'static str> { } } +/// The Hunspell name, `.dic` file and reason of a dictionary that is installed for this language +/// but couldn't be read (unsupported `SET` encoding, too large, a parse error). `None` when it +/// loaded, or when no file exists (then [`missing_dictionary`] names it). Reads the dictionary +/// once (it is cached afterwards). +pub fn unreadable_dictionary(lang: Option<&str>) -> Option<(&'static str, std::path::PathBuf, String)> { + let (proof_lang, loaded) = match ProofLang::from_tag(lang) { + ProofLang::Pl => (ProofLang::Pl, pl::has_dictionary()), + ProofLang::Pt => (ProofLang::Pt, pt::has_dictionary()), + _ => return None, + }; + let name = dictionary_needed(proof_lang, false)?; + if loaded { + return None; + } + hunspell::load_error(name).map(|(file, reason)| (name, file, reason)) +} + /// The Hunspell name `lang` needs when `installed` is false; only Polish and Portuguese use one. fn dictionary_needed(lang: ProofLang, installed: bool) -> Option<&'static str> { match lang { diff --git a/crates/proof/src/pl/mod.rs b/crates/proof/src/pl/mod.rs index 22725340..5330dd72 100644 --- a/crates/proof/src/pl/mod.rs +++ b/crates/proof/src/pl/mod.rs @@ -44,10 +44,11 @@ pub fn dictionary() -> Arc { fn load_installed() -> AffixDict { let Some((aff, dic)) = crate::hunspell::find(NAMES) else { + crate::hunspell::clear_load_error(NAMES[0]); log::info!("Polish spelling: no Hunspell pl_PL dictionary installed"); return AffixDict::default(); }; - crate::hunspell::load(&aff, &dic).unwrap_or_else(|e| { + crate::hunspell::note_load(NAMES[0], &dic, crate::hunspell::load(&aff, &dic)).unwrap_or_else(|e| { log::warn!("Polish spelling: {e}"); AffixDict::default() }) diff --git a/crates/proof/src/pt/mod.rs b/crates/proof/src/pt/mod.rs index 24c3d3b8..e0f62e52 100644 --- a/crates/proof/src/pt/mod.rs +++ b/crates/proof/src/pt/mod.rs @@ -43,10 +43,11 @@ pub fn dictionary() -> Arc { fn load_installed() -> AffixDict { let Some((aff, dic)) = crate::hunspell::find(NAMES) else { + crate::hunspell::clear_load_error(NAMES[0]); log::info!("Portuguese spelling: no Hunspell pt_BR dictionary installed"); return AffixDict::default(); }; - crate::hunspell::load(&aff, &dic).unwrap_or_else(|e| { + crate::hunspell::note_load(NAMES[0], &dic, crate::hunspell::load(&aff, &dic)).unwrap_or_else(|e| { log::warn!("Portuguese spelling: {e}"); AffixDict::default() }) diff --git a/crates/proof/src/tr/mod.rs b/crates/proof/src/tr/mod.rs index 2b18158a..d814dc2a 100644 --- a/crates/proof/src/tr/mod.rs +++ b/crates/proof/src/tr/mod.rs @@ -40,16 +40,21 @@ pub fn dictionary() -> Arc { fn load_installed() -> TrDict { let Some((aff, dic)) = crate::hunspell::find(NAMES) else { + crate::hunspell::clear_load_error(NAMES[0]); log::info!("Turkish spelling: no Hunspell tr_TR dictionary installed"); return TrDict::default(); }; let read = |p: &std::path::Path| std::fs::read_to_string(p); match (read(&aff), read(&dic)) { - (Ok(aff_text), Ok(dic_text)) => TrDict::from_hunspell(&aff_text, &dic_text).unwrap_or_else(|e| { + (Ok(aff_text), Ok(dic_text)) => crate::hunspell::note_load(NAMES[0], &dic, TrDict::from_hunspell(&aff_text, &dic_text)).unwrap_or_else(|e| { log::warn!("Turkish spelling: {e}"); TrDict::default() }), - _ => TrDict::default(), + (a, d) => { + let reason = a.err().or(d.err()).map_or_else(String::new, |e| e.to_string()); + let _ = crate::hunspell::note_load::<()>(NAMES[0], &dic, Err(reason)); + TrDict::default() + } } }