From 24744c9256066fbe3976dc3e0d1ec4e5a23a7b4a Mon Sep 17 00:00:00 2001 From: Brady Fomegne Date: Fri, 15 Sep 2023 15:14:13 +0100 Subject: [PATCH] chore(clafrica): code review (#85) --- clafrica/src/api.rs | 96 +++++++++++++++++++++----------------- clafrica/src/config.rs | 53 ++++++++++----------- clafrica/src/lib.rs | 5 +- clafrica/src/processor.rs | 3 +- clafrica/src/translator.rs | 19 +++++--- 5 files changed, 94 insertions(+), 82 deletions(-) diff --git a/clafrica/src/api.rs b/clafrica/src/api.rs index 3ebbcca..2efccd7 100644 --- a/clafrica/src/api.rs +++ b/clafrica/src/api.rs @@ -49,14 +49,14 @@ impl Frontend for Console { .chain(self.predicates.iter().enumerate()) .skip(self.current_predicate_id) .take(page_size) - .map(|(i, (_code, remaining_code, text))| format!( + .map(|(id, (_code, remaining_code, text))| format!( "{}{}. {} ~{}\t ", - if i == self.current_predicate_id { + if id == self.current_predicate_id { "*" } else { "" }, - i + 1, + id + 1, text, remaining_code )) @@ -99,44 +99,54 @@ impl Frontend for Console { } } -#[test] -fn test_console() { - let mut none = None; - none.set_input("hello"); - none.update_screen((64, 64)); - none.update_position((64.0, 64.0)); - none.set_input("input"); - none.set_page_size(10); - none.add_predicate("hey", "y", "hello"); - none.display(); - none.clear_predicates(); - none.previous_predicate(); - none.next_predicate(); - none.get_selected_predicate(); - - let mut console = Console::default(); - console.set_page_size(10); - console.update_screen((0, 0)); - console.update_position((0.0, 0.0)); - console.set_input("he"); - - console.add_predicate("hell", "llo", "hello"); - console.add_predicate("helip", "lip", "helicopter"); - console.add_predicate("heal", "al", "health"); - console.display(); - console.previous_predicate(); - assert_eq!( - console.get_selected_predicate(), - Some(&("heal".to_owned(), "al".to_owned(), "health".to_owned())) - ); - console.next_predicate(); - assert_eq!( - console.get_selected_predicate(), - Some(&("hell".to_owned(), "llo".to_owned(), "hello".to_owned())) - ); - - console.clear_predicates(); - console.previous_predicate(); - console.next_predicate(); - assert!(console.get_selected_predicate().is_none()); +#[cfg(test)] +mod tests { + #[test] + fn test_none() { + use crate::api::{Frontend, None}; + + let mut none = None; + none.set_input("hello"); + none.update_screen((64, 64)); + none.update_position((64.0, 64.0)); + none.set_input("input"); + none.set_page_size(10); + none.add_predicate("hey", "y", "hello"); + none.display(); + none.clear_predicates(); + none.previous_predicate(); + none.next_predicate(); + none.get_selected_predicate(); + } + + #[test] + fn test_console() { + use crate::api::{Console, Frontend}; + + let mut console = Console::default(); + console.set_page_size(10); + console.update_screen((0, 0)); + console.update_position((0.0, 0.0)); + console.set_input("he"); + + console.add_predicate("hell", "llo", "hello"); + console.add_predicate("helip", "lip", "helicopter"); + console.add_predicate("heal", "al", "health"); + console.display(); + console.previous_predicate(); + assert_eq!( + console.get_selected_predicate(), + Some(&("heal".to_owned(), "al".to_owned(), "health".to_owned())) + ); + console.next_predicate(); + assert_eq!( + console.get_selected_predicate(), + Some(&("hell".to_owned(), "llo".to_owned(), "hello".to_owned())) + ); + + console.clear_predicates(); + console.previous_predicate(); + console.next_predicate(); + assert!(console.get_selected_predicate().is_none()); + } } diff --git a/clafrica/src/config.rs b/clafrica/src/config.rs index b800a1e..4f210b5 100644 --- a/clafrica/src/config.rs +++ b/clafrica/src/config.rs @@ -15,7 +15,7 @@ pub struct Config { #[derive(Deserialize, Debug, Clone)] pub struct CoreConfig { pub buffer_size: Option, - pub auto_capitalize: Option, + auto_capitalize: Option, pub page_size: Option, pub auto_commit: Option, } @@ -70,7 +70,7 @@ impl Config { let auto_capitalize = config .core .as_ref() - .map(|c| c.auto_capitalize.unwrap_or(true)) + .and_then(|c| c.auto_capitalize) .unwrap_or(true); // Data @@ -110,8 +110,8 @@ impl Config { let conf = Config::from_file(&filepath)?; translators.extend(conf.translators.unwrap_or_default()); } - Data::Simple(v) => { - let filepath = config_path.join(v.clone()).to_str().unwrap().to_string(); + Data::Simple(value) => { + let filepath = config_path.join(value.clone()).to_str().unwrap().to_string(); translators.insert(key.to_owned(), Data::Simple(filepath)); } _ => Err(format!("Invalid script file `{filepath:?}`.\nCaused by:\n\t{value:?} not allowed in the translator table."))?, @@ -141,8 +141,8 @@ impl Config { }); } Data::MoreDetailed(MoreDetailedData { values, alias }) => { - alias.iter().chain([key.to_owned()].iter()).for_each(|e| { - translation.insert(e.to_owned(), Data::Multi(values.clone())); + alias.iter().chain([key.to_owned()].iter()).for_each(|key| { + translation.insert(key.to_owned(), Data::Multi(values.clone())); }); } }; @@ -162,12 +162,12 @@ impl Config { .as_ref() .unwrap_or(&empty) .iter() - .filter_map(|(k, v)| { - let v = match v { + .filter_map(|(key, value)| { + let value = match value { Data::Simple(value) => Some(value), _ => None, }; - v.map(|v| (k.to_owned(), v.to_owned())) + value.map(|value| (key.to_owned(), value.to_owned())) }) .collect() } @@ -211,14 +211,14 @@ impl Config { .as_ref() .unwrap_or(&empty) .iter() - .filter_map(|(k, v)| { - let v = match v { - Data::Simple(v) => Some(vec![v.to_owned()]), - Data::Multi(v) => Some(v.to_owned()), + .filter_map(|(key, value)| { + let value = match value { + Data::Simple(value) => Some(vec![value.to_owned()]), + Data::Multi(value) => Some(value.to_owned()), _ => None, }; - v.map(|v| (k.to_owned(), v)) + value.map(|value| (key.to_owned(), value)) }) .collect() } @@ -226,11 +226,11 @@ impl Config { #[cfg(test)] mod tests { + use crate::config::Config; + use std::path::Path; + #[test] fn from_file() { - use crate::config::Config; - use std::path::Path; - let conf = Config::from_file(Path::new("./data/config_sample.toml")).unwrap(); assert_eq!( @@ -247,6 +247,11 @@ mod tests { let data = conf.extract_data(); assert_eq!(data.keys().len(), 23); + // data and core not provided + let conf = Config::from_file(Path::new("./data/blank_sample.toml")).unwrap(); + let data = conf.extract_data(); + assert_eq!(data.keys().len(), 0); + // parsing error let conf = Config::from_file(Path::new("./data/invalid_file.toml")); assert!(conf.is_err()); @@ -254,12 +259,10 @@ mod tests { // config file not found let conf = Config::from_file(Path::new("./data/not_found")); assert!(conf.is_err()); + } - // data and and core not provided - let conf = Config::from_file(Path::new("./data/blank_sample.toml")).unwrap(); - let data = conf.extract_data(); - assert_eq!(data.keys().len(), 0); - + #[test] + fn from_invalid_file() { // invalid data let conf = Config::from_file(Path::new("./data/invalid_data.toml")); assert!(conf.is_err()); @@ -271,9 +274,6 @@ mod tests { #[test] fn from_file_with_translators() { - use crate::config::Config; - use std::path::Path; - let conf = Config::from_file(Path::new("./data/config_sample.toml")).unwrap(); let translators = conf.extract_translators().unwrap(); assert_eq!(translators.keys().len(), 2); @@ -294,9 +294,6 @@ mod tests { #[test] fn from_file_with_translation() { - use crate::config::Config; - use std::path::Path; - let conf = Config::from_file(Path::new("./data/config_sample.toml")).unwrap(); let translation = conf.extract_translation(); assert_eq!(translation.keys().len(), 4); diff --git a/clafrica/src/lib.rs b/clafrica/src/lib.rs index 800e30b..35b03ca 100644 --- a/clafrica/src/lib.rs +++ b/clafrica/src/lib.rs @@ -22,7 +22,7 @@ pub fn run( config .extract_data() .iter() - .map(|(k, v)| [k.as_str(), v.as_str()]) + .map(|(key, value)| [key.as_str(), value.as_str()]) .collect(), ); let (buffer_size, auto_commit, page_size) = config @@ -99,7 +99,7 @@ pub fn run( } EventType::KeyRelease(E_Key::ControlRight) if is_special_pressed => { rdev::simulate(&EventType::KeyRelease(E_Key::ControlLeft)) - .expect("We couldn't cancel the special function"); + .expect("We couldn't cancel the special function key"); is_special_pressed = false; if let Some(predicate) = frontend.get_selected_predicate() { @@ -191,6 +191,7 @@ mod tests { fn start_sandbox() -> rstk::TkText { let root = rstk::trace_with("wish").unwrap(); root.title("Clafrica Test Environment"); + let input_field = rstk::make_text(&root); input_field.width(50); input_field.height(12); diff --git a/clafrica/src/processor.rs b/clafrica/src/processor.rs index f8964d4..2f8eab1 100644 --- a/clafrica/src/processor.rs +++ b/clafrica/src/processor.rs @@ -30,8 +30,7 @@ impl Processor { self.pause(); self.keyboard.key_up(Key::Backspace); - let i = out.chars().count(); - (1..i).for_each(|_| self.keyboard.key_click(Key::Backspace)); + (1..out.chars().count()).for_each(|_| self.keyboard.key_click(Key::Backspace)); // Clear the remaining code while let (None, 1.., ..) = self.cursor.state() { diff --git a/clafrica/src/translator.rs b/clafrica/src/translator.rs index 6d55c8a..7d7750a 100644 --- a/clafrica/src/translator.rs +++ b/clafrica/src/translator.rs @@ -26,14 +26,19 @@ impl Translator { self.dictionary .iter() - .filter_map(|(k, v)| { - if k == input { - Some((k.to_owned(), "".to_owned(), v.to_owned(), self.auto_commit)) - } else if input.len() > 1 && k.starts_with(input) { + .filter_map(|(key, value)| { + if key == input { Some(( - k.to_owned(), - k.chars().skip(input.len()).collect(), - v.to_owned(), + key.to_owned(), + "".to_owned(), + value.to_owned(), + self.auto_commit, + )) + } else if input.len() > 1 && key.starts_with(input) { + Some(( + key.to_owned(), + key.chars().skip(input.len()).collect(), + value.to_owned(), false, )) } else {