Skip to content

Commit aa2157a

Browse files
committed
Only load first parent on commit diff
When generating diffs, the first parent should be used to show the changes that were introduced in the change. Diffing against the other parents slows down diff loading and does not provide any useful information.
1 parent e6e5b90 commit aa2157a

File tree

2 files changed

+13
-22
lines changed

2 files changed

+13
-22
lines changed

src/git/commit_diff_loader.rs

+10-18
Original file line numberDiff line numberDiff line change
@@ -30,34 +30,26 @@ impl<'options> CommitDiffLoader<'options> {
3030
Self { config, repo }
3131
}
3232

33-
pub(crate) fn load_from_hash(&self, oid: Oid) -> Result<Vec<CommitDiff>, git2::Error> {
33+
pub(crate) fn load_from_hash(&self, oid: Oid) -> Result<CommitDiff, git2::Error> {
3434
let repo = self.repo.lock();
3535
let commit = repo.find_commit(oid)?;
36-
let no_parents = commit.parent_ids().count() == 0;
36+
// only the first parent matter for the diff, the second parent, if it exists, was only used
37+
// for conflict resolution
38+
let parent = commit.parents().next();
3739

38-
// some commits do not have parents, and can't have file stats
39-
let diffs = if no_parents {
40-
vec![self.load_diff(&repo, None, &commit)?]
41-
}
42-
else {
43-
//
44-
let mut diffs = vec![];
45-
for parent in commit.parents() {
46-
diffs.push(self.load_diff(&repo, Some(&parent), &commit)?);
47-
}
48-
diffs
49-
};
50-
Ok(diffs)
40+
self.load_diff(&repo, parent, &commit)
5141
}
5242

5343
#[expect(clippy::as_conversions, reason = "Mostly safe difference between APIs.")]
5444
#[expect(clippy::unwrap_in_result, reason = "Unwrap usage failure considered a bug.")]
5545
fn load_diff(
5646
&self,
5747
repo: &MutexGuard<'_, Repository>,
58-
parent: Option<&git2::Commit<'_>>,
48+
parent: Option<git2::Commit<'_>>,
5949
commit: &git2::Commit<'_>,
6050
) -> Result<CommitDiff, git2::Error> {
51+
let parent_commit = parent.as_ref().map(Commit::from);
52+
6153
let mut diff_options = DiffOptions::new();
6254
// include_unmodified added to find copies from unmodified files
6355
_ = diff_options
@@ -148,7 +140,7 @@ impl<'options> CommitDiffLoader<'options> {
148140

149141
Ok(CommitDiff::new(
150142
Commit::from(commit),
151-
parent.map(Commit::from),
143+
parent_commit,
152144
fsb.build(),
153145
number_files_changed,
154146
number_insertions,
@@ -316,7 +308,7 @@ mod tests {
316308
fn diff_from_head(repository: &crate::git::Repository, options: &CommitDiffLoaderOptions) -> CommitDiff {
317309
let id = repository.commit_id_from_ref("refs/heads/main").unwrap();
318310
let loader = CommitDiffLoader::new(repository.repository(), options);
319-
loader.load_from_hash(id).unwrap().remove(0)
311+
loader.load_from_hash(id).unwrap()
320312
}
321313

322314
#[test]

src/git/repository.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,10 @@ impl Repository {
6060
.id();
6161
let diff_loader_repository = Arc::clone(&self.repository);
6262
let loader = CommitDiffLoader::new(diff_loader_repository, config);
63-
// TODO this is ugly because it assumes one parent
64-
Ok(loader
63+
64+
loader
6565
.load_from_hash(oid)
66-
.map_err(|e| GitError::CommitLoad { cause: e })?
67-
.remove(0))
66+
.map_err(|e| GitError::CommitLoad { cause: e })
6867
}
6968
}
7069

0 commit comments

Comments
 (0)