fix: Don't use Result for early return in try_fold

Move is_topo_consecutive into revwalk::is_continuous.
Drop single selection  match case, it's handled by catch-all.

Author: Esgariot

asyncgit/src/sync/mod.rs

@@ -24,7 +24,7 @@ mod rebase;
 pub mod remotes;
 mod repository;
 mod reset;
-mod revwalk;
+pub mod revwalk;
 mod reword;
 pub mod sign;
 mod staging;
@@ -90,7 +90,6 @@ pub use remotes::{
 pub(crate) use repository::repo;
 pub use repository::{RepoPath, RepoPathRef};
 pub use reset::{reset_repo, reset_stage, reset_workdir};
-pub use revwalk::revwalk;
 pub use reword::reword;
 pub use staging::{discard_lines, stage_lines};
 pub use stash::{

asyncgit/src/sync/revwalk.rs

@@ -1,4 +1,5 @@
-use std::ops::Bound;
+//! git revwalk utils
+use std::ops::{Bound, ControlFlow};
 
 use crate::Result;
 use git2::{Commit, Oid};
@@ -58,3 +59,32 @@ fn resolve<'r>(
 		Bound::Unbounded => Ok(None),
 	}
 }
+
+/// Checks if `commits` range is continuous under `sort` flags.
+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(
+				Ok(true),
+				|acc, (r, c)| match r
+					.map(CommitId::new)
+					.and_then(|r| acc.map(|acc| acc && (&r == c)))
+				{
+					Ok(true) => ControlFlow::Continue(Ok(true)),
+					otherwise => ControlFlow::Break(otherwise),
+				},
+			) {
+				ControlFlow::Continue(v) | ControlFlow::Break(v) => v,
+			},
+		),
+	}
+}

src/components/commitlist.rs

@@ -29,8 +29,8 @@ use ratatui::{
 	Frame,
 };
 use std::{
-	borrow::Cow, cell::Cell, cmp, collections::BTreeMap, ops::Bound,
-	rc::Rc, time::Instant,
+	borrow::Cow, cell::Cell, cmp, collections::BTreeMap, rc::Rc,
+	time::Instant,
 };
 
 const ELEMENTS_PER_LINE: usize = 9;
@@ -138,45 +138,32 @@ impl CommitList {
 
 	/// Build string of marked or selected (if none are marked) commit ids
 	fn concat_selected_commit_ids(&self) -> Result<Option<String>> {
-		match self.marked.as_slice() {
-			[] => Ok(self
+		let ret = match self.marked.as_slice() {
+			[] => self
 				.items
 				.iter()
 				.nth(
 					self.selection
 						.saturating_sub(self.items.index_offset()),
 				)
-				.map(|e| e.id.to_string())),
-			[(_idx, commit)] => Ok(Some(commit.to_string())),
-			[latest, .., earliest] => {
-				let marked_rev = self.marked.iter().rev();
-				let marked_topo_consecutive = revwalk(
+				.map(|e| e.id.to_string()),
+			[latest, .., earliest]
+				if revwalk::is_continuous(
 					&self.repo.borrow(),
-					Bound::Excluded(&earliest.1),
-					Bound::Included(&latest.1),
-					Sort::TOPOLOGICAL | Sort::REVERSE,
-					|revwalk| {
-						revwalk.zip(marked_rev).try_fold(
-							true,
-							|acc, (r, m)| {
-								let revwalked = CommitId::new(r?);
-								let marked = m.1;
-								Ok(acc && (revwalked == marked))
-							},
-						)
-					},
-				)?;
-				let yank = if marked_topo_consecutive {
-					format!("{}^..{}", earliest.1, latest.1)
-				} else {
-					self.marked
-						.iter()
-						.map(|(_idx, commit)| commit.to_string())
-						.join(" ")
-				};
-				Ok(Some(yank))
+					Sort::TOPOLOGICAL,
+					&self.marked.iter().map(|x| x.1).collect_vec(),
+				)? =>
+			{
+				Some(format!("{}^..{}", earliest.1, latest.1))
 			}
-		}
+			marked => Some(
+				marked
+					.iter()
+					.map(|(_idx, commit)| commit.to_string())
+					.join(" "),
+			),
+		};
+		Ok(ret)
 	}
 
 	/// Copy currently marked or selected (if none are marked) commit ids