From 5315fca4bf25ace2e9de6faf09331b706492715b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Gro=C3=9F?= Date: Sat, 19 Sep 2026 17:06:40 +0200 Subject: [PATCH 1/4] remove pointless cloning --- src/prefs.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/prefs.rs b/src/prefs.rs index 4dc51c4bc..4dce7daf3 100644 --- a/src/prefs.rs +++ b/src/prefs.rs @@ -133,12 +133,11 @@ impl Preferences{ verify_keys(doc, "Other", file_name)?; } - let prefs = &mut base_prefs.prefs; - add_prefs(prefs, &doc["Speech"], "", file_name); - add_prefs(prefs, &doc["Navigation"], "", file_name); - add_prefs(prefs, &doc["Braille"], "", file_name); - add_prefs(prefs, &doc["Other"], "", file_name); - return Ok( Preferences{ prefs: prefs.to_owned() } ); + add_prefs(&mut base_prefs.prefs, &doc["Speech"], "", file_name); + add_prefs(&mut base_prefs.prefs, &doc["Navigation"], "", file_name); + add_prefs(&mut base_prefs.prefs, &doc["Braille"], "", file_name); + add_prefs(&mut base_prefs.prefs, &doc["Other"], "", file_name); + return Ok(base_prefs); From e2cf3858751296fbbb84be2a8b108c7a0f69a690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Gro=C3=9F?= Date: Sat, 19 Sep 2026 17:28:28 +0200 Subject: [PATCH 2/4] simplify logic flow of is_user_pref --- src/prefs.rs | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/prefs.rs b/src/prefs.rs index 4dce7daf3..277107a35 100644 --- a/src/prefs.rs +++ b/src/prefs.rs @@ -718,20 +718,15 @@ impl PreferenceManager { bail!("{} is an invalid value! Must contains only ascii letters, '_', or'-'", key); } - // don't do an update if the value hasn't changed - let mut is_user_pref = true; - if let Some(pref_value) = self.api_prefs.prefs.get(key) { - if pref_value.as_str().unwrap() != value { - is_user_pref = false; - self.reset_files_from_preference_change(key, value)?; - } - } else if let Some(pref_value) = self.user_prefs.prefs.get(key) { - if pref_value.as_str().unwrap() != value { - self.reset_files_from_preference_change(key, value)?; - } - } else { - bail!("{} is an unknown MathCAT preference!", key); + let api_pref: Option<&Yaml> = self.api_prefs.prefs.get(key); + let Some(pref_value) = api_pref.or_else(|| self.user_prefs.prefs.get(key)) else { + bail!("{key} is an unknown MathCAT preference!"); + }; + if pref_value.as_str().unwrap() == value { + return Ok( () ); } + let is_user_pref: bool = api_pref.is_none(); + self.reset_files_from_preference_change(key, value)?; // debug!("Setting ({}) {} to '{}'", if is_user_pref {"user"} else {"sys"}, key, value); if is_user_pref { @@ -1177,6 +1172,19 @@ cfg_if::cfg_if! {if #[cfg(not(feature = "include-zip"))] { }); } + /// Setting `TTS` to its existing value should not copy it from API preferences into user preferences. + #[test] + fn unchanged_api_string_pref_is_noop() { + let mut pref_manager = PreferenceManager { + api_prefs: Preferences::api_defaults(), + ..PreferenceManager::default() + }; + + pref_manager.set_string_pref("TTS", "none").unwrap(); + + assert!(!pref_manager.user_prefs.prefs.contains_key("TTS")); + } + /// #262: MathCAT must notice when a rule file on disk changes and reload it. /// This copies the rule files it needs (`en`, `zz`, and `Nemeth`) into a uniquely-named /// temp dir, so the test can freely modify files and run in parallel without touching the From a2fad45ab85de69f2be16cb750602f9578d49820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Gro=C3=9F?= Date: Sat, 19 Sep 2026 17:57:41 +0200 Subject: [PATCH 3/4] simplify path joins --- src/prefs.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/prefs.rs b/src/prefs.rs index 277107a35..c91ac56c8 100644 --- a/src/prefs.rs +++ b/src/prefs.rs @@ -324,8 +324,7 @@ impl PreferenceManager { let mut prefs = Preferences::default(); - let mut system_prefs_file = self.rules_dir.to_path_buf(); - system_prefs_file.push("prefs.yaml"); + let system_prefs_file: PathBuf = self.rules_dir.join("prefs.yaml"); if is_file_shim(&system_prefs_file) { let defaults = DEFAULT_USER_PREFERENCES.with(|defaults| defaults.clone()); prefs = Preferences::read_prefs_file(&system_prefs_file, defaults)?; @@ -370,11 +369,11 @@ impl PreferenceManager { let language = self.pref_to_string("Language"); let language = if language.as_str() == "Auto" {"en"} else {language.as_str()}; // avoid 'temp value dropped while borrowed' error - let language_dir = rules_dir.to_path_buf().join("Languages"); + let language_dir = rules_dir.join("Languages"); self.set_speech_files(&language_dir, language, None)?; // also sets style file let braille_code = self.pref_to_string("BrailleCode"); - let braille_dir = rules_dir.to_path_buf().join("Braille"); + let braille_dir = rules_dir.join("Braille"); self.set_braille_files(&braille_dir, &braille_code)?; return Ok(()); } @@ -433,12 +432,12 @@ impl PreferenceManager { let new_language = new_prefs.prefs.get("Language").unwrap(); debug!("set_files_based_on_changes: old_language={old_language:?}, new_language={new_language:?}"); if old_language != new_language { - let language_dir = self.rules_dir.to_path_buf().join("Languages"); + let language_dir = self.rules_dir.join("Languages"); self.set_speech_files(&language_dir, new_language.as_str().unwrap(), None)?; // also sets style file } else { let old_speech_style = self.user_prefs.prefs.get("SpeechStyle").unwrap(); let new_speech_style = new_prefs.prefs.get("SpeechStyle").unwrap(); - let language_dir = self.rules_dir.to_path_buf().join("Languages"); + let language_dir = self.rules_dir.join("Languages"); if old_speech_style != new_speech_style { self.set_speech_files(&language_dir, new_language.as_str().unwrap(), new_speech_style.as_str())?; } @@ -447,7 +446,7 @@ impl PreferenceManager { let old_braille_code = self.user_prefs.prefs.get("BrailleCode").unwrap(); let new_braille_code = new_prefs.prefs.get("BrailleCode").unwrap(); if old_braille_code != new_braille_code { - let braille_code_dir = self.rules_dir.to_path_buf().join("Braille"); + let braille_code_dir = self.rules_dir.join("Braille"); self.set_braille_files(&braille_code_dir, new_braille_code.as_str().unwrap())?; // also sets style file } @@ -573,7 +572,7 @@ impl PreferenceManager { let mut alternative_style_file = None; // back up in case we don't find the target style in lang_dir let looking_for_style_file = file_name.ends_with("_Rules.yaml"); for os_path in lang_dir.ancestors() { // ancestor returns self and ancestors - let path = PathBuf::from(os_path).join(file_name); + let path = os_path.join(file_name); // debug!("find_file: checking file: {}", path.to_string_lossy()); if is_file_shim(&path) { // we make an exception for definitions.yaml -- there a language specific checks for Hundreds, etc @@ -641,8 +640,7 @@ impl PreferenceManager { fn get_language_dir(rules_dir: &Path, lang: &str, default_lang: Option<&str>) -> Result { // return 'Rules/Language/fr', 'Rules/Language/en/gb', etc, if they exist. // fall back to main language, and then to default_dir if language dir doesn't exist - let mut full_path = rules_dir.to_path_buf(); - full_path.push(lang.replace('-', std::path::MAIN_SEPARATOR_STR)); + let full_path = rules_dir.join(lang.replace('-', std::path::MAIN_SEPARATOR_STR)); for parent in full_path.ancestors() { if parent == rules_dir { break; @@ -757,7 +755,7 @@ impl PreferenceManager { } let changed_pref = if changed_pref == "LanguageAuto" {"Language"} else {changed_pref}; - let language_dir = self.rules_dir.to_path_buf().join("Languages"); + let language_dir = self.rules_dir.join("Languages"); match changed_pref { "Language" => { self.set_speech_files(&language_dir, changed_value, None)?; @@ -770,7 +768,7 @@ impl PreferenceManager { crate::speech::invalidate_speech_style_caches(); }, "BrailleCode" => { - let braille_dir = self.rules_dir.to_path_buf().join("Braille"); + let braille_dir = self.rules_dir.join("Braille"); self.set_braille_files(&braille_dir, changed_value)?; crate::speech::invalidate_braille_caches(); }, From d8e0f8353e59699ce9009bb08a44f55fd15063ed Mon Sep 17 00:00:00 2001 From: NSoiffer Date: Mon, 21 Sep 2026 23:41:00 -0700 Subject: [PATCH 4/4] Add comment to clarify update condition Added a comment to prevent unnecessary updates if the value hasn't changed. --- src/prefs.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/prefs.rs b/src/prefs.rs index c91ac56c8..12bb3cd51 100644 --- a/src/prefs.rs +++ b/src/prefs.rs @@ -715,7 +715,8 @@ impl PreferenceManager { !value.chars().all(|c| matches!(c, 'a'..='z' | 'A'..='Z' | '_' | '-')) { bail!("{} is an invalid value! Must contains only ascii letters, '_', or'-'", key); } - + + // don't do an update if the value hasn't changed let api_pref: Option<&Yaml> = self.api_prefs.prefs.get(key); let Some(pref_value) = api_pref.or_else(|| self.user_prefs.prefs.get(key)) else { bail!("{key} is an unknown MathCAT preference!");