refactoring commit filter (#1843)

asyncgit/src/filter_commits.rs

@@ -1,7 +1,7 @@
 use crate::{
 	asyncjob::{AsyncJob, RunParams},
 	error::Result,
-	sync::{self, CommitId, LogWalkerFilter, RepoPath},
+	sync::{self, CommitId, RepoPath, SharedCommitFilterFn},
 	AsyncGitNotification, ProgressPercent,
 };
 use std::{
@@ -29,7 +29,7 @@ enum JobState {
 #[derive(Clone)]
 pub struct AsyncCommitFilterJob {
 	state: Arc<Mutex<Option<JobState>>>,
-	filter: LogWalkerFilter,
+	filter: SharedCommitFilterFn,
 }
 
 ///
@@ -38,7 +38,7 @@ impl AsyncCommitFilterJob {
 	pub fn new(
 		repo_path: RepoPath,
 		commits: Vec<CommitId>,
-		filter: LogWalkerFilter,
+		filter: SharedCommitFilterFn,
 	) -> Self {
 		Self {
 			state: Arc::new(Mutex::new(Some(JobState::Request {

asyncgit/src/revlog.rs

@@ -1,6 +1,8 @@
 use crate::{
 	error::Result,
-	sync::{repo, CommitId, LogWalker, LogWalkerFilter, RepoPath},
+	sync::{
+		repo, CommitId, LogWalker, RepoPath, SharedCommitFilterFn,
+	},
 	AsyncGitNotification, Error,
 };
 use crossbeam_channel::Sender;
@@ -39,7 +41,7 @@ pub struct AsyncLog {
 	sender: Sender<AsyncGitNotification>,
 	pending: Arc<AtomicBool>,
 	background: Arc<AtomicBool>,
-	filter: Option<LogWalkerFilter>,
+	filter: Option<SharedCommitFilterFn>,
 	partial_extract: AtomicBool,
 	repo: RepoPath,
 }
@@ -53,7 +55,7 @@ impl AsyncLog {
 	pub fn new(
 		repo: RepoPath,
 		sender: &Sender<AsyncGitNotification>,
-		filter: Option<LogWalkerFilter>,
+		filter: Option<SharedCommitFilterFn>,
 	) -> Self {
 		Self {
 			repo,
@@ -195,7 +197,7 @@ impl AsyncLog {
 		arc_current: &Arc<Mutex<AsyncLogResult>>,
 		arc_background: &Arc<AtomicBool>,
 		sender: &Sender<AsyncGitNotification>,
-		filter: Option<LogWalkerFilter>,
+		filter: Option<SharedCommitFilterFn>,
 	) -> Result<()> {
 		let start_time = Instant::now();
 

asyncgit/src/sync/commit_filter.rs

@@ -0,0 +1,209 @@
+use super::{commit_files::get_commit_diff, CommitId};
+use crate::error::Result;
+use bitflags::bitflags;
+use fuzzy_matcher::FuzzyMatcher;
+use git2::{Diff, Repository};
+use std::sync::Arc;
+
+///
+pub type SharedCommitFilterFn = Arc<
+	Box<dyn Fn(&Repository, &CommitId) -> Result<bool> + Send + Sync>,
+>;
+
+///
+pub fn diff_contains_file(file_path: String) -> SharedCommitFilterFn {
+	Arc::new(Box::new(
+		move |repo: &Repository,
+		      commit_id: &CommitId|
+		      -> Result<bool> {
+			let diff = get_commit_diff(
+				repo,
+				*commit_id,
+				Some(file_path.clone()),
+				None,
+				None,
+			)?;
+
+			let contains_file = diff.deltas().len() > 0;
+
+			Ok(contains_file)
+		},
+	))
+}
+
+bitflags! {
+	///
+	pub struct SearchFields: u32 {
+		///
+		const MESSAGE = 1 << 0;
+		///
+		const FILENAMES = 1 << 1;
+		///
+		const AUTHORS = 1 << 2;
+		//TODO:
+		// const COMMIT_HASHES = 1 << 3;
+		// ///
+		// const DATES = 1 << 4;
+		// ///
+		// const DIFFS = 1 << 5;
+	}
+}
+
+impl Default for SearchFields {
+	fn default() -> Self {
+		Self::MESSAGE
+	}
+}
+
+bitflags! {
+	///
+	pub struct SearchOptions: u32 {
+		///
+		const CASE_SENSITIVE = 1 << 0;
+		///
+		const FUZZY_SEARCH = 1 << 1;
+	}
+}
+
+impl Default for SearchOptions {
+	fn default() -> Self {
+		Self::empty()
+	}
+}
+
+///
+#[derive(Default, Debug, Clone)]
+pub struct LogFilterSearchOptions {
+	///
+	pub search_pattern: String,
+	///
+	pub fields: SearchFields,
+	///
+	pub options: SearchOptions,
+}
+
+///
+#[derive(Default)]
+pub struct LogFilterSearch {
+	///
+	pub matcher: fuzzy_matcher::skim::SkimMatcherV2,
+	///
+	pub options: LogFilterSearchOptions,
+}
+
+impl LogFilterSearch {
+	///
+	pub fn new(options: LogFilterSearchOptions) -> Self {
+		let mut options = options;
+		if !options.options.contains(SearchOptions::CASE_SENSITIVE) {
+			options.search_pattern =
+				options.search_pattern.to_lowercase();
+		}
+		Self {
+			matcher: fuzzy_matcher::skim::SkimMatcherV2::default(),
+			options,
+		}
+	}
+
+	fn match_diff(&self, diff: &Diff<'_>) -> bool {
+		diff.deltas().any(|delta| {
+			if delta
+				.new_file()
+				.path()
+				.and_then(|file| file.as_os_str().to_str())
+				.map(|file| self.match_text(file))
+				.unwrap_or_default()
+			{
+				return true;
+			}
+
+			delta
+				.old_file()
+				.path()
+				.and_then(|file| file.as_os_str().to_str())
+				.map(|file| self.match_text(file))
+				.unwrap_or_default()
+		})
+	}
+
+	///
+	pub fn match_text(&self, text: &str) -> bool {
+		if self.options.options.contains(SearchOptions::FUZZY_SEARCH)
+		{
+			self.matcher
+				.fuzzy_match(
+					text,
+					self.options.search_pattern.as_str(),
+				)
+				.is_some()
+		} else if self
+			.options
+			.options
+			.contains(SearchOptions::CASE_SENSITIVE)
+		{
+			text.contains(self.options.search_pattern.as_str())
+		} else {
+			text.to_lowercase()
+				.contains(self.options.search_pattern.as_str())
+		}
+	}
+}
+
+///
+pub fn filter_commit_by_search(
+	filter: LogFilterSearch,
+) -> SharedCommitFilterFn {
+	Arc::new(Box::new(
+		move |repo: &Repository,
+		      commit_id: &CommitId|
+		      -> Result<bool> {
+			let commit = repo.find_commit((*commit_id).into())?;
+
+			let msg_match = filter
+				.options
+				.fields
+				.contains(SearchFields::MESSAGE)
+				.then(|| {
+					commit.message().map(|msg| filter.match_text(msg))
+				})
+				.flatten()
+				.unwrap_or_default();
+
+			let file_match = filter
+				.options
+				.fields
+				.contains(SearchFields::FILENAMES)
+				.then(|| {
+					get_commit_diff(
+						repo, *commit_id, None, None, None,
+					)
+					.ok()
+				})
+				.flatten()
+				.map(|diff| filter.match_diff(&diff))
+				.unwrap_or_default();
+
+			let authors_match = filter
+				.options
+				.fields
+				.contains(SearchFields::AUTHORS)
+				.then(|| {
+					let name_match = commit
+						.author()
+						.name()
+						.map(|name| filter.match_text(name))
+						.unwrap_or_default();
+					let mail_match = commit
+						.author()
+						.email()
+						.map(|name| filter.match_text(name))
+						.unwrap_or_default();
+
+					name_match || mail_match
+				})
+				.unwrap_or_default();
+
+			Ok(msg_match || file_match || authors_match)
+		},
+	))
+}

asyncgit/src/sync/logwalker.rs

@@ -1,13 +1,10 @@
 #![allow(dead_code)]
-use super::CommitId;
-use crate::{error::Result, sync::commit_files::get_commit_diff};
-use bitflags::bitflags;
-use fuzzy_matcher::FuzzyMatcher;
-use git2::{Commit, Diff, Oid, Repository};
+use super::{CommitId, SharedCommitFilterFn};
+use crate::error::Result;
+use git2::{Commit, Oid, Repository};
 use std::{
 	cmp::Ordering,
 	collections::{BinaryHeap, HashSet},
-	sync::Arc,
 };
 
 struct TimeOrderedCommit<'a>(Commit<'a>);
@@ -32,217 +29,13 @@ impl<'a> Ord for TimeOrderedCommit<'a> {
 	}
 }
 
-//TODO: since its used in more than just the log walker now, we should rename and put in its own file
-///
-pub type LogWalkerFilter = Arc<
-	Box<dyn Fn(&Repository, &CommitId) -> Result<bool> + Send + Sync>,
->;
-
-///
-pub fn diff_contains_file(file_path: String) -> LogWalkerFilter {
-	Arc::new(Box::new(
-		move |repo: &Repository,
-		      commit_id: &CommitId|
-		      -> Result<bool> {
-			let diff = get_commit_diff(
-				repo,
-				*commit_id,
-				Some(file_path.clone()),
-				None,
-				None,
-			)?;
-
-			let contains_file = diff.deltas().len() > 0;
-
-			Ok(contains_file)
-		},
-	))
-}
-
-bitflags! {
-	///
-	pub struct SearchFields: u32 {
-		///
-		const MESSAGE = 0b0000_0001;
-		///
-		const FILENAMES = 0b0000_0010;
-		///
-		const AUTHORS = 0b0000_0100;
-		//TODO:
-		// const COMMIT_HASHES = 0b0000_0100;
-		// ///
-		// const DATES = 0b0000_1000;
-		// ///
-		// const DIFFS = 0b0010_0000;
-	}
-}
-
-impl Default for SearchFields {
-	fn default() -> Self {
-		Self::MESSAGE
-	}
-}
-
-bitflags! {
-	///
-	pub struct SearchOptions: u32 {
-		///
-		const CASE_SENSITIVE = 0b0000_0001;
-		///
-		const FUZZY_SEARCH = 0b0000_0010;
-	}
-}
-
-impl Default for SearchOptions {
-	fn default() -> Self {
-		Self::empty()
-	}
-}
-
-///
-#[derive(Default, Debug, Clone)]
-pub struct LogFilterSearchOptions {
-	///
-	pub search_pattern: String,
-	///
-	pub fields: SearchFields,
-	///
-	pub options: SearchOptions,
-}
-
-///
-#[derive(Default)]
-pub struct LogFilterSearch {
-	///
-	pub matcher: fuzzy_matcher::skim::SkimMatcherV2,
-	///
-	pub options: LogFilterSearchOptions,
-}
-
-impl LogFilterSearch {
-	///
-	pub fn new(options: LogFilterSearchOptions) -> Self {
-		let mut options = options;
-		if !options.options.contains(SearchOptions::CASE_SENSITIVE) {
-			options.search_pattern =
-				options.search_pattern.to_lowercase();
-		}
-		Self {
-			matcher: fuzzy_matcher::skim::SkimMatcherV2::default(),
-			options,
-		}
-	}
-
-	fn match_diff(&self, diff: &Diff<'_>) -> bool {
-		diff.deltas().any(|delta| {
-			if delta
-				.new_file()
-				.path()
-				.and_then(|file| file.as_os_str().to_str())
-				.map(|file| self.match_text(file))
-				.unwrap_or_default()
-			{
-				return true;
-			}
-
-			delta
-				.old_file()
-				.path()
-				.and_then(|file| file.as_os_str().to_str())
-				.map(|file| self.match_text(file))
-				.unwrap_or_default()
-		})
-	}
-
-	///
-	pub fn match_text(&self, text: &str) -> bool {
-		if self.options.options.contains(SearchOptions::FUZZY_SEARCH)
-		{
-			self.matcher
-				.fuzzy_match(
-					text,
-					self.options.search_pattern.as_str(),
-				)
-				.is_some()
-		} else if self
-			.options
-			.options
-			.contains(SearchOptions::CASE_SENSITIVE)
-		{
-			text.contains(self.options.search_pattern.as_str())
-		} else {
-			text.to_lowercase()
-				.contains(self.options.search_pattern.as_str())
-		}
-	}
-}
-
-///
-pub fn filter_commit_by_search(
-	filter: LogFilterSearch,
-) -> LogWalkerFilter {
-	Arc::new(Box::new(
-		move |repo: &Repository,
-		      commit_id: &CommitId|
-		      -> Result<bool> {
-			let commit = repo.find_commit((*commit_id).into())?;
-
-			let msg_match = filter
-				.options
-				.fields
-				.contains(SearchFields::MESSAGE)
-				.then(|| {
-					commit.message().map(|msg| filter.match_text(msg))
-				})
-				.flatten()
-				.unwrap_or_default();
-
-			let file_match = filter
-				.options
-				.fields
-				.contains(SearchFields::FILENAMES)
-				.then(|| {
-					get_commit_diff(
-						repo, *commit_id, None, None, None,
-					)
-					.ok()
-				})
-				.flatten()
-				.map(|diff| filter.match_diff(&diff))
-				.unwrap_or_default();
-
-			let authors_match = filter
-				.options
-				.fields
-				.contains(SearchFields::AUTHORS)
-				.then(|| {
-					let name_match = commit
-						.author()
-						.name()
-						.map(|name| filter.match_text(name))
-						.unwrap_or_default();
-					let mail_match = commit
-						.author()
-						.email()
-						.map(|name| filter.match_text(name))
-						.unwrap_or_default();
-
-					name_match || mail_match
-				})
-				.unwrap_or_default();
-
-			Ok(msg_match || file_match || authors_match)
-		},
-	))
-}
-
 ///
 pub struct LogWalker<'a> {
 	commits: BinaryHeap<TimeOrderedCommit<'a>>,
 	visited: HashSet<Oid>,
 	limit: usize,
 	repo: &'a Repository,
-	filter: Option<LogWalkerFilter>,
+	filter: Option<SharedCommitFilterFn>,
 }
 
 impl<'a> LogWalker<'a> {
@@ -269,7 +62,10 @@ impl<'a> LogWalker<'a> {
 
 	///
 	#[must_use]
-	pub fn filter(self, filter: Option<LogWalkerFilter>) -> Self {
+	pub fn filter(
+		self,
+		filter: Option<SharedCommitFilterFn>,
+	) -> Self {
 		Self { filter, ..self }
 	}
 
@@ -316,12 +112,16 @@ impl<'a> LogWalker<'a> {
 mod tests {
 	use super::*;
 	use crate::error::Result;
+	use crate::sync::commit_filter::{SearchFields, SearchOptions};
 	use crate::sync::tests::write_commit_file;
-	use crate::sync::RepoPath;
 	use crate::sync::{
 		commit, get_commits_info, stage_add_file,
 		tests::repo_init_empty,
 	};
+	use crate::sync::{
+		diff_contains_file, filter_commit_by_search, LogFilterSearch,
+		LogFilterSearchOptions, RepoPath,
+	};
 	use pretty_assertions::assert_eq;
 	use std::{fs::File, io::Write, path::Path};
 

asyncgit/src/sync/mod.rs

@@ -8,6 +8,7 @@ pub mod branch;
 mod commit;
 mod commit_details;
 pub mod commit_files;
+mod commit_filter;
 mod commit_revert;
 mod commits_info;
 mod config;
@@ -48,6 +49,11 @@ pub use commit_details::{
 	get_commit_details, CommitDetails, CommitMessage, CommitSignature,
 };
 pub use commit_files::get_commit_files;
+pub use commit_filter::{
+	diff_contains_file, filter_commit_by_search, LogFilterSearch,
+	LogFilterSearchOptions, SearchFields, SearchOptions,
+	SharedCommitFilterFn,
+};
 pub use commit_revert::{commit_revert, revert_commit, revert_head};
 pub use commits_info::{
 	get_commit_info, get_commits_info, CommitId, CommitInfo,
@@ -63,11 +69,7 @@ pub use hooks::{
 };
 pub use hunks::{reset_hunk, stage_hunk, unstage_hunk};
 pub use ignore::add_to_ignore;
-pub use logwalker::{
-	diff_contains_file, filter_commit_by_search, LogFilterSearch,
-	LogFilterSearchOptions, LogWalker, LogWalkerFilter, SearchFields,
-	SearchOptions,
-};
+pub use logwalker::LogWalker;
 pub use merge::{
 	abort_pending_rebase, abort_pending_state,
 	continue_pending_rebase, merge_branch, merge_commit, merge_msg,