From a3d92b4d1f129b18217d789273c4991964891de0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Fri, 10 Jan 2025 08:09:52 +0100 Subject: [PATCH 1/2] Rework how blame is passed to parents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, blame would have been passed to the first parent in cases where there was more than one. This commit changes that by only removing a particular suspect from further consideration once it has been compared to all of its parents. For hunks where blame cannot be passed to any parent, we can safely assume that they were introduced by a particular suspect, so we remove those hunks from `hunks_to_blame` and create a `BlameEntry` out of them. We can illustrate the change using the following small example history: ```text ---1----2 \ \ 3----4--- ``` Let’s now say that we’re blaming a file that has the following content: ```text line 1 # introduced by (1), then changed by (3) line 2 # introduced by (1) line 3 # introduced by (1), then changed by (2) ``` The resulting blame should look like this: ```text (3) line 1 (1) line 2 (2) line 3 ``` The previous version of the algorithm would have passed blame to just (2) or (3), depending on which one came first in the list of parents. --- gix-blame/src/file/function.rs | 57 +- gix-blame/src/file/mod.rs | 119 +-- gix-blame/src/file/tests.rs | 819 ++++++++++---------- gix-blame/tests/blame.rs | 1 + gix-blame/tests/fixtures/make_blame_repo.sh | 18 + 5 files changed, 497 insertions(+), 517 deletions(-) diff --git a/gix-blame/src/file/function.rs b/gix-blame/src/file/function.rs index 51bd2e7e1a0..357abe555fe 100644 --- a/gix-blame/src/file/function.rs +++ b/gix-blame/src/file/function.rs @@ -172,9 +172,10 @@ where if more_than_one_parent { // None of the changes affected the file we’re currently blaming. // Copy blame to parent. - for unblamed_hunk in &mut hunks_to_blame { - unblamed_hunk.clone_blame(suspect, parent_id); - } + hunks_to_blame = hunks_to_blame + .into_iter() + .map(|unblamed_hunk| unblamed_hunk.clone_blame(suspect, parent_id)) + .collect(); } else { pass_blame_from_to(suspect, parent_id, &mut hunks_to_blame); } @@ -196,14 +197,56 @@ where } gix_diff::tree::recorder::Change::Modification { previous_oid, oid, .. } => { let changes = blob_changes(&odb, resource_cache, oid, previous_oid, file_path, &mut stats)?; - hunks_to_blame = process_changes(&mut out, hunks_to_blame, changes, suspect); - pass_blame_from_to(suspect, parent_id, &mut hunks_to_blame); + hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent_id); } } } - if more_than_one_parent { - for unblamed_hunk in &mut hunks_to_blame { + + hunks_to_blame = hunks_to_blame + .into_iter() + .filter_map(|mut unblamed_hunk| { + if unblamed_hunk.suspects.len() == 1 { + if let Some(entry) = BlameEntry::from_unblamed_hunk(&unblamed_hunk, suspect) { + // At this point, we have copied blame for every hunk to a parent. Hunks + // that have only `suspect` left in `suspects` have not passed blame to any + // parent and so they can be converted to a `BlameEntry` and moved to + // `out`. + out.push(entry); + + return None; + } + } + unblamed_hunk.remove_blame(suspect); + + Some(unblamed_hunk) + }) + .collect(); + + // This block asserts that line ranges for each suspect never overlap. If they did overlap + // this would mean that the same line in a *Source File* would map to more than one line in + // the *Blamed File* and this is not possible. + #[cfg(debug_assertions)] + { + let ranges = hunks_to_blame.iter().fold( + std::collections::BTreeMap::>>::new(), + |mut acc, hunk| { + for (suspect, range) in hunk.suspects.clone() { + acc.entry(suspect).or_default().push(range); + } + + acc + }, + ); + + for (_, mut ranges) in ranges { + ranges.sort_by(|a, b| a.start.cmp(&b.start)); + + for window in ranges.windows(2) { + if let [a, b] = window { + assert!(a.end <= b.start, "#{hunks_to_blame:#?}"); + } + } } } } diff --git a/gix-blame/src/file/mod.rs b/gix-blame/src/file/mod.rs index 1afa77723e0..dd7b7763ca4 100644 --- a/gix-blame/src/file/mod.rs +++ b/gix-blame/src/file/mod.rs @@ -16,10 +16,10 @@ pub(super) mod function; /// /// This is the core of the blame implementation as it matches regions in *Source File* to the *Blamed File*. fn process_change( - out: &mut Vec, new_hunks_to_blame: &mut Vec, offset: &mut Offset, suspect: ObjectId, + parent: ObjectId, hunk: Option, change: Option, ) -> (Option, Option) { @@ -40,6 +40,8 @@ fn process_change( match (hunk, change) { (Some(hunk), Some(Change::Unchanged(unchanged))) => { let Some(range_in_suspect) = hunk.suspects.get(&suspect) else { + // We don’t clone blame to `parent` as `suspect` has nothing to do with this + // `hunk`. new_hunks_to_blame.push(hunk); return (None, Some(Change::Unchanged(unchanged))); }; @@ -64,7 +66,7 @@ fn process_change( // Nothing to do with `hunk` except shifting it, // but `unchanged` needs to be checked against the next hunk to catch up. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); (None, Some(Change::Unchanged(unchanged))) } (false, false) => { @@ -93,7 +95,7 @@ fn process_change( // Nothing to do with `hunk` except shifting it, // but `unchanged` needs to be checked against the next hunk to catch up. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); (None, Some(Change::Unchanged(unchanged))) } } @@ -123,7 +125,7 @@ fn process_change( } Either::Right((before, after)) => { // requeue the left side `before` after offsetting it… - new_hunks_to_blame.push(before.shift_by(suspect, *offset)); + new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); // …and treat `after` as `new_hunk`, which contains the `added` range. after } @@ -132,20 +134,18 @@ fn process_change( *offset += added.end - added.start; *offset -= number_of_lines_deleted; - // The overlapping `added` section was successfully located. - out.push(BlameEntry::with_offset( - added.clone(), - suspect, - hunk_starting_at_added.offset_for(suspect), - )); - + // The overlapping `added` section was successfully located. // Re-split at the end of `added` to continue with what's after. match hunk_starting_at_added.split_at(suspect, added.end) { - Either::Left(_) => { + Either::Left(hunk) => { + new_hunks_to_blame.push(hunk); + // Nothing to split, so we are done with this hunk. (None, None) } - Either::Right((_, after)) => { + Either::Right((hunk, after)) => { + new_hunks_to_blame.push(hunk); + // Keep processing the unblamed range after `added` (Some(after), None) } @@ -162,17 +162,13 @@ fn process_change( Either::Left(hunk) => hunk, Either::Right((before, after)) => { // Keep looking for the left side of the unblamed portion. - new_hunks_to_blame.push(before.shift_by(suspect, *offset)); + new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); after } }; // We can 'blame' the overlapping area of `added` and `hunk`. - out.push(BlameEntry::with_offset( - added.start..range_in_suspect.end, - suspect, - hunk_starting_at_added.offset_for(suspect), - )); + new_hunks_to_blame.push(hunk_starting_at_added); // Keep processing `added`, it's portion past `hunk` may still contribute. (None, Some(Change::AddedOrReplaced(added, number_of_lines_deleted))) } @@ -183,18 +179,20 @@ fn process_change( // <---> (blamed) // <--> (new hunk) - out.push(BlameEntry::with_offset( - range_in_suspect.start..added.end, - suspect, - hunk.offset_for(suspect), - )); - *offset += added.end - added.start; *offset -= number_of_lines_deleted; match hunk.split_at(suspect, added.end) { - Either::Left(_) => (None, None), - Either::Right((_, after)) => (Some(after), None), + Either::Left(hunk) => { + new_hunks_to_blame.push(hunk); + + (None, None) + } + Either::Right((before, after)) => { + new_hunks_to_blame.push(before); + + (Some(after), None) + } } } (false, false) => { @@ -222,7 +220,7 @@ fn process_change( // <----> (added) // Retry `hunk` once there is overlapping changes to process. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); // Let hunks catchup with this change. ( @@ -237,11 +235,7 @@ fn process_change( // <---> (blamed) // Successfully blame the whole range. - out.push(BlameEntry::with_offset( - range_in_suspect.clone(), - suspect, - hunk.offset_for(suspect), - )); + new_hunks_to_blame.push(hunk); // And keep processing `added` with future `hunks` that might be affected by it. ( @@ -279,7 +273,7 @@ fn process_change( } Either::Right((before, after)) => { // `before` isn't affected by deletion, so keep it for later. - new_hunks_to_blame.push(before.shift_by(suspect, *offset)); + new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); // after will be affected by offset, and we will see if there are more changes affecting it. after } @@ -291,7 +285,8 @@ fn process_change( // | (line_number_in_destination) // Catchup with changes. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); + ( None, Some(Change::Deleted(line_number_in_destination, number_of_lines_deleted)), @@ -300,7 +295,7 @@ fn process_change( } (Some(hunk), None) => { // nothing to do - changes are exhausted, re-evaluate `hunk`. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); (None, None) } (None, Some(Change::Unchanged(_))) => { @@ -328,10 +323,10 @@ fn process_change( /// Consume `hunks_to_blame` and `changes` to pair up matches ranges (also overlapping) with each other. /// Once a match is found, it's pushed onto `out`. fn process_changes( - out: &mut Vec, hunks_to_blame: Vec, changes: Vec, suspect: ObjectId, + parent: ObjectId, ) -> Vec { let mut hunks_iter = hunks_to_blame.into_iter(); let mut changes_iter = changes.into_iter(); @@ -344,10 +339,10 @@ fn process_changes( loop { (hunk, change) = process_change( - out, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, hunk, change, ); @@ -407,19 +402,6 @@ impl UnblamedHunk { } } - fn offset_for(&self, suspect: ObjectId) -> Offset { - let range_in_suspect = self - .suspects - .get(&suspect) - .expect("Internal and we know suspect is present"); - - if self.range_in_blamed_file.start > range_in_suspect.start { - Offset::Added(self.range_in_blamed_file.start - range_in_suspect.start) - } else { - Offset::Deleted(range_in_suspect.start - self.range_in_blamed_file.start) - } - } - /// Transfer all ranges from the commit at `from` to the commit at `to`. fn pass_blame(&mut self, from: ObjectId, to: ObjectId) { if let Some(range_in_suspect) = self.suspects.remove(&from) { @@ -427,10 +409,13 @@ impl UnblamedHunk { } } - fn clone_blame(&mut self, from: ObjectId, to: ObjectId) { + // TODO + // Should this also accept `&mut self` as the other functions do? + fn clone_blame(mut self, from: ObjectId, to: ObjectId) -> Self { if let Some(range_in_suspect) = self.suspects.get(&from) { self.suspects.insert(to, range_in_suspect.clone()); } + self } fn remove_blame(&mut self, suspect: ObjectId) { @@ -439,36 +424,6 @@ impl UnblamedHunk { } impl BlameEntry { - /// Create a new instance by creating `range_in_blamed_file` after applying `offset` to `range_in_source_file`. - fn with_offset(range_in_source_file: Range, commit_id: ObjectId, offset: Offset) -> Self { - debug_assert!( - range_in_source_file.end > range_in_source_file.start, - "{range_in_source_file:?}" - ); - - match offset { - Offset::Added(added) => Self { - start_in_blamed_file: range_in_source_file.start + added, - start_in_source_file: range_in_source_file.start, - len: force_non_zero(range_in_source_file.len() as u32), - commit_id, - }, - Offset::Deleted(deleted) => { - debug_assert!( - range_in_source_file.start >= deleted, - "{range_in_source_file:?} {offset:?}" - ); - - Self { - start_in_blamed_file: range_in_source_file.start - deleted, - start_in_source_file: range_in_source_file.start, - len: force_non_zero(range_in_source_file.len() as u32), - commit_id, - } - } - } - } - /// Create an offset from a portion of the *Blamed File*. fn from_unblamed_hunk(unblamed_hunk: &UnblamedHunk, commit_id: ObjectId) -> Option { let range_in_source_file = unblamed_hunk.suspects.get(&commit_id)?; diff --git a/gix-blame/src/file/tests.rs b/gix-blame/src/file/tests.rs index c6ed47b29c3..165c0e5c768 100644 --- a/gix-blame/src/file/tests.rs +++ b/gix-blame/src/file/tests.rs @@ -16,23 +16,24 @@ fn new_unblamed_hunk(range_in_blamed_file: Range, suspect: ObjectId, offset } mod process_change { + use std::str::FromStr; + use super::*; - use crate::file::{force_non_zero, process_change, Change, Offset, UnblamedHunk}; - use crate::BlameEntry; + use crate::file::{process_change, Change, Offset, UnblamedHunk}; use gix_hash::ObjectId; #[test] fn nothing() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, None, None, ); @@ -44,16 +45,16 @@ mod process_change { #[test] fn added_hunk() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::AddedOrReplaced(0..3, 0)), ); @@ -67,30 +68,27 @@ mod process_change { ); assert_eq!(change, None); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(3), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 0..3, + suspects: [(suspect, 0..3)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn added_hunk_2() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::AddedOrReplaced(2..3, 0)), ); @@ -103,37 +101,34 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 2, - start_in_source_file: 2, - len: force_non_zero(1), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 0..2, - suspects: [(suspect, 0..2)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 0..2, + suspects: [(suspect, 0..2), (parent, 0..2)].into() + }, + UnblamedHunk { + range_in_blamed_file: 2..3, + suspects: [(suspect, 2..3)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn added_hunk_3() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(5); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(10..15, suspect, Offset::Added(0))), Some(Change::AddedOrReplaced(12..13, 0)), ); @@ -146,37 +141,34 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 12, - start_in_source_file: 12, - len: force_non_zero(1), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 10..12, - suspects: [(suspect, 5..7)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 10..12, + suspects: [(suspect, 10..12), (parent, 5..7)].into() + }, + UnblamedHunk { + range_in_blamed_file: 12..13, + suspects: [(suspect, 12..13)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(6)); } #[test] fn added_hunk_4() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 7..12 Some(new_unblamed_hunk(12..17, suspect, Offset::Added(5))), Some(Change::AddedOrReplaced(9..10, 0)), @@ -190,37 +182,34 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 14, - start_in_source_file: 9, - len: force_non_zero(1), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 12..14, - suspects: [(suspect, 7..9)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 12..14, + suspects: [(suspect, 7..9), (parent, 7..9)].into() + }, + UnblamedHunk { + range_in_blamed_file: 14..15, + suspects: [(suspect, 9..10)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn added_hunk_5() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::AddedOrReplaced(0..3, 1)), ); @@ -234,30 +223,27 @@ mod process_change { ); assert_eq!(change, None); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(3), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 0..3, + suspects: [(suspect, 0..3)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(2)); } #[test] fn added_hunk_6() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 0..4 Some(new_unblamed_hunk(1..5, suspect, Offset::Added(1))), Some(Change::AddedOrReplaced(0..3, 1)), @@ -272,30 +258,27 @@ mod process_change { ); assert_eq!(change, None); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 1, - start_in_source_file: 0, - len: force_non_zero(3), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 1..4, + suspects: [(suspect, 0..3)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(2)); } #[test] fn added_hunk_7() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(2); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 2..6 Some(new_unblamed_hunk(3..7, suspect, Offset::Added(1))), Some(Change::AddedOrReplaced(3..5, 1)), @@ -309,37 +292,34 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 4, - start_in_source_file: 3, - len: force_non_zero(2), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 3..4, - suspects: [(suspect, 0..1)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 3..4, + suspects: [(suspect, 2..3), (parent, 0..1)].into() + }, + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 3..5)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn added_hunk_8() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 25..26 Some(new_unblamed_hunk(23..24, suspect, Offset::Deleted(2))), Some(Change::AddedOrReplaced(25..27, 1)), @@ -348,30 +328,27 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(25..27, 1))); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 23, - start_in_source_file: 25, - len: force_non_zero(1), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 23..24, + suspects: [(suspect, 25..26)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn added_hunk_9() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 21..22 Some(new_unblamed_hunk(23..24, suspect, Offset::Added(2))), Some(Change::AddedOrReplaced(18..22, 3)), @@ -380,30 +357,27 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, None); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 23, - start_in_source_file: 21, - len: force_non_zero(1), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 23..24, + suspects: [(suspect, 21..22)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn added_hunk_10() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 70..108 Some(new_unblamed_hunk(71..109, suspect, Offset::Added(1))), Some(Change::AddedOrReplaced(106..109, 0)), @@ -411,37 +385,34 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(106..109, 0))); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 107, - start_in_source_file: 106, - len: force_non_zero(2), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 71..107, - suspects: [(suspect, 70..106)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 71..107, + suspects: [(suspect, 70..106), (parent, 70..106)].into() + }, + UnblamedHunk { + range_in_blamed_file: 107..109, + suspects: [(suspect, 106..108)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(0)); } #[test] fn added_hunk_11() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 137..144 Some(new_unblamed_hunk(149..156, suspect, Offset::Added(12))), Some(Change::AddedOrReplaced(143..146, 0)), @@ -449,37 +420,34 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(143..146, 0))); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 155, - start_in_source_file: 143, - len: force_non_zero(1), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 149..155, - suspects: [(suspect, 137..143)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 149..155, + suspects: [(suspect, 137..143), (parent, 137..143)].into() + }, + UnblamedHunk { + range_in_blamed_file: 155..156, + suspects: [(suspect, 143..144)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(0)); } #[test] fn no_overlap() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Deleted(3); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 2..5 Some(new_unblamed_hunk(3..6, suspect, Offset::Added(1))), Some(Change::AddedOrReplaced(7..10, 1)), @@ -487,12 +455,11 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(7..10, 1))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 3..6, - suspects: [(suspect, 5..8)].into() + suspects: [(suspect, 2..5), (parent, 5..8)].into() }] ); assert_eq!(offset_in_destination, Offset::Deleted(3)); @@ -500,16 +467,16 @@ mod process_change { #[test] fn no_overlap_2() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 6..8 Some(new_unblamed_hunk(9..11, suspect, Offset::Added(3))), Some(Change::AddedOrReplaced(2..5, 0)), @@ -523,23 +490,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn no_overlap_3() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 5..15 Some(new_unblamed_hunk(4..15, suspect, Offset::Deleted(1))), Some(Change::AddedOrReplaced(4..5, 1)), @@ -553,23 +519,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(0)); } #[test] fn no_overlap_4() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 25..27 Some(new_unblamed_hunk(23..25, suspect, Offset::Deleted(2))), Some(Change::Unchanged(21..22)), @@ -583,23 +548,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn no_overlap_5() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 17..18 Some(new_unblamed_hunk(15..16, suspect, Offset::Deleted(2))), Some(Change::Deleted(20, 1)), @@ -607,12 +571,11 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::Deleted(20, 1))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 15..16, - suspects: [(suspect, 16..17)].into() + suspects: [(suspect, 17..18), (parent, 16..17)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(1)); @@ -620,16 +583,16 @@ mod process_change { #[test] fn no_overlap_6() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 22..24 Some(new_unblamed_hunk(23..25, suspect, Offset::Added(1))), Some(Change::Deleted(20, 1)), @@ -643,23 +606,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Deleted(1)); } #[test] fn enclosing_addition() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(3); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 5..8 Some(new_unblamed_hunk(2..5, suspect, Offset::Deleted(3))), Some(Change::AddedOrReplaced(3..12, 2)), @@ -668,30 +630,27 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(3..12, 2))); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 2, - start_in_source_file: 5, - len: force_non_zero(3), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 2..5, + suspects: [(suspect, 5..8)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn enclosing_deletion() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(3); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 13..20 Some(new_unblamed_hunk(12..19, suspect, Offset::Deleted(1))), Some(Change::Deleted(15, 2)), @@ -705,12 +664,11 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 12..14, - suspects: [(suspect, 10..12)].into() + suspects: [(suspect, 13..15), (parent, 10..12)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(1)); @@ -718,16 +676,16 @@ mod process_change { #[test] fn enclosing_unchanged_lines() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(3); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 109..113 Some(new_unblamed_hunk(110..114, suspect, Offset::Added(1))), Some(Change::Unchanged(109..172)), @@ -735,12 +693,11 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::Unchanged(109..172))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 110..114, - suspects: [(suspect, 106..110)].into() + suspects: [(suspect, 109..113), (parent, 106..110)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(3)); @@ -748,16 +705,16 @@ mod process_change { #[test] fn unchanged_hunk() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::Unchanged(0..3)), ); @@ -770,35 +727,33 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(0)); } #[test] fn unchanged_hunk_2() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::Unchanged(0..7)), ); assert_eq!(hunk, None); assert_eq!(change, Some(Change::Unchanged(0..7))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 0..5, - suspects: [(suspect, 0..5)].into() + suspects: [(suspect, 0..5), (parent, 0..5)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(0)); @@ -806,16 +761,16 @@ mod process_change { #[test] fn unchanged_hunk_3() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Deleted(2); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(UnblamedHunk { range_in_blamed_file: 22..30, suspects: [(suspect, 21..29)].into(), @@ -831,35 +786,33 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Deleted(2)); } #[test] fn deleted_hunk() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::Deleted(5, 3)), ); assert_eq!(hunk, None); assert_eq!(change, Some(Change::Deleted(5, 3))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 0..5, - suspects: [(suspect, 0..5)].into() + suspects: [(suspect, 0..5), (parent, 0..5)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(0)); @@ -867,16 +820,16 @@ mod process_change { #[test] fn deleted_hunk_2() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(2..16, suspect, Offset::Added(0))), Some(Change::Deleted(0, 4)), ); @@ -889,23 +842,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Deleted(4)); } #[test] fn deleted_hunk_3() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(2..16, suspect, Offset::Added(0))), Some(Change::Deleted(14, 4)), ); @@ -918,368 +870,345 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, - [new_unblamed_hunk(2..14, suspect, Offset::Added(0))] + [UnblamedHunk { + range_in_blamed_file: 2..14, + suspects: [(suspect, 2..14), (parent, 2..14)].into() + }] ); assert_eq!(offset_in_destination, Offset::Deleted(4)); } #[test] fn addition_only() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, None, Some(Change::AddedOrReplaced(22..25, 1)), ); assert_eq!(hunk, None); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn deletion_only() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, None, Some(Change::Deleted(11, 5)), ); assert_eq!(hunk, None); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Deleted(4)); } #[test] fn unchanged_only() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, None, Some(Change::Unchanged(11..13)), ); assert_eq!(hunk, None); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(1)); } } + mod process_changes { + use std::str::FromStr; + use crate::file::tests::new_unblamed_hunk; - use crate::file::{force_non_zero, process_changes, Change, Offset, UnblamedHunk}; - use crate::BlameEntry; + use crate::file::{process_changes, Change, Offset, UnblamedHunk}; use gix_hash::ObjectId; #[test] fn nothing() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); - let new_hunks_to_blame = process_changes(&mut lines_blamed, vec![], vec![], suspect); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); + let new_hunks_to_blame = process_changes(vec![], vec![], suspect, parent); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); } #[test] fn added_hunk() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..4, suspect, Offset::Added(0))]; let changes = vec![Change::AddedOrReplaced(0..4, 0)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(4), - commit_id: suspect - }] + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 0..4, + suspects: [(suspect, 0..4)].into(), + },] ); - assert_eq!(new_hunks_to_blame, []); } #[test] fn added_hunk_2() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![Change::AddedOrReplaced(0..4, 0), Change::Unchanged(4..6)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(4), - commit_id: suspect - }] + new_hunks_to_blame, + [ + UnblamedHunk { + range_in_blamed_file: 0..4, + suspects: [(suspect, 0..4)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 4..6), (parent, 0..2)].into(), + }, + ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(4..6, suspect, Offset::Added(4))]); } #[test] fn added_hunk_3() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![ Change::Unchanged(0..2), Change::AddedOrReplaced(2..4, 0), Change::Unchanged(4..6), ]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 2, - start_in_source_file: 2, - len: force_non_zero(2), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, [ - new_unblamed_hunk(0..2, suspect, Offset::Added(0)), - new_unblamed_hunk(4..6, suspect, Offset::Added(2)) + UnblamedHunk { + range_in_blamed_file: 0..2, + suspects: [(suspect, 0..2), (parent, 0..2)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 2..4, + suspects: [(suspect, 2..4)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 4..6), (parent, 2..4)].into(), + }, ] ); } #[test] fn added_hunk_4_0() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![ Change::AddedOrReplaced(0..1, 0), Change::AddedOrReplaced(1..4, 0), Change::Unchanged(4..6), ]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 0..1, + suspects: [(suspect, 0..1)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 1..4, + suspects: [(suspect, 1..4)].into(), }, - BlameEntry { - start_in_blamed_file: 1, - start_in_source_file: 1, - len: force_non_zero(3), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 4..6), (parent, 0..2)].into(), } ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(4..6, suspect, Offset::Added(4))]); } #[test] fn added_hunk_4_1() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![Change::AddedOrReplaced(0..1, 0)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect - }] + new_hunks_to_blame, + [ + UnblamedHunk { + range_in_blamed_file: 0..1, + suspects: [(suspect, 0..1)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 1..6, + suspects: [(suspect, 1..6), (parent, 0..5)].into(), + } + ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(1..6, suspect, Offset::Added(1))]); } #[test] fn added_hunk_4_2() { - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let suspect_2 = ObjectId::from_hex(b"2222222222222222222222222222222222222222").unwrap(); - let mut lines_blamed: Vec = vec![BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(2), - commit_id: suspect, - }]; let hunks_to_blame = vec![new_unblamed_hunk(2..6, suspect_2, Offset::Added(2))]; let changes = vec![Change::AddedOrReplaced(0..1, 0)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect_2); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect_2, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(2), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 2..3, + suspects: [(suspect_2, 0..1)].into(), }, - BlameEntry { - start_in_blamed_file: 2, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect_2 + UnblamedHunk { + range_in_blamed_file: 3..6, + suspects: [(suspect_2, 1..4), (parent, 0..3)].into(), } ] ); - assert_eq!( - new_hunks_to_blame, - [new_unblamed_hunk(3..6, suspect_2, Offset::Added(3))] - ); } #[test] fn added_hunk_5() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![Change::AddedOrReplaced(0..4, 3), Change::Unchanged(4..6)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(4), - commit_id: suspect - }] + new_hunks_to_blame, + [ + UnblamedHunk { + range_in_blamed_file: 0..4, + suspects: [(suspect, 0..4)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 4..6), (parent, 3..5)].into(), + } + ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(4..6, suspect, Offset::Added(1))]); } #[test] fn added_hunk_6() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(4..6, suspect, Offset::Added(1))]; let changes = vec![Change::AddedOrReplaced(0..3, 0), Change::Unchanged(3..5)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); - assert_eq!(lines_blamed, []); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(4..6, suspect, Offset::Added(4))]); + assert_eq!( + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 3..5), (parent, 0..2)].into(), + }] + ); } #[test] fn added_hunk_7() { - let suspect = ObjectId::null(gix_hash::Kind::Sha1); - let suspect_2 = ObjectId::from_hex(b"2222222222222222222222222222222222222222").unwrap(); - let mut lines_blamed: Vec = vec![BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect, - }]; - let hunks_to_blame = vec![new_unblamed_hunk(1..3, suspect_2, Offset::Added(1))]; + let suspect = ObjectId::from_hex(b"0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); + let hunks_to_blame = vec![new_unblamed_hunk(1..3, suspect, Offset::Added(1))]; let changes = vec![Change::AddedOrReplaced(0..1, 2)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect_2); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 1..2, + suspects: [(suspect, 0..1)].into(), }, - BlameEntry { - start_in_blamed_file: 1, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect_2 + UnblamedHunk { + range_in_blamed_file: 2..3, + suspects: [(suspect, 1..2), (parent, 2..3)].into(), } ] ); - assert_eq!( - new_hunks_to_blame, - [new_unblamed_hunk(2..3, suspect_2, Offset::Added(0))] - ); } #[test] fn added_hunk_8() { - let suspect = ObjectId::null(gix_hash::Kind::Sha1); - let mut lines_blamed = Vec::new(); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..4, suspect, Offset::Added(0))]; let changes = vec![ Change::AddedOrReplaced(0..2, 0), Change::Unchanged(2..3), Change::AddedOrReplaced(3..4, 0), ]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(2), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 0..2, + suspects: [(suspect, 0..2)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 2..3, + suspects: [(suspect, 2..3), (parent, 0..1)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 3..4, + suspects: [(suspect, 3..4)].into(), }, - BlameEntry { - start_in_blamed_file: 3, - start_in_source_file: 3, - len: force_non_zero(1), - commit_id: suspect - } ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(2..3, suspect, Offset::Added(2))]); } #[test] fn added_hunk_9() { - let suspect = ObjectId::null(gix_hash::Kind::Sha1); - let mut lines_blamed: Vec = vec![BlameEntry { - start_in_blamed_file: 30, - start_in_source_file: 30, - len: force_non_zero(1), - commit_id: suspect, - }]; + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![ UnblamedHunk { range_in_blamed_file: 0..30, @@ -1295,72 +1224,106 @@ mod process_changes { Change::AddedOrReplaced(16..17, 0), Change::Unchanged(17..37), ]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); - - lines_blamed.sort_by(|a, b| a.start_in_blamed_file.cmp(&b.start_in_blamed_file)); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 16, - start_in_source_file: 16, - len: force_non_zero(1), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 0..16, + suspects: [(suspect, 0..16), (parent, 0..16)].into() + }, + UnblamedHunk { + range_in_blamed_file: 16..17, + suspects: [(suspect, 16..17)].into() + }, + UnblamedHunk { + range_in_blamed_file: 17..30, + suspects: [(suspect, 17..30), (parent, 16..29)].into() }, - BlameEntry { - start_in_blamed_file: 30, - start_in_source_file: 30, - len: force_non_zero(1), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 31..37, + suspects: [(suspect, 31..37), (parent, 30..36)].into() } ] ); + } + + #[test] + fn added_hunk_10() { + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); + let hunks_to_blame = vec![ + UnblamedHunk { + range_in_blamed_file: 1..3, + suspects: [(suspect, 1..3)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 5..7, + suspects: [(suspect, 5..7)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 8..10, + suspects: [(suspect, 8..10)].into(), + }, + ]; + let changes = vec![ + Change::Unchanged(0..6), + Change::AddedOrReplaced(6..9, 0), + Change::Unchanged(9..11), + ]; + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); + assert_eq!( new_hunks_to_blame, [ UnblamedHunk { - range_in_blamed_file: 0..16, - suspects: [(suspect, 0..16)].into() + range_in_blamed_file: 1..3, + suspects: [(suspect, 1..3), (parent, 1..3)].into(), }, UnblamedHunk { - range_in_blamed_file: 17..30, - suspects: [(suspect, 16..29)].into() + range_in_blamed_file: 5..6, + suspects: [(suspect, 5..6), (parent, 5..6)].into(), }, UnblamedHunk { - range_in_blamed_file: 31..37, - suspects: [(suspect, 30..36)].into() - } + range_in_blamed_file: 6..7, + suspects: [(suspect, 6..7)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 8..9, + suspects: [(suspect, 8..9)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 9..10, + suspects: [(suspect, 9..10), (parent, 6..7)].into(), + }, ] ); } #[test] fn deleted_hunk() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![ new_unblamed_hunk(0..4, suspect, Offset::Added(0)), new_unblamed_hunk(4..7, suspect, Offset::Added(0)), ]; let changes = vec![Change::Deleted(0, 3), Change::AddedOrReplaced(0..4, 0)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(4), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 4..7, - suspects: [(suspect, 3..6)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 0..4, + suspects: [(suspect, 0..4)].into() + }, + UnblamedHunk { + range_in_blamed_file: 4..7, + suspects: [(suspect, 4..7), (parent, 3..6)].into() + } + ] ); } } diff --git a/gix-blame/tests/blame.rs b/gix-blame/tests/blame.rs index 258a3457c4a..48926531c1a 100644 --- a/gix-blame/tests/blame.rs +++ b/gix-blame/tests/blame.rs @@ -234,6 +234,7 @@ mktest!( 1 ); mktest!(file_only_changed_in_branch, "file-only-changed-in-branch", 2); +mktest!(file_changed_in_two_branches, "file-changed-in-two-branches", 3); /// As of 2024-09-24, these tests are expected to fail. /// diff --git a/gix-blame/tests/fixtures/make_blame_repo.sh b/gix-blame/tests/fixtures/make_blame_repo.sh index 31e30c42e4d..9a62d45fc1a 100755 --- a/gix-blame/tests/fixtures/make_blame_repo.sh +++ b/gix-blame/tests/fixtures/make_blame_repo.sh @@ -181,6 +181,23 @@ git commit -q -m c13 git checkout main git merge branch-that-has-one-commit || true +echo -e "line 1\nline 2\n line 3" > file-changed-in-two-branches.txt +git add file-changed-in-two-branches.txt +git commit -q -m c14 + +git checkout -b branch-that-has-one-of-the-changes + +echo -e "line 1\nline 2\n line 3 changed" > file-changed-in-two-branches.txt +git add file-changed-in-two-branches.txt +git commit -q -m c14.1 + +git checkout main +echo -e "line 1 changed\nline 2\n line 3" > file-changed-in-two-branches.txt +git add file-changed-in-two-branches.txt +git commit -q -m c14.2 + +git merge branch-that-has-one-of-the-changes || true + git blame --porcelain simple.txt > .git/simple.baseline git blame --porcelain multiline-hunks.txt > .git/multiline-hunks.baseline git blame --porcelain deleted-lines.txt > .git/deleted-lines.baseline @@ -198,6 +215,7 @@ git blame --porcelain resolved-conflict.txt > .git/resolved-conflict.baseline git blame --porcelain file-in-one-chain-of-ancestors.txt > .git/file-in-one-chain-of-ancestors.baseline git blame --porcelain different-file-in-another-chain-of-ancestors.txt > .git/different-file-in-another-chain-of-ancestors.baseline git blame --porcelain file-only-changed-in-branch.txt > .git/file-only-changed-in-branch.baseline +git blame --porcelain file-changed-in-two-branches.txt > .git/file-changed-in-two-branches.baseline git blame --porcelain empty-lines-histogram.txt > .git/empty-lines-histogram.baseline From 360bf383a3ebdeeda1db161d42bb057a05cdf32b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 10 Jan 2025 08:11:18 +0100 Subject: [PATCH 2/2] refactor - try to not force `clone_from()` into move semantics unless necessary --- gix-blame/src/file/function.rs | 39 ++++++++++++++-------------------- gix-blame/src/file/mod.rs | 28 +++++++++++++----------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/gix-blame/src/file/function.rs b/gix-blame/src/file/function.rs index 357abe555fe..8c01a033edc 100644 --- a/gix-blame/src/file/function.rs +++ b/gix-blame/src/file/function.rs @@ -172,10 +172,9 @@ where if more_than_one_parent { // None of the changes affected the file we’re currently blaming. // Copy blame to parent. - hunks_to_blame = hunks_to_blame - .into_iter() - .map(|unblamed_hunk| unblamed_hunk.clone_blame(suspect, parent_id)) - .collect(); + for unblamed_hunk in &mut hunks_to_blame { + unblamed_hunk.clone_blame(suspect, parent_id); + } } else { pass_blame_from_to(suspect, parent_id, &mut hunks_to_blame); } @@ -202,26 +201,20 @@ where } } - hunks_to_blame = hunks_to_blame - .into_iter() - .filter_map(|mut unblamed_hunk| { - if unblamed_hunk.suspects.len() == 1 { - if let Some(entry) = BlameEntry::from_unblamed_hunk(&unblamed_hunk, suspect) { - // At this point, we have copied blame for every hunk to a parent. Hunks - // that have only `suspect` left in `suspects` have not passed blame to any - // parent and so they can be converted to a `BlameEntry` and moved to - // `out`. - out.push(entry); - - return None; - } + hunks_to_blame.retain_mut(|unblamed_hunk| { + if unblamed_hunk.suspects.len() == 1 { + if let Some(entry) = BlameEntry::from_unblamed_hunk(&unblamed_hunk, suspect) { + // At this point, we have copied blame for every hunk to a parent. Hunks + // that have only `suspect` left in `suspects` have not passed blame to any + // parent, and so they can be converted to a `BlameEntry` and moved to + // `out`. + out.push(entry); + return false; } - - unblamed_hunk.remove_blame(suspect); - - Some(unblamed_hunk) - }) - .collect(); + } + unblamed_hunk.remove_blame(suspect); + true + }); // This block asserts that line ranges for each suspect never overlap. If they did overlap // this would mean that the same line in a *Source File* would map to more than one line in diff --git a/gix-blame/src/file/mod.rs b/gix-blame/src/file/mod.rs index dd7b7763ca4..086923bac45 100644 --- a/gix-blame/src/file/mod.rs +++ b/gix-blame/src/file/mod.rs @@ -66,7 +66,7 @@ fn process_change( // Nothing to do with `hunk` except shifting it, // but `unchanged` needs to be checked against the next hunk to catch up. - new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); + new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset)); (None, Some(Change::Unchanged(unchanged))) } (false, false) => { @@ -95,7 +95,7 @@ fn process_change( // Nothing to do with `hunk` except shifting it, // but `unchanged` needs to be checked against the next hunk to catch up. - new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); + new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset)); (None, Some(Change::Unchanged(unchanged))) } } @@ -125,7 +125,7 @@ fn process_change( } Either::Right((before, after)) => { // requeue the left side `before` after offsetting it… - new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); + new_hunks_to_blame.push(before.cloned_blame(suspect, parent).shift_by(parent, *offset)); // …and treat `after` as `new_hunk`, which contains the `added` range. after } @@ -162,7 +162,7 @@ fn process_change( Either::Left(hunk) => hunk, Either::Right((before, after)) => { // Keep looking for the left side of the unblamed portion. - new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); + new_hunks_to_blame.push(before.cloned_blame(suspect, parent).shift_by(parent, *offset)); after } }; @@ -220,7 +220,7 @@ fn process_change( // <----> (added) // Retry `hunk` once there is overlapping changes to process. - new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); + new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset)); // Let hunks catchup with this change. ( @@ -273,7 +273,7 @@ fn process_change( } Either::Right((before, after)) => { // `before` isn't affected by deletion, so keep it for later. - new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); + new_hunks_to_blame.push(before.cloned_blame(suspect, parent).shift_by(parent, *offset)); // after will be affected by offset, and we will see if there are more changes affecting it. after } @@ -285,7 +285,7 @@ fn process_change( // | (line_number_in_destination) // Catchup with changes. - new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); + new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset)); ( None, @@ -295,7 +295,7 @@ fn process_change( } (Some(hunk), None) => { // nothing to do - changes are exhausted, re-evaluate `hunk`. - new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); + new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset)); (None, None) } (None, Some(Change::Unchanged(_))) => { @@ -409,13 +409,17 @@ impl UnblamedHunk { } } - // TODO - // Should this also accept `&mut self` as the other functions do? - fn clone_blame(mut self, from: ObjectId, to: ObjectId) -> Self { + /// This is like [`Self::clone_blame()`], but easier to use in places + /// where the cloning is done 'inline'. + fn cloned_blame(mut self, from: ObjectId, to: ObjectId) -> Self { + self.clone_blame(from, to); + self + } + + fn clone_blame(&mut self, from: ObjectId, to: ObjectId) { if let Some(range_in_suspect) = self.suspects.get(&from) { self.suspects.insert(to, range_in_suspect.clone()); } - self } fn remove_blame(&mut self, suspect: ObjectId) {