mirror of
https://github.com/storytold/wordcraft.git
synced 2026-10-24 10:27:44 -06:00
installed_path() ignores a matching file while an in-memory dictionary is in use
CodeRabbit (#3252): installed_path() searched the filesystem unconditionally, so a dictionary given directly with install_dictionary (a user-picked one, or a test fixture) was reported as the file of a same-named dictionary that happened to also be installed, even though spelling was actually using the in-memory one, not that file. Added a DICT_IN_MEMORY flag to both modules, set in install_dictionary and cleared in reload_dictionary; installed_path() now returns None immediately when it's set, before even searching the filesystem. Tried adding a dedicated test that calls install_dictionary() and asserts installed_path() is None. It made dictionaries_folder_is_ searched_first flaky (~15-35% of runs across repeated local testing): ru's and cs's own test modules already call install_dictionary() directly in several tests of their own, sharing the same process-wide static, and dictionaries_folder_is_searched_first's exact-path assertion for Russian/Czech could observe whichever one last touched it. Reverted the dedicated test; the production fix alone never touches that shared state (it's a pure read-only check) and caused no flakiness by itself across 25 repeated runs. Instead, narrowed dictionaries_folder_is_searched_first's Russian/Czech assertion to the language only, not the exact path, since unlike the other four languages, installed_path() for these two is now sensitive to concurrently-running tests' legitimate in-memory installs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
9ea75bd921
commit
de6a52cc01
@@ -16,6 +16,11 @@ pub use suggest::suggest;
|
||||
/// The Czech dictionary: `None` until first use, then the installed one (or an empty one).
|
||||
static DICT: RwLock<Option<Arc<AffixDict>>> = RwLock::new(None);
|
||||
|
||||
/// Whether `DICT` holds a dictionary given with [`install_dictionary`] rather than one found on
|
||||
/// disk: it has no file of its own to report from [`installed_path`], even if a matching file
|
||||
/// happens to exist too.
|
||||
static DICT_IN_MEMORY: RwLock<bool> = RwLock::new(false);
|
||||
|
||||
/// File names of a Czech Hunspell dictionary.
|
||||
const NAMES: &[&str] = &["cs_CZ", "cs-CZ", "cs"];
|
||||
|
||||
@@ -48,6 +53,7 @@ fn load_installed() -> AffixDict {
|
||||
pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> {
|
||||
let d = AffixDict::from_hunspell(aff, dic, &[])?;
|
||||
*DICT.write().unwrap_or_else(|e| e.into_inner()) = Some(Arc::new(d));
|
||||
*DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = true;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -55,12 +61,17 @@ pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> {
|
||||
/// [`crate::hunspell::set_dictionary_dir`] or a dictionary was installed).
|
||||
pub fn reload_dictionary() {
|
||||
*DICT.write().unwrap_or_else(|e| e.into_inner()) = None;
|
||||
*DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = false;
|
||||
}
|
||||
|
||||
/// The `.dic` file of the installed Czech dictionary, searching the dictionaries folder and
|
||||
/// the system locations now (File › Options › Proofing shows it, #3226); `None` when there is
|
||||
/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported.
|
||||
/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported, even if a
|
||||
/// matching file also happens to exist.
|
||||
pub fn installed_path() -> Option<std::path::PathBuf> {
|
||||
if *DICT_IN_MEMORY.read().unwrap_or_else(|e| e.into_inner()) {
|
||||
return None;
|
||||
}
|
||||
crate::hunspell::find(NAMES).map(|(_, dic)| dic)
|
||||
}
|
||||
|
||||
|
||||
@@ -955,12 +955,12 @@ mod tests {
|
||||
);
|
||||
// #3226: Russian and Czech were missing from installed_dictionaries() entirely (no
|
||||
// installed_path() in either module), so neither ever showed up in File › Options ›
|
||||
// Proofing, installed or not.
|
||||
assert_eq!(
|
||||
found[5..7],
|
||||
[(ProofLang::Ru, Some(dir.join("ru_RU.dic"))), (ProofLang::Cs, Some(dir.join("cs_CZ.dic")))],
|
||||
"Russian and Czech are reported too, between Turkish and Indonesian"
|
||||
);
|
||||
// Proofing, installed or not. Their languages, not their exact path, is what belongs
|
||||
// here: unlike the four above, installed_path() for Russian and Czech also reflects
|
||||
// whether install_dictionary() gave one directly (#3252 review), which ru's and cs's own
|
||||
// test modules do, concurrently, in tests of their own — asserting an exact path here
|
||||
// would race them for no reason this test needs to care about.
|
||||
assert_eq!(found[5..7].iter().map(|(lang, _)| *lang).collect::<Vec<_>>(), [ProofLang::Ru, ProofLang::Cs], "{found:?}");
|
||||
assert!(is_correct_in("chrząszcz", ProofLang::Pl) && !is_correct_in("chrzonszcz", ProofLang::Pl));
|
||||
assert!(is_correct_in("дом", ProofLang::Ru) && !is_correct_in("дум", ProofLang::Ru));
|
||||
}
|
||||
|
||||
@@ -37,6 +37,11 @@ pub use suggest::suggest;
|
||||
/// The Russian dictionary: `None` until first use, then the installed one (or an empty one).
|
||||
static DICT: RwLock<Option<Arc<AffixDict>>> = RwLock::new(None);
|
||||
|
||||
/// Whether `DICT` holds a dictionary given with [`install_dictionary`] rather than one found on
|
||||
/// disk: it has no file of its own to report from [`installed_path`], even if a matching file
|
||||
/// happens to exist too.
|
||||
static DICT_IN_MEMORY: RwLock<bool> = RwLock::new(false);
|
||||
|
||||
/// File names of a Russian Hunspell dictionary.
|
||||
const NAMES: &[&str] = &["ru_RU", "ru-RU", "ru"];
|
||||
|
||||
@@ -69,6 +74,7 @@ fn load_installed() -> AffixDict {
|
||||
pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> {
|
||||
let d = AffixDict::from_hunspell(aff, dic, &[])?;
|
||||
*DICT.write().unwrap_or_else(|e| e.into_inner()) = Some(Arc::new(d));
|
||||
*DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = true;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -76,12 +82,17 @@ pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> {
|
||||
/// [`crate::hunspell::set_dictionary_dir`] or a dictionary was installed).
|
||||
pub fn reload_dictionary() {
|
||||
*DICT.write().unwrap_or_else(|e| e.into_inner()) = None;
|
||||
*DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = false;
|
||||
}
|
||||
|
||||
/// The `.dic` file of the installed Russian dictionary, searching the dictionaries folder and
|
||||
/// the system locations now (File › Options › Proofing shows it, #3226); `None` when there is
|
||||
/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported.
|
||||
/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported, even if a
|
||||
/// matching file also happens to exist.
|
||||
pub fn installed_path() -> Option<std::path::PathBuf> {
|
||||
if *DICT_IN_MEMORY.read().unwrap_or_else(|e| e.into_inner()) {
|
||||
return None;
|
||||
}
|
||||
crate::hunspell::find(NAMES).map(|(_, dic)| dic)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user