From f1905dcaf4cba63c6becea0b93abec6c0d57384f Mon Sep 17 00:00:00 2001 From: Ossama Hjaji Date: Tue, 28 Nov 2023 19:08:14 +0100 Subject: [PATCH] Refactoring git metrics module (#1217) * refacot * add break line --- src/info/authors.rs | 98 ++++++++++++++++++- src/info/churn.rs | 95 +++++++++++++++++-- src/info/contributors.rs | 20 +--- src/info/git/metrics.rs | 177 +---------------------------------- src/info/git/mod.rs | 28 +++--- src/info/langs/language.rs | 18 ++-- src/info/license.rs | 3 +- src/info/loc.rs | 6 +- src/info/mod.rs | 75 ++++++++++++--- src/info/utils/info_field.rs | 3 +- src/info/utils/mod.rs | 9 +- 11 files changed, 284 insertions(+), 248 deletions(-) diff --git a/src/info/authors.rs b/src/info/authors.rs index 35f3cf0b9..8d0579a77 100644 --- a/src/info/authors.rs +++ b/src/info/authors.rs @@ -1,10 +1,10 @@ -use super::git::metrics::GitMetrics; +use super::git::sig::Sig; use crate::{ cli::NumberSeparator, info::utils::{format_number, info_field::InfoField}, }; use serde::Serialize; -use std::fmt::Write; +use std::{collections::HashMap, fmt::Write}; #[derive(Serialize, Clone, Debug, PartialEq)] #[serde(rename_all = "camelCase")] @@ -66,8 +66,20 @@ pub struct AuthorsInfo { } impl AuthorsInfo { - pub fn new(git_metrics: &GitMetrics) -> Self { - let authors = git_metrics.authors_to_display.clone(); + pub fn new( + number_of_commits_by_signature: &HashMap, + total_number_of_commits: usize, + number_of_authors_to_display: usize, + show_email: bool, + number_separator: NumberSeparator, + ) -> Self { + let authors = compute_authors( + number_of_commits_by_signature, + total_number_of_commits, + number_of_authors_to_display, + show_email, + number_separator, + ); Self { authors } } @@ -79,6 +91,40 @@ impl AuthorsInfo { } } +fn compute_authors( + number_of_commits_by_signature: &HashMap, + total_number_of_commits: usize, + number_of_authors_to_display: usize, + show_email: bool, + number_separator: NumberSeparator, +) -> Vec { + let mut signature_with_number_of_commits_sorted: Vec<(&Sig, &usize)> = + Vec::from_iter(number_of_commits_by_signature); + + signature_with_number_of_commits_sorted.sort_by(|(sa, a_count), (sb, b_count)| { + b_count.cmp(a_count).then_with(|| sa.name.cmp(&sb.name)) + }); + + let authors: Vec = signature_with_number_of_commits_sorted + .into_iter() + .map(|(author, author_nbr_of_commits)| { + Author::new( + author.name.to_string(), + if show_email { + Some(author.email.to_string()) + } else { + None + }, + *author_nbr_of_commits, + total_number_of_commits, + number_separator, + ) + }) + .take(number_of_authors_to_display) + .collect(); + authors +} + fn digit_difference(num1: usize, num2: usize) -> usize { let count_digits = |num: usize| (num.checked_ilog10().unwrap_or(0) + 1) as usize; count_digits(num1).abs_diff(count_digits(num2)) @@ -285,4 +331,48 @@ mod test { let result = digit_difference(num1, num2); assert_eq!(result, expected); } + + #[test] + fn test_compute_authors() { + let mut number_of_commits_by_signature: HashMap = HashMap::new(); + number_of_commits_by_signature.insert( + Sig { + name: "John Doe".into(), + email: "johndoe@example.com".into(), + }, + 30, + ); + number_of_commits_by_signature.insert( + Sig { + name: "Jane Doe".into(), + email: "janedoe@example.com".into(), + }, + 20, + ); + number_of_commits_by_signature.insert( + Sig { + name: "Ellen Smith".into(), + email: "ellensmith@example.com".into(), + }, + 50, + ); + let total_number_of_commits = 100; + let number_of_authors_to_display = 2; + let show_email = false; + let number_separator = NumberSeparator::Comma; + + let actual = compute_authors( + &number_of_commits_by_signature, + total_number_of_commits, + number_of_authors_to_display, + show_email, + number_separator, + ); + + let expected = vec![ + Author::new(String::from("Ellen Smith"), None, 50, 100, number_separator), + Author::new(String::from("John Doe"), None, 30, 100, number_separator), + ]; + assert_eq!(actual, expected); + } } diff --git a/src/info/churn.rs b/src/info/churn.rs index 0f9547889..48012c654 100644 --- a/src/info/churn.rs +++ b/src/info/churn.rs @@ -1,7 +1,10 @@ -use super::{git::metrics::GitMetrics, utils::info_field::InfoField}; +use super::utils::info_field::InfoField; use crate::{cli::NumberSeparator, info::utils::format_number}; +use anyhow::Result; +use gix::bstr::BString; +use globset::{Glob, GlobSetBuilder}; use serde::Serialize; -use std::fmt::Write; +use std::{collections::HashMap, fmt::Write}; #[derive(Serialize, Clone, Debug, PartialEq)] #[serde(rename_all = "camelCase")] @@ -42,15 +45,62 @@ pub struct ChurnInfo { pub file_churns: Vec, pub churn_pool_size: usize, } + impl ChurnInfo { - pub fn new(git_metrics: &GitMetrics) -> Self { - let file_churns = git_metrics.file_churns_to_display.clone(); - Self { + pub fn new( + number_of_commits_by_file_path: &HashMap, + churn_pool_size: usize, + number_of_file_churns_to_display: usize, + globs_to_exclude: &[String], + number_separator: NumberSeparator, + ) -> Result { + let file_churns = compute_file_churns( + number_of_commits_by_file_path, + number_of_file_churns_to_display, + globs_to_exclude, + number_separator, + )?; + + Ok(Self { file_churns, - churn_pool_size: git_metrics.churn_pool_size, - } + churn_pool_size, + }) + } +} + +fn compute_file_churns( + number_of_commits_by_file_path: &HashMap, + number_of_file_churns_to_display: usize, + globs_to_exclude: &[String], + number_separator: NumberSeparator, +) -> Result> { + let mut builder = GlobSetBuilder::new(); + for glob in globs_to_exclude { + builder.add(Glob::new(glob)?); } + let glob_set = builder.build()?; + let mut number_of_commits_by_file_path_sorted = Vec::from_iter(number_of_commits_by_file_path); + + number_of_commits_by_file_path_sorted + .sort_by(|(_, a_count), (_, b_count)| b_count.cmp(a_count)); + + Ok(number_of_commits_by_file_path_sorted + .into_iter() + .filter_map(|(file_path, nbr_of_commits)| { + if !glob_set.is_match(file_path.to_string()) { + Some(FileChurn::new( + file_path.to_string(), + *nbr_of_commits, + number_separator, + )) + } else { + None + } + }) + .take(number_of_file_churns_to_display) + .collect()) } + impl std::fmt::Display for ChurnInfo { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { let mut churn_info = String::new(); @@ -139,4 +189,35 @@ mod tests { ); assert_eq!(shorten_file_path("file.txt", 0), "file.txt"); } + + #[test] + fn test_compute_file_churns() -> Result<()> { + let mut number_of_commits_by_file_path = HashMap::new(); + number_of_commits_by_file_path.insert("path/to/file1.txt".into(), 2); + number_of_commits_by_file_path.insert("path/to/file2.txt".into(), 5); + number_of_commits_by_file_path.insert("path/to/file3.txt".into(), 3); + number_of_commits_by_file_path.insert("path/to/file4.txt".into(), 7); + number_of_commits_by_file_path.insert("foo/x/y/file.txt".into(), 70); + number_of_commits_by_file_path.insert("foo/x/file.txt".into(), 10); + + let number_of_file_churns_to_display = 3; + let number_separator = NumberSeparator::Comma; + let globs_to_exclude = vec![ + "foo/**/file.txt".to_string(), + "path/to/file2.txt".to_string(), + ]; + let actual = compute_file_churns( + &number_of_commits_by_file_path, + number_of_file_churns_to_display, + &globs_to_exclude, + number_separator, + )?; + let expected = vec![ + FileChurn::new(String::from("path/to/file4.txt"), 7, number_separator), + FileChurn::new(String::from("path/to/file3.txt"), 3, number_separator), + FileChurn::new(String::from("path/to/file1.txt"), 2, number_separator), + ]; + assert_eq!(actual, expected); + Ok(()) + } } diff --git a/src/info/contributors.rs b/src/info/contributors.rs index 252a127fc..0ae8d3895 100644 --- a/src/info/contributors.rs +++ b/src/info/contributors.rs @@ -1,4 +1,4 @@ -use super::{git::metrics::GitMetrics, utils::format_number}; +use super::utils::format_number; use crate::{cli::NumberSeparator, info::utils::info_field::InfoField}; use serde::Serialize; @@ -14,12 +14,12 @@ pub struct ContributorsInfo { impl ContributorsInfo { pub fn new( - git_metrics: &GitMetrics, + total_number_of_authors: usize, number_of_authors_to_display: usize, number_separator: NumberSeparator, ) -> Self { Self { - total_number_of_authors: git_metrics.total_number_of_authors, + total_number_of_authors, number_of_authors_to_display, number_separator, } @@ -44,22 +44,10 @@ impl InfoField for ContributorsInfo { #[cfg(test)] mod test { use super::*; - use gix::date::Time; #[test] fn test_display_contributors_info() { - let timestamp = Time::now_utc(); - let git_metrics = GitMetrics { - authors_to_display: vec![], - file_churns_to_display: vec![], - total_number_of_authors: 12, - total_number_of_commits: 2, - churn_pool_size: 0, - time_of_most_recent_commit: timestamp, - time_of_first_commit: timestamp, - }; - - let contributors_info = ContributorsInfo::new(&git_metrics, 2, NumberSeparator::Plain); + let contributors_info = ContributorsInfo::new(12, 2, NumberSeparator::Plain); assert_eq!(contributors_info.value(), "12".to_string()); assert_eq!(contributors_info.title(), "Contributors".to_string()); } diff --git a/src/info/git/metrics.rs b/src/info/git/metrics.rs index 4158170ea..a9da66abf 100644 --- a/src/info/git/metrics.rs +++ b/src/info/git/metrics.rs @@ -1,16 +1,12 @@ use super::sig::Sig; -use crate::cli::{CliOptions, NumberSeparator}; -use crate::info::authors::Author; -use crate::info::churn::FileChurn; use anyhow::Result; use gix::bstr::BString; use gix::date::Time; -use globset::{Glob, GlobSetBuilder}; use std::collections::HashMap; pub struct GitMetrics { - pub authors_to_display: Vec, - pub file_churns_to_display: Vec, + pub number_of_commits_by_signature: HashMap, + pub number_of_commits_by_file_path: HashMap, pub total_number_of_authors: usize, pub total_number_of_commits: usize, pub churn_pool_size: usize, @@ -25,23 +21,9 @@ impl GitMetrics { churn_pool_size: usize, time_of_first_commit: Option