From a5c285d22cafa0096536f576919fc25fd087a7b2 Mon Sep 17 00:00:00 2001 From: Eric Driussi Date: Mon, 15 Jan 2024 17:56:22 +0000 Subject: [PATCH] minor refactors --- src/authors/author.rs | 2 +- src/authors/author_err.rs | 9 ++-- src/authors/fs_repo.rs | 75 +++++++++++++++++++++------------ src/authors/mod.rs | 6 +-- src/authors/test/author.rs | 14 +++--- src/authors/test/fs_repo.rs | 20 ++++----- src/authors/test/integration.rs | 16 +++---- src/authors/test/service.rs | 2 +- src/cli/test.rs | 2 +- src/lib.rs | 6 ++- 10 files changed, 88 insertions(+), 64 deletions(-) diff --git a/src/authors/author.rs b/src/authors/author.rs index b400c34..b107609 100644 --- a/src/authors/author.rs +++ b/src/authors/author.rs @@ -6,7 +6,7 @@ pub struct Author { } impl Author { - pub fn new(alias: &str, name: &str, email: &str) -> Self { + pub fn from(alias: &str, name: &str, email: &str) -> Self { Self { alias: String::from(alias), name: String::from(name), diff --git a/src/authors/author_err.rs b/src/authors/author_err.rs index 5153b17..3658116 100644 --- a/src/authors/author_err.rs +++ b/src/authors/author_err.rs @@ -1,10 +1,13 @@ -use std::error::Error; +use std::{ + error::Error, + fmt::{Display, Formatter, Result}, +}; #[derive(Debug)] pub struct AuthorError(String); -impl std::fmt::Display for AuthorError { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { +impl Display for AuthorError { + fn fmt(&self, f: &mut Formatter) -> Result { write!(f, "AUTHORS: {}", self.0) } } diff --git a/src/authors/fs_repo.rs b/src/authors/fs_repo.rs index 245ed1e..3913b2c 100644 --- a/src/authors/fs_repo.rs +++ b/src/authors/fs_repo.rs @@ -19,7 +19,7 @@ pub struct FSRepo { } impl FSRepo { - pub fn new_default() -> Result> { + pub fn from_cwd_with_home_fallback() -> Result> { let mut local_file = env::current_dir().unwrap(); local_file.push(conf::authors_file_name()); if local_file.is_file() { @@ -51,7 +51,7 @@ impl FSRepo { } } - fn filter_by_alias(line: &str, aliases: &[String]) -> bool { + fn line_contains_any_alias(line: &str, aliases: &[String]) -> bool { aliases.iter().any(|given_alias| { let found_alias: &str = line.split(',').collect::>()[0]; given_alias.eq_ignore_ascii_case(found_alias.trim()) @@ -63,41 +63,53 @@ impl FSRepo { if fields.len() != 3 { return None; } - Some(Author::new(fields[0], fields[1], fields[2])) + Some(Author::from(fields[0], fields[1], fields[2])) } - fn extract_mathcing_authors_from_lines(alias: String, valid_lines: &[String], matching_authors: &mut Vec) { - for line in valid_lines { - if Self::filter_by_alias(line, &[alias.clone()]) { - if let Some(author) = Self::parse_author(line) { - matching_authors.push(author); - } - } + fn extract_author(line: &str, aliases: &[String]) -> Option { + if Self::line_contains_any_alias(line, aliases) { + Self::parse_author(line) + } else { + None } } + + fn extract_mathcing_authors_from_lines(alias: String, valid_lines: &[String]) -> Vec { + let mut matching_authors: Vec = Vec::new(); + valid_lines.iter().for_each(|line| { + if let Some(author) = Self::extract_author(line, &[alias.clone()]) { + matching_authors.push(author); + } + }); + matching_authors + } } impl AuthorsRepo for FSRepo { fn find(&self, aliases: Vec) -> Vec { - let mut matching_authors: Vec = Vec::new(); - - if let Some(lines) = self.read_lines() { - let valid_lines = lines.map_while(Result::ok).collect::>(); - for alias in aliases { - Self::extract_mathcing_authors_from_lines(alias, &valid_lines, &mut matching_authors); + match self.read_lines() { + None => Vec::new(), + Some(lines) => { + let valid_lines = lines.map_while(Result::ok).collect::>(); + let mut matching_authors: Vec = Vec::new(); + aliases.iter().for_each(|alias| { + matching_authors.append(&mut Self::extract_mathcing_authors_from_lines( + alias.clone(), + &valid_lines, + )); + }); + matching_authors } - }; - - matching_authors + } } fn all(&self) -> Vec { match self.read_lines() { + None => Vec::new(), Some(lines) => lines .map_while(Result::ok) .filter_map(|line| Self::parse_author(line.as_str())) .collect(), - None => Vec::new(), } } } @@ -116,32 +128,39 @@ mod test { #[test] fn should_filter_by_alias() { - let matching_alias = FSRepo::filter_by_alias("a,John,Doe", &[String::from("a")]); + let matching_alias = FSRepo::line_contains_any_alias("a,John,Doe", &[String::from("a")]); assert!(matching_alias); - let no_matching_alias = !FSRepo::filter_by_alias("b,Jane,Dane", &[String::from("a")]); + let no_matching_alias = !FSRepo::line_contains_any_alias("b,Jane,Dane", &[String::from("a")]); assert!(no_matching_alias); } #[test] fn should_parse_author() { let valid_result = FSRepo::parse_author("j,John,email"); - assert!(valid_result.is_some_and(|a| a == Author::new("j", "John", "email"))); + assert!(valid_result.is_some_and(|a| a == Author::from("j", "John", "email"))); let invalid_result = FSRepo::parse_author("hi,invalid_line"); assert!(invalid_result.is_none()); } + #[test] + fn should_extract_author() { + let valid_result = FSRepo::extract_author("j,John,email", &[String::from("j")]); + assert!(valid_result.is_some_and(|a| a == Author::from("j", "John", "email"))); + + let invalid_result = FSRepo::extract_author("a,alice,gmail", &[String::from("j")]); + assert!(invalid_result.is_none()); + } + #[test] fn should_extract_mathcing_authors_from_lines() { - let matching_authors: &mut Vec = &mut Vec::new(); - FSRepo::extract_mathcing_authors_from_lines( + let matching_authors: Vec = FSRepo::extract_mathcing_authors_from_lines( "j".to_string(), &["j,John,email".to_string(), "a,alice,gmail".to_string()], - matching_authors, ); - assert!(matching_authors.contains(&Author::new("j", "John", "email"))); - assert!(!matching_authors.contains(&Author::new("a", "alice", "gmail"))); + assert!(matching_authors.contains(&Author::from("j", "John", "email"))); + assert!(!matching_authors.contains(&Author::from("a", "alice", "gmail"))); } } diff --git a/src/authors/mod.rs b/src/authors/mod.rs index 92bf55b..94673f8 100644 --- a/src/authors/mod.rs +++ b/src/authors/mod.rs @@ -9,15 +9,15 @@ mod author_err; pub mod fs_repo; pub mod service; -pub fn fs_setup_from_file(authors_file: String) -> Result, Box> { +pub fn from_file(authors_file: String) -> Result, Box> { match FSRepo::from(authors_file) { Ok(repo) => Ok(AuthorsService::new(repo)), Err(e) => Err(AuthorError::with(format!("Couldn't load file: {}", e))), } } -pub fn new_fs_default_setup() -> Result, Box> { - match FSRepo::new_default() { +pub fn default() -> Result, Box> { + match FSRepo::from_cwd_with_home_fallback() { Ok(repo) => Ok(AuthorsService::new(repo)), Err(e) => Err(AuthorError::with(format!("Couldn't load file: {}", e))), } diff --git a/src/authors/test/author.rs b/src/authors/test/author.rs index ea6b86a..ed8b78d 100644 --- a/src/authors/test/author.rs +++ b/src/authors/test/author.rs @@ -5,7 +5,7 @@ fn setup_author() -> Author { let alias = "a"; let name = "alice"; let email = "alice@wonderland.not"; - Author::new(alias, name, email) + Author::from(alias, name, email) } #[test] @@ -28,17 +28,17 @@ fn should_provide_a_name_getter() { #[test] fn should_be_equal_to_another_author_with_equal_data() { - let author = Author::new("a", "alice", "alice@wonderland.not"); - let same_author = Author::new("a", "alice", "alice@wonderland.not"); + let author = Author::from("a", "alice", "alice@wonderland.not"); + let same_author = Author::from("a", "alice", "alice@wonderland.not"); assert_eq!(author, same_author) } #[parameterized(different_author = { - Author::new("b", "alice", "alice@wonderland.not"), - Author::new("a", "not_alice", "alice@wonderland.not"), - Author::new("a", "alice", "someone@wonderland.not") + Author::from("b", "alice", "alice@wonderland.not"), + Author::from("a", "not_alice", "alice@wonderland.not"), + Author::from("a", "alice", "someone@wonderland.not") })] fn should_not_be_equal_to_another_author_with_different_data(different_author: Author) { - let author = Author::new("a", "alice", "alice@wonderland.not"); + let author = Author::from("a", "alice", "alice@wonderland.not"); assert_ne!(author, different_author) } diff --git a/src/authors/test/fs_repo.rs b/src/authors/test/fs_repo.rs index 52171ba..7186cee 100644 --- a/src/authors/test/fs_repo.rs +++ b/src/authors/test/fs_repo.rs @@ -16,7 +16,7 @@ fn should_connect_to_an_authors_file_in_cwd_if_available() { fs::File::create(cwd_authors_file_path.clone()).unwrap(); let _after = AfterAssert::cleanup(&[&cwd_authors_file_path]); - assert!(FSRepo::new_default().is_ok()); + assert!(FSRepo::from_cwd_with_home_fallback().is_ok()); } #[test] @@ -26,13 +26,13 @@ fn should_connect_to_the_default_authors_file_if_no_file_is_available_in_cwd() { fs::File::create(&default_authors_file_path).unwrap(); let _after = AfterAssert::cleanup(&[default_authors_file_path.as_str()]); - assert!(FSRepo::new_default().is_ok()); + assert!(FSRepo::from_cwd_with_home_fallback().is_ok()); } #[test] #[serial] fn should_error_when_neither_cwd_or_default_authors_file_are_available() { - assert!(FSRepo::new_default().is_err()); + assert!(FSRepo::from_cwd_with_home_fallback().is_err()); } #[test] @@ -59,10 +59,10 @@ fn should_fetch_all_available_authors() { assert_eq!( actual_authors, [ - Author::new("a", "Name Surname", "someone@users.noreply.github.com"), - Author::new("b", "username", "something@gmail.com"), - Author::new("b", "username2", "something2@gmail.com"), - Author::new("ab", "Another Surname", "someone@something.hi"), + Author::from("a", "Name Surname", "someone@users.noreply.github.com"), + Author::from("b", "username", "something@gmail.com"), + Author::from("b", "username2", "something2@gmail.com"), + Author::from("ab", "Another Surname", "someone@something.hi"), ] ); } @@ -77,7 +77,7 @@ fn should_fetch_authors_based_on_alias() { assert_eq!( actual_author, - [Author::new(alias, "Name Surname", "someone@users.noreply.github.com")] + [Author::from(alias, "Name Surname", "someone@users.noreply.github.com")] ); } @@ -92,8 +92,8 @@ fn should_fetch_all_authors_for_a_given_alias() { assert_eq!( actual_authors, [ - Author::new(alias, "username", "something@gmail.com"), - Author::new(alias, "username2", "something2@gmail.com"), + Author::from(alias, "username", "something@gmail.com"), + Author::from(alias, "username2", "something2@gmail.com"), ] ); } diff --git a/src/authors/test/integration.rs b/src/authors/test/integration.rs index 84fa32a..948531e 100644 --- a/src/authors/test/integration.rs +++ b/src/authors/test/integration.rs @@ -11,26 +11,26 @@ use crate::{ #[test] #[serial] fn authors_module_should_setup_repo_from_default_file_path_if_present() { - assert!(authors::new_fs_default_setup().is_err()); + assert!(authors::default().is_err()); let default_authors_file_path = conf::authors_file_path(); fs::File::create(&default_authors_file_path).unwrap(); let _after = AfterAssert::cleanup(&[default_authors_file_path.as_str()]); - assert!(authors::new_fs_default_setup().is_ok()); + assert!(authors::default().is_ok()); } #[test] fn authors_module_should_setup_repo_from_given_file_path_if_present() { - assert!(authors::fs_setup_from_file("/tmp/not_real".to_string()).is_err()); + assert!(authors::from_file("/tmp/not_real".to_string()).is_err()); - let result = authors::fs_setup_from_file(conf::dummy_data()); + let result = authors::from_file(conf::dummy_data()); assert!(result.is_ok_and(|service| service.all_available() == [ - Author::new("a", "Name Surname", "someone@users.noreply.github.com"), - Author::new("b", "username", "something@gmail.com"), - Author::new("b", "username2", "something2@gmail.com"), - Author::new("ab", "Another Surname", "someone@something.hi"), + Author::from("a", "Name Surname", "someone@users.noreply.github.com"), + Author::from("b", "username", "something@gmail.com"), + Author::from("b", "username2", "something2@gmail.com"), + Author::from("ab", "Another Surname", "someone@something.hi"), ])); } diff --git a/src/authors/test/service.rs b/src/authors/test/service.rs index 5705d05..e289687 100644 --- a/src/authors/test/service.rs +++ b/src/authors/test/service.rs @@ -38,7 +38,7 @@ impl MockRepo { } fn hardcoded_authors() -> Vec { - Vec::from([Author::new("a", "John", "Doe"), Author::new("b", "Jane", "Smith")]) + Vec::from([Author::from("a", "John", "Doe"), Author::from("b", "Jane", "Smith")]) } } diff --git a/src/cli/test.rs b/src/cli/test.rs index d29ea6f..dacdc9e 100644 --- a/src/cli/test.rs +++ b/src/cli/test.rs @@ -47,7 +47,7 @@ fn should_parse_an_empty_list_of_aliases() { #[test] fn should_format_authors_for_prompt() { - let author = Author::new("alias", "name", "email"); + let author = Author::from("alias", "name", "email"); let prompt = FancyCli::format_author(&author); assert_eq!(strip_ansi::strip_ansi(prompt.as_str()), "⦔ alias -> name"); } diff --git a/src/lib.rs b/src/lib.rs index 362c9f2..275c0dc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,12 +11,13 @@ pub mod cli; pub fn handle_authors(args: &Args, cli: &mut impl Cli) -> Result, Box> { let authors_service = match &args.file { - Some(file) => authors::fs_setup_from_file(file.to_string())?, - None => authors::new_fs_default_setup()?, + Some(file) => authors::from_file(file.to_string())?, + None => authors::default()?, }; if args.all { return match args.sort { + // TODO: get sorted from service true => Ok(sort(authors_service.all_signatures())), false => Ok(authors_service.all_signatures()), }; @@ -44,6 +45,7 @@ pub fn handle_commit_msg(args: &Args, cli: &mut impl Cli, prev: String) -> Resul } } +// TODO: Don't mutate pub fn sort(mut vector: Vec) -> Vec { vector.sort(); vector