compare two commits (#860)

Author: Stephan Dilly

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+**compare commits**
+
+![compare](assets/compare.gif)
+
 **options**
 
 ![options](assets/options.gif)
@@ -20,7 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ![name-validation](assets/branch-validation.gif)
 
 ## Added
-- allow opening top commit of a branch
+- allow inspecting top commit of a branch from list
+- compare commits in revlog and head against branch ([#852](https://github.com/extrawurst/gitui/issues/852))
 - new options popup (show untracked files, diff settings) ([#849](https://github.com/extrawurst/gitui/issues/849))
 - mark and drop multiple stashes ([#854](https://github.com/extrawurst/gitui/issues/854))
 - check branch name validity while typing ([#559](https://github.com/extrawurst/gitui/issues/559))

assets/compare.gif

@@ -12,9 +12,34 @@ use std::sync::{
 type ResultType = Vec<StatusItem>;
 struct Request<R, A>(R, A);
 
+///
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub struct CommitFilesParams {
+	///
+	pub id: CommitId,
+	///
+	pub other: Option<CommitId>,
+}
+
+impl From<CommitId> for CommitFilesParams {
+	fn from(id: CommitId) -> Self {
+		Self { id, other: None }
+	}
+}
+
+impl From<(CommitId, CommitId)> for CommitFilesParams {
+	fn from((id, other): (CommitId, CommitId)) -> Self {
+		Self {
+			id,
+			other: Some(other),
+		}
+	}
+}
+
 ///
 pub struct AsyncCommitFiles {
-	current: Arc<Mutex<Option<Request<CommitId, ResultType>>>>,
+	current:
+		Arc<Mutex<Option<Request<CommitFilesParams, ResultType>>>>,
 	sender: Sender<AsyncGitNotification>,
 	pending: Arc<AtomicUsize>,
 }
@@ -32,7 +57,7 @@ impl AsyncCommitFiles {
 	///
 	pub fn current(
 		&mut self,
-	) -> Result<Option<(CommitId, ResultType)>> {
+	) -> Result<Option<(CommitFilesParams, ResultType)>> {
 		let c = self.current.lock()?;
 
 		c.as_ref()
@@ -45,17 +70,17 @@ impl AsyncCommitFiles {
 	}
 
 	///
-	pub fn fetch(&mut self, id: CommitId) -> Result<()> {
+	pub fn fetch(&mut self, params: CommitFilesParams) -> Result<()> {
 		if self.is_pending() {
 			return Ok(());
 		}
 
-		log::trace!("request: {}", id.to_string());
+		log::trace!("request: {:?}", params);
 
 		{
 			let current = self.current.lock()?;
 			if let Some(c) = &*current {
-				if c.0 == id {
+				if c.0 == params {
 					return Ok(());
 				}
 			}
@@ -68,7 +93,7 @@ impl AsyncCommitFiles {
 		self.pending.fetch_add(1, Ordering::Relaxed);
 
 		rayon_core::spawn(move || {
-			Self::fetch_helper(id, &arc_current)
+			Self::fetch_helper(params, &arc_current)
 				.expect("failed to fetch");
 
 			arc_pending.fetch_sub(1, Ordering::Relaxed);
@@ -82,22 +107,19 @@ impl AsyncCommitFiles {
 	}
 
 	fn fetch_helper(
-		id: CommitId,
+		params: CommitFilesParams,
 		arc_current: &Arc<
-			Mutex<Option<Request<CommitId, ResultType>>>,
+			Mutex<Option<Request<CommitFilesParams, ResultType>>>,
 		>,
 	) -> Result<()> {
-		let res = sync::get_commit_files(CWD, id)?;
+		let res =
+			sync::get_commit_files(CWD, params.id, params.other)?;
 
-		log::trace!(
-			"get_commit_files: {} ({})",
-			id.to_string(),
-			res.len()
-		);
+		log::trace!("get_commit_files: {:?} ({})", params, res.len());
 
 		{
 			let mut current = arc_current.lock()?;
-			*current = Some(Request(id, res));
+			*current = Some(Request(params, res));
 		}
 
 		Ok(())

asyncgit/src/commit_files.rs

@@ -16,6 +16,8 @@ use std::{
 ///
 #[derive(Debug, Hash, Clone, PartialEq)]
 pub enum DiffType {
+	/// diff two commits
+	Commits((CommitId, CommitId)),
 	/// diff in a given commit
 	Commit(CommitId),
 	/// diff against staged file
@@ -167,6 +169,11 @@ impl AsyncDiff {
 				id,
 				params.path.clone(),
 			)?,
+			DiffType::Commits(ids) => sync::diff::get_diff_commits(
+				CWD,
+				ids,
+				params.path.clone(),
+			)?,
 		};
 
 		let mut notify = false;

asyncgit/src/diff.rs

@@ -41,7 +41,7 @@ mod tags;
 
 pub use crate::{
 	blame::{AsyncBlame, BlameParams},
-	commit_files::AsyncCommitFiles,
+	commit_files::{AsyncCommitFiles, CommitFilesParams},
 	diff::{AsyncDiff, DiffParams, DiffType},
 	fetch::{AsyncFetch, FetchRequest},
 	push::{AsyncPush, PushRequest},

asyncgit/src/lib.rs

@@ -205,7 +205,7 @@ mod tests {
 		let details = get_commit_details(repo_path, new_id)?;
 		assert_eq!(details.message.unwrap().subject, "amended");
 
-		let files = get_commit_files(repo_path, new_id)?;
+		let files = get_commit_files(repo_path, new_id, None)?;
 
 		assert_eq!(files.len(), 2);
 

asyncgit/src/sync/commit.rs

@@ -1,3 +1,5 @@
+use std::cmp::Ordering;
+
 use super::{stash::is_stash_commit, utils::repo, CommitId};
 use crate::{
 	error::Error, error::Result, StatusItem, StatusItemType,
@@ -9,12 +11,17 @@ use scopetime::scope_time;
 pub fn get_commit_files(
 	repo_path: &str,
 	id: CommitId,
+	other: Option<CommitId>,
 ) -> Result<Vec<StatusItem>> {
 	scope_time!("get_commit_files");
 
 	let repo = repo(repo_path)?;
 
-	let diff = get_commit_diff(&repo, id, None)?;
+	let diff = if let Some(other) = other {
+		get_compare_commits_diff(&repo, (id, other), None)?
+	} else {
+		get_commit_diff(&repo, id, None)?
+	};
 
 	let mut res = Vec::new();
 
@@ -38,6 +45,44 @@ pub fn get_commit_files(
 	Ok(res)
 }
 
+#[allow(clippy::needless_pass_by_value)]
+pub fn get_compare_commits_diff(
+	repo: &Repository,
+	ids: (CommitId, CommitId),
+	pathspec: Option<String>,
+) -> Result<Diff<'_>> {
+	// scope_time!("get_compare_commits_diff");
+
+	let commits = (
+		repo.find_commit(ids.0.into())?,
+		repo.find_commit(ids.1.into())?,
+	);
+
+	let commits = if commits.0.time().cmp(&commits.1.time())
+		== Ordering::Greater
+	{
+		(commits.1, commits.0)
+	} else {
+		commits
+	};
+
+	let trees = (commits.0.tree()?, commits.1.tree()?);
+
+	let mut opts = DiffOptions::new();
+	if let Some(p) = &pathspec {
+		opts.pathspec(p.clone());
+	}
+	opts.show_binary(true);
+
+	let diff = repo.diff_tree_to_tree(
+		Some(&trees.0),
+		Some(&trees.1),
+		Some(&mut opts),
+	)?;
+
+	Ok(diff)
+}
+
 #[allow(clippy::redundant_pub_crate)]
 pub(crate) fn get_commit_diff(
 	repo: &Repository,
@@ -48,6 +93,7 @@ pub(crate) fn get_commit_diff(
 
 	let commit = repo.find_commit(id.into())?;
 	let commit_tree = commit.tree()?;
+
 	let parent = if commit.parent_count() > 0 {
 		repo.find_commit(commit.parent_id(0)?)
 			.ok()
@@ -116,7 +162,7 @@ mod tests {
 
 		let id = commit(repo_path, "commit msg")?;
 
-		let diff = get_commit_files(repo_path, id)?;
+		let diff = get_commit_files(repo_path, id, None)?;
 
 		assert_eq!(diff.len(), 1);
 		assert_eq!(diff[0].status, StatusItemType::New);
@@ -136,7 +182,7 @@ mod tests {
 
 		let id = stash_save(repo_path, None, true, false)?;
 
-		let diff = get_commit_files(repo_path, id)?;
+		let diff = get_commit_files(repo_path, id, None)?;
 
 		assert_eq!(diff.len(), 1);
 		assert_eq!(diff[0].status, StatusItemType::New);
@@ -164,7 +210,7 @@ mod tests {
 
 		let id = stash_save(repo_path, None, true, false)?;
 
-		let diff = get_commit_files(repo_path, id)?;
+		let diff = get_commit_files(repo_path, id, None)?;
 
 		assert_eq!(diff.len(), 2);
 		assert_eq!(diff[0].status, StatusItemType::Modified);

asyncgit/src/sync/commit_files.rs

@@ -1,7 +1,7 @@
 //! sync git api for fetching a diff
 
 use super::{
-	commit_files::get_commit_diff,
+	commit_files::{get_commit_diff, get_compare_commits_diff},
 	utils::{self, get_head_repo, work_dir},
 	CommitId,
 };
@@ -222,6 +222,22 @@ pub fn get_diff_commit(
 	raw_diff_to_file_diff(&diff, work_dir)
 }
 
+/// get file changes of a diff between two commits
+pub fn get_diff_commits(
+	repo_path: &str,
+	ids: (CommitId, CommitId),
+	p: String,
+) -> Result<FileDiff> {
+	scope_time!("get_diff_commits");
+
+	let repo = utils::repo(repo_path)?;
+	let work_dir = work_dir(&repo)?;
+	let diff =
+		get_compare_commits_diff(&repo, (ids.0, ids.1), Some(p))?;
+
+	raw_diff_to_file_diff(&diff, work_dir)
+}
+
 ///
 //TODO: refactor into helper type with the inline closures as dedicated functions
 #[allow(clippy::too_many_lines)]

asyncgit/src/sync/diff.rs

@@ -284,7 +284,8 @@ mod tests {
 		assert_eq!(
 			sync::get_commit_files(
 				tmp_repo_dir.path().to_str().unwrap(),
-				repo_1_commit
+				repo_1_commit,
+				None
 			)
 			.unwrap()[0]
 				.path,

asyncgit/src/sync/remotes/push.rs

@@ -231,7 +231,7 @@ mod tests {
 
 		let stash = get_stashes(repo_path)?[0];
 
-		let diff = get_commit_files(repo_path, stash)?;
+		let diff = get_commit_files(repo_path, stash, None)?;
 
 		assert_eq!(diff.len(), 1);