feat: inline revwalk into is_continuous

`revwalk` turned out too complicated,
include simplified version into `is_continuous`.
Add tests.

Author: Esgariot

asyncgit/src/sync/revwalk.rs

@@ -1,90 +1,141 @@
 //! git revwalk utils
-use std::ops::{Bound, ControlFlow};
-
-use crate::Result;
-use git2::{Commit, Oid};
-
 use super::{repo, CommitId, RepoPath};
+use crate::Result;
+use git2::Oid;
+use std::ops::ControlFlow;
 
-/// Performs a Git revision walk.
+/// Checks if `commits` range is topologically continuous
 ///
-/// The revwalk is optionally bounded by `start` and `end` commits, sorted according to `sort`.
-/// The revwalk iterator bound by repository's lifetime is exposed through the `iter_fn`.
-pub fn revwalk<R>(
-	repo_path: &RepoPath,
-	start: Bound<&CommitId>,
-	end: Bound<&CommitId>,
-	sort: git2::Sort,
-	iter_fn: impl FnOnce(
-		&mut (dyn Iterator<Item = Result<Oid>>),
-	) -> Result<R>,
-) -> Result<R> {
-	let repo = repo(repo_path)?;
-	let mut revwalk = repo.revwalk()?;
-	revwalk.set_sorting(sort)?;
-	let start = resolve(&repo, start)?;
-	let end = resolve(&repo, end)?;
-
-	if let Some(s) = start {
-		revwalk.hide(s.id())?;
-	}
-	if let Some(e) = end {
-		revwalk.push(e.id())?;
-	}
-	{
-		#![allow(clippy::let_and_return)]
-		let ret = iter_fn(&mut revwalk.map(|r| {
-			r.map_err(|x| crate::Error::Generic(x.to_string()))
-		}));
-		ret
-	}
-}
-
-fn resolve<'r>(
-	repo: &'r git2::Repository,
-	commit: Bound<&CommitId>,
-) -> Result<Option<Commit<'r>>> {
-	match commit {
-		Bound::Included(c) => {
-			let commit = repo.find_commit(c.get_oid())?;
-			Ok(Some(commit))
-		}
-		Bound::Excluded(s) => {
-			let commit = repo.find_commit(s.get_oid())?;
-			let res = (commit.parent_count() == 1)
-				.then(|| commit.parent(0))
-				.transpose()?;
-			Ok(res)
-		}
-		Bound::Unbounded => Ok(None),
-	}
-}
-
-/// Checks if `commits` range is continuous under `sort` flags.
+/// Supports only linear paths - presence of merge commits results in `false`.
 pub fn is_continuous(
 	repo_path: &RepoPath,
-	sort: git2::Sort,
 	commits: &[CommitId],
 ) -> Result<bool> {
 	match commits {
 		[] | [_] => Ok(true),
-		[start, .., end] => revwalk(
-			repo_path,
-			Bound::Excluded(start),
-			Bound::Included(end),
-			sort,
-			|revwalk| match revwalk.zip(commits).try_fold(
+		commits => {
+			let repo = repo(repo_path)?;
+			let mut revwalk = repo.revwalk()?;
+			revwalk.set_sorting(git2::Sort::TOPOLOGICAL)?;
+			revwalk.simplify_first_parent()?;
+			revwalk.push(commits[0].get_oid())?;
+			let revwalked: Vec<Oid> =
+				revwalk
+					.take(commits.len())
+					.collect::<std::result::Result<Vec<_>, _>>()?;
+
+			if revwalked.len() != commits.len() {
+				return Ok(false);
+			}
+
+			match revwalked.iter().zip(commits).try_fold(
 				Ok(true),
-				|acc, (r, c)| match r
-					.map(CommitId::new)
-					.and_then(|r| acc.map(|acc| acc && (&r == c)))
+				|acc, (r, c)| match acc
+					.map(|acc| acc && (&(CommitId::from(*r)) == c))
 				{
-					Ok(true) => ControlFlow::Continue(Ok(true)),
+					ok @ Ok(true) => ControlFlow::Continue(ok),
 					otherwise => ControlFlow::Break(otherwise),
 				},
 			) {
 				ControlFlow::Continue(v) | ControlFlow::Break(v) => v,
-			},
-		),
+			}
+		}
+	}
+}
+#[cfg(test)]
+mod tests_is_continuous {
+	use crate::sync::{
+		checkout_commit, commit, merge_commit,
+		revwalk::is_continuous, tests::repo_init_empty, RepoPath,
+	};
+
+	#[test]
+	fn test_linear_commits_are_continuous() {
+		// * c3 (HEAD)
+		// * c2
+		// * c1
+		// * c0 (root)
+
+		let (_td, repo) = repo_init_empty().unwrap();
+		let root = repo.path().parent().unwrap();
+		let repo_path: &RepoPath =
+			&root.as_os_str().to_str().unwrap().into();
+		let _c0 = commit(repo_path, "commit 0").unwrap();
+		let c1 = commit(repo_path, "commit 1").unwrap();
+		let c2 = commit(repo_path, "commit 2").unwrap();
+		let c3 = commit(repo_path, "commit 3").unwrap();
+
+		let result = is_continuous(repo_path, &[c3, c2, c1]).unwrap();
+		assert!(result, "linear commits should be continuous");
+
+		let result = is_continuous(repo_path, &[c2]).unwrap();
+		assert!(result, "single commit should be continuous");
+
+		let result = is_continuous(repo_path, &[]).unwrap();
+		assert!(result, "empty range should be continuous");
+	}
+
+	#[test]
+	fn test_merge_commits_break_continuity() {
+		// *   c4 (HEAD)
+		// |\
+		// | * c3
+		// * | c2
+		// |/
+		// * c1
+		// * c0 (root)
+
+		let (_td, repo) = repo_init_empty().unwrap();
+		let root = repo.path().parent().unwrap();
+		let repo_path: &RepoPath =
+			&root.as_os_str().to_str().unwrap().into();
+
+		let _c0 = commit(repo_path, "commit 0").unwrap();
+		let c1 = commit(repo_path, "commit 1").unwrap();
+		let c2 = commit(repo_path, "commit 2").unwrap();
+
+		checkout_commit(repo_path, c1).unwrap();
+		let c3 = commit(repo_path, "commit 3").unwrap();
+
+		let c4 =
+			merge_commit(repo_path, "commit 4", &[c2, c3]).unwrap();
+
+		let result = is_continuous(repo_path, &[c4, c2, c1]).unwrap();
+		assert!(
+			!result,
+			"range including merge should not be continuous"
+		);
+
+		let result = is_continuous(repo_path, &[c2, c1]).unwrap();
+		assert!(
+			result,
+			"linear range before merge should be continuous"
+		);
+	}
+
+	#[test]
+	fn test_non_continuous_commits() {
+		// * c4 (HEAD)
+		// * c3
+		// * c2
+		// * c1
+		// * c0 (root)
+
+		let (_td, repo) = repo_init_empty().unwrap();
+		let root = repo.path().parent().unwrap();
+		let repo_path: &RepoPath =
+			&root.as_os_str().to_str().unwrap().into();
+
+		let _c0 = commit(repo_path, "commit 0").unwrap();
+		let c1 = commit(repo_path, "commit 1").unwrap();
+		let c2 = commit(repo_path, "commit 2").unwrap();
+		let c3 = commit(repo_path, "commit 3").unwrap();
+		let c4 = commit(repo_path, "commit 4").unwrap();
+
+		let result = is_continuous(repo_path, &[c4, c3, c1]).unwrap();
+		assert!(!result, "commit range with gap should return false");
+
+		let result = is_continuous(repo_path, &[c1, c2, c3]).unwrap();
+		assert!(!result, "wrong order should return false");
 	}
 }

src/components/commitlist.rs

@@ -15,7 +15,7 @@ use crate::{
 use anyhow::Result;
 use asyncgit::sync::{
 	self, checkout_commit, revwalk, BranchDetails, BranchInfo,
-	CommitId, RepoPathRef, Sort, Tags,
+	CommitId, RepoPathRef, Tags,
 };
 use chrono::{DateTime, Local};
 use crossterm::event::Event;
@@ -144,7 +144,6 @@ impl CommitList {
 			[latest, .., earliest]
 				if revwalk::is_continuous(
 					&self.repo.borrow(),
-					Sort::TOPOLOGICAL,
 					&self.marked.iter().map(|x| x.1).collect_vec(),
 				)? =>
 			{