Refactor: Replace String with impl AsRef<str> in public APIs and tests (daisy#449)

* Refactor: Replace `String` with `impl AsRef<str>` in public APIs and tests for improved flexibility

* IDE autoformat changed imports on accident

Author: moritz-gross

docs/callers.md

@@ -62,7 +62,7 @@ pub fn get_overview_text() -> Result<String>
 
 /// Get the value of the named preference.
 /// None is returned if `name` is not a known preference.
-pub fn get_preference(name: String) -> Result<String>
+pub fn get_preference(name: impl AsRef<str>) -> Result<String>
 
 /// Set a MathCAT preference. The preference name should be a known preference name.
 /// The value should either be a string or a number (depending upon the preference being set)
@@ -92,7 +92,7 @@ pub fn get_preference(name: String) -> Result<String>
 /// A value can be overwritten by calling this function again with a different value.
 /// 
 /// FIX: Some preferences are both API and user preferences and something such as '!name' should be used for overrides. Not implemented yet.
-pub fn set_preference(name: String, value: String) -> Result<()>
+pub fn set_preference(name: impl AsRef<str>, value: impl AsRef<str>) -> Result<()>
 
 /// Given a key code along with the modifier keys, the current node is moved accordingly (or value reported in some cases).
 /// `key` is the [keycode](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode#constants_for_keycode_value) for the key (in JavaScript, `ev.key_code`)
@@ -132,7 +132,7 @@ pub fn do_navigate_keypress(key: usize, shift_key: bool, control_key: bool, alt_
 /// `MoveTo0`, `MoveTo1`, `MoveTo2`, `MoveTo3`, `MoveTo4`, `MoveTo5`, `MoveTo6`, `MoveTo7`, `MoveTo8`, `MoveTo9`
 /// 
 /// When done with Navigation, call with `Exit`
-pub fn do_navigate_command(command: String) -> Result<String>
+pub fn do_navigate_command(command: impl AsRef<str>) -> Result<String>
 
 /// Return the MathML associated with the current (navigation) node.
 /// The returned result is the `id` of the node and the offset (0-based) from that node (not yet implemented)

src/braille.rs

@@ -298,13 +298,13 @@ pub fn get_navigation_node_from_braille_position(mathml: Element, position: usiz
     // save the current highlight state, set the state to be the end points so we can find the braille, then restore the state
     // FIX: this can fail if there is 8-dot braille
     use crate::interface::{get_preference, set_preference};
-    let saved_highlight_style = get_preference("BrailleNavHighlight".to_string()).unwrap();
-    set_preference("BrailleNavHighlight".to_string(), "EndPoints".to_string()).unwrap();
+    let saved_highlight_style = get_preference("BrailleNavHighlight").unwrap();
+    set_preference("BrailleNavHighlight", "EndPoints").unwrap();
 
     N_PROBES.with(|n| {*n.borrow_mut() = 0});
     // dive into the child of the <math> element (should only be one)
     let search_state = find_navigation_node(mathml, as_element(mathml.children()[0]), position)?;
-    set_preference("BrailleNavHighlight".to_string(), saved_highlight_style.to_string()).unwrap();
+    set_preference("BrailleNavHighlight", saved_highlight_style.as_str()).unwrap();
 
     // we know the attr value exists because it was found internally
     // FIX: what should be done if we never did the search?
@@ -2237,13 +2237,13 @@ fn ASCIIMath_cleanup(_pref_manager: Ref<PreferenceManager>, raw_braille: String)
 
 
 /************** Braille xpath functionality ***************/
-use crate::canonicalize::{name, as_element, as_text};
-use crate::xpath_functions::{is_leaf, IsBracketed, validate_one_node};
+use crate::canonicalize::{as_element, as_text, name};
+use crate::xpath_functions::{is_leaf, validate_one_node, IsBracketed};
+use std::result::Result as StdResult;
 use sxd_document::dom::ParentOfChild;
-use sxd_xpath::{Value, context, nodeset::*};
-use sxd_xpath::function::{Function, Args};
 use sxd_xpath::function::Error as XPathError;
-use std::result::Result as StdResult;
+use sxd_xpath::function::{Args, Function};
+use sxd_xpath::{context, nodeset::*, Value};
 
 pub struct NemethNestingChars;
 const NEMETH_FRAC_LEVEL: &str = "data-nemeth-frac-level";    // name of attr where value is cached
@@ -2781,7 +2781,7 @@ impl NeedsToBeGrouped {
         // if the fraction starts with a "-", it is still a numeric fraction that doesn't need parens
         let mut numerator = as_element(children[0]);
         let denominator = as_element(children[children.len()-1]);
-        let decimal_separator = crate::interface::get_preference("DecimalSeparators".to_string()).unwrap()
+        let decimal_separator = crate::interface::get_preference("DecimalSeparators").unwrap()
                                                         .chars().next().unwrap_or('.');
         if is_integer(denominator, decimal_separator) {
             // check numerator being either an integer "- integer"
@@ -3066,7 +3066,7 @@ mod tests {
     #[allow(unused_imports)]
     use crate::init_logger;
     use crate::interface::*;
-    
+
     #[test]
     fn ueb_highlight_24() -> Result<()> {       // issue 24
         let mathml_str = "<math display='block' id='id-0'>
@@ -3080,16 +3080,16 @@ mod tests {
         </math>";
         crate::interface::set_rules_dir(super::super::abs_rules_dir_path()).unwrap();
         set_mathml(mathml_str.to_string()).unwrap();
-        set_preference("BrailleCode".to_string(), "UEB".to_string()).unwrap();
-        set_preference("BrailleNavHighlight".to_string(), "All".to_string()).unwrap();
-        let braille = get_braille("id-2".to_string())?;
+        set_preference("BrailleCode", "UEB").unwrap();
+        set_preference("BrailleNavHighlight", "All").unwrap();
+        let braille = get_braille("id-2")?;
         assert_eq!("⣼⣙⠰⠁⠉", braille);
-        set_navigation_node("id-2".to_string(), 0)?;
+        set_navigation_node("id-2", 0)?;
         assert_eq!( get_braille_position()?, (0,2));
 
-        let braille = get_braille("id-4".to_string())?;
+        let braille = get_braille("id-4")?;
         assert_eq!("⠼⠙⣰⣁⠉", braille);
-        set_navigation_node("id-4".to_string(), 0)?;
+        set_navigation_node("id-4", 0)?;
         assert_eq!( get_braille_position()?, (2,4));
         return Ok( () );
     }
@@ -3136,10 +3136,10 @@ mod tests {
        </math>";
         crate::interface::set_rules_dir(super::super::abs_rules_dir_path()).unwrap();
         set_mathml(mathml_str.to_string()).unwrap();
-        set_preference("BrailleNavHighlight".to_string(), "Off".to_string()).unwrap();
-        
-        set_preference("BrailleCode".to_string(), "Nemeth".to_string()).unwrap();
-        let braille = get_braille("".to_string())?;
+        set_preference("BrailleNavHighlight", "Off").unwrap();
+
+        set_preference("BrailleCode", "Nemeth").unwrap();
+        let braille = get_braille("")?;
         let answers= &[2, 3, 3, 3, 3, 4, 7, 8, 9, 9,   10, 13, 12, 14, 12, 15, 17, 19, 21, 10,   4, 23, 25, 4];
         let answers = answers.map(|num| format!("id-{}", num));
         debug!("\n*** Testing Nemeth ***");
@@ -3152,8 +3152,8 @@ mod tests {
             assert_eq!(answers[i], id, "\nNemeth test ith position={}", i);
         }
 
-        set_preference("BrailleCode".to_string(), "UEB".to_string()).unwrap();
-        let braille = get_braille("".to_string())?;
+        set_preference("BrailleCode", "UEB").unwrap();
+        let braille = get_braille("")?;
         let answers= &[0, 0, 0, 2, 3, 3, 3, 3, 4, 7,   7, 8, 9, 9, 10, 13, 12, 14, 14, 15,   15, 17, 17, 19, 19, 21, 10, 4, 4, 23,   23, 25, 25, 4, 0, 0];
         let answers = answers.map(|num| format!("id-{}", num));
         debug!("\n\n*** Testing UEB ***");
@@ -3165,8 +3165,8 @@ mod tests {
             debug!("Time taken: {}ms", instant.elapsed().as_millis());
             assert_eq!(answers[i], id, "\nUEB test ith position={}", i);
         }
-        set_preference("BrailleCode".to_string(), "CMU".to_string()).unwrap();
-        let braille = get_braille("".to_string())?;
+        set_preference("BrailleCode", "CMU").unwrap();
+        let braille = get_braille("")?;
         let answers= &[2, 3, 5, 7, 8, 9, 9, 9, 10, 10,   11, 13, 12, 14, 14, 15, 17, 17, 19, 19,   21, 11, 5, 4, 22, 23, 23, 25, 25, 22,];
         let answers = answers.map(|num| format!("id-{}", num));
         debug!("\n\n*** Testing CMU ***");
@@ -3188,12 +3188,12 @@ mod tests {
         let mathml_str = "<math><msup><mi>x</mi><mi>n</mi></msup></math>";
         crate::interface::set_rules_dir(super::super::abs_rules_dir_path()).unwrap();
         set_mathml(mathml_str.to_string()).unwrap();
-        set_preference("BrailleCode".to_string(), "UEB".to_string()).unwrap();
-        set_preference("UEB_START_MODE".to_string(), "Grade2".to_string()).unwrap();
-        let braille = get_braille("".to_string())?;
+        set_preference("BrailleCode", "UEB").unwrap();
+        set_preference("UEB_START_MODE", "Grade2").unwrap();
+        let braille = get_braille("")?;
         assert_eq!("⠭⠰⠔⠝", braille, "Grade2");
-        set_preference("UEB_START_MODE".to_string(), "Grade1".to_string()).unwrap();
-        let braille = get_braille("".to_string())?;
+        set_preference("UEB_START_MODE", "Grade1").unwrap();
+        let braille = get_braille("")?;
         assert_eq!("⠭⠔⠝", braille, "Grade1");
         return Ok( () );
     }

src/infer_intent.rs

@@ -628,8 +628,8 @@ mod tests {
 		// this forces initialization
         crate::interface::set_rules_dir(super::super::abs_rules_dir_path()).unwrap();
         // crate::speech::SpeechRules::initialize_all_rules().unwrap();
-        set_preference("IntentErrorRecovery".to_string(), intent_error_recovery.to_string()).unwrap();
-        set_preference("SpeechStyle".to_string(), "SimpleSpeak".to_string()).unwrap();      // avoids possibility of "LiteralSpeak"
+        set_preference("IntentErrorRecovery", intent_error_recovery).unwrap();
+        set_preference("SpeechStyle", "SimpleSpeak").unwrap();      // avoids possibility of "LiteralSpeak"
         let package1 = &parser::parse(mathml).expect("Failed to parse test input");
         let mathml = get_element(package1);
         trim_element(mathml, false);

src/interface.rs

@@ -188,18 +188,19 @@ pub fn get_overview_text() -> Result<String> {
 
 /// Get the value of the named preference.
 /// None is returned if `name` is not a known preference.
-pub fn get_preference(name: String) -> Result<String> {
+pub fn get_preference(name: impl AsRef<str>) -> Result<String> {
     enable_logs();
+    let name = name.as_ref();
     use crate::prefs::NO_PREFERENCE;
     return crate::speech::SPEECH_RULES.with(|rules| {
         let rules = rules.borrow();
         let pref_manager = rules.pref_manager.borrow();
-        let mut value = pref_manager.pref_to_string(&name);
+        let mut value = pref_manager.pref_to_string(name);
         if value == NO_PREFERENCE {
-            value = pref_manager.pref_to_string(&name);
+            value = pref_manager.pref_to_string(name);
         }
         if value == NO_PREFERENCE {
-            bail!("No preference named '{}'", &name);
+            bail!("No preference named '{}'", name);
         } else {
             return Ok(value);
         }
@@ -226,10 +227,11 @@ pub fn get_preference(name: String) -> Result<String> {
 /// A value can be overwritten by calling this function again with a different value.
 ///
 /// Be careful setting preferences -- these potentially override user settings, so only preferences that really need setting should be set.
-pub fn set_preference(name: String, value: String) -> Result<()> {
+pub fn set_preference(name: impl AsRef<str>, value: impl AsRef<str>) -> Result<()> {
     enable_logs();
+    let name = name.as_ref();
     // "LanguageAuto" allows setting the language dir without actually changing the value of "Language" from Auto
-    let mut value = value;
+    let mut value = value.as_ref().to_string();
     if name == "Language" || name == "LanguageAuto" {
         // check the format
         if value != "Auto" {
@@ -274,14 +276,14 @@ pub fn set_preference(name: String, value: String) -> Result<()> {
         }
         let lower_case_value = value.to_lowercase();
         if lower_case_value == "true" || lower_case_value == "false" {
-            pref_manager.set_api_boolean_pref(&name, value.to_lowercase() == "true");
+            pref_manager.set_api_boolean_pref(name, value.to_lowercase() == "true");
         } else {
-            match name.as_str() {
+            match name {
                 "Pitch" | "Rate" | "Volume" | "CapitalLetters_Pitch" | "MathRate" | "PauseFactor" => {
-                    pref_manager.set_api_float_pref(&name, to_float(&name, &value)?)
+                    pref_manager.set_api_float_pref(name, to_float(name, &value)?)
                 }
                 _ => {
-                    pref_manager.set_string_pref(&name, &value)?;
+                    pref_manager.set_string_pref(name, &value)?;
                 }
             }
         };
@@ -301,14 +303,14 @@ pub fn set_preference(name: String, value: String) -> Result<()> {
 /// Get the braille associated with the MathML that was set by [`set_mathml`].
 /// The braille returned depends upon the preference for the `code` preference (default `Nemeth`).
 /// If 'nav_node_id' is given, it is highlighted based on the value of `BrailleNavHighlight` (default: `EndPoints`)
-pub fn get_braille(nav_node_id: String) -> Result<String> {
+pub fn get_braille(nav_node_id: impl AsRef<str>) -> Result<String> {
     enable_logs();
     // use std::time::{Instant};
     // let instant = Instant::now();
     return MATHML_INSTANCE.with(|package_instance| {
         let package_instance = package_instance.borrow();
         let mathml = get_element(&package_instance);
-        let braille = crate::braille::braille_mathml(mathml, &nav_node_id)?.0;
+        let braille = crate::braille::braille_mathml(mathml, nav_node_id.as_ref())?.0;
         // info!("Time taken: {}ms", instant.elapsed().as_millis());
         return Ok(braille);
     });
@@ -418,9 +420,9 @@ pub fn do_navigate_keypress(
 ///   `MoveTo0`, `MoveTo1`, `MoveTo2`, `MoveTo3`, `MoveTo4`, `MoveTo5`, `MoveTo6`, `MoveTo7`, `MoveTo8`, `MoveTo9`
 ///
 /// When done with Navigation, call with `Exit`
-pub fn do_navigate_command(command: String) -> Result<String> {
+pub fn do_navigate_command(command: impl AsRef<str>) -> Result<String> {
     enable_logs();
-    let command = NAV_COMMANDS.get_key(&command); // gets a &'static version of the command
+    let command = NAV_COMMANDS.get_key(command.as_ref()); // gets a &'static version of the command
     if command.is_none() {
         bail!("Unknown command in call to DoNavigateCommand()");
     };
@@ -434,12 +436,12 @@ pub fn do_navigate_command(command: String) -> Result<String> {
 
 /// Given an 'id' and an offset (for tokens), set the navigation node to that id.
 /// An error is returned if the 'id' doesn't exist
-pub fn set_navigation_node(id: String, offset: usize) -> Result<()> {
+pub fn set_navigation_node(id: impl AsRef<str>, offset: usize) -> Result<()> {
     enable_logs();
     return MATHML_INSTANCE.with(|package_instance| {
         let package_instance = package_instance.borrow();
         let mathml = get_element(&package_instance);
-        return set_navigation_node_from_id(mathml, id, offset);
+        return set_navigation_node_from_id(mathml, id.as_ref(), offset);
     });
 }
 

src/lib.rs

@@ -86,9 +86,9 @@ pub fn are_strs_canonically_equal_with_locale(test: &str, target: &str, ignore_a
     // this forces initialization
     crate::interface::set_rules_dir(abs_rules_dir_path()).unwrap();
     crate::speech::SPEECH_RULES.with(|rules|  rules.borrow_mut().read_files().unwrap());
-    set_preference("Language".to_string(), "en".to_string()).unwrap();
-    set_preference("BlockSeparators".to_string(), block_separators.to_string()).unwrap();
-    set_preference("DecimalSeparators".to_string(), decimal_separators.to_string()).unwrap();
+    set_preference("Language", "en").unwrap();
+    set_preference("BlockSeparators", block_separators).unwrap();
+    set_preference("DecimalSeparators", decimal_separators).unwrap();
 
     let package1 = &parser::parse(test).expect("Failed to parse test input");
     let mathml = get_element(package1);