refactor

* make test baseline more idiomatic
* make some bits of the blame implementation more idiomatic

gix-blame/src/lib.rs

@@ -286,7 +286,7 @@ impl Sink for ChangeRecorder {
 }
 
 pub fn process_change(
-    lines_blamed: &mut Vec<BlameEntry>,
+    out: &mut Vec<BlameEntry>,
     new_hunks_to_blame: &mut Vec<UnblamedHunk>,
     offset_in_destination: &mut Offset,
     suspect: ObjectId,
@@ -297,7 +297,7 @@ pub fn process_change(
         (Some(hunk), Some(Change::Unchanged(unchanged))) => {
             match (
                 // Since `unchanged` is a range that is not inclusive at the end,
-                // `unchanged.end` is not part of `unchanged`. The first line that is is
+                // `unchanged.end` is not part of `unchanged`. The first line that is
                 // `unchanged.end - 1`.
                 hunk.range_in_destination.contains(&unchanged.start),
                 (unchanged.end - 1) >= hunk.range_in_destination.start
@@ -370,7 +370,7 @@ pub fn process_change(
                     *offset_in_destination += added.end - added.start;
                     *offset_in_destination -= number_of_lines_deleted;
 
-                    lines_blamed.push(BlameEntry::with_offset(added.clone(), suspect, hunk.offset()));
+                    out.push(BlameEntry::with_offset(added.clone(), suspect, hunk.offset()));
 
                     let new_hunk = if hunk.range_in_destination.end > added.end {
                         Some(UnblamedHunk::from_destination(
@@ -401,7 +401,7 @@ pub fn process_change(
                         ));
                     }
 
-                    lines_blamed.push(BlameEntry::with_offset(
+                    out.push(BlameEntry::with_offset(
                         added.start..hunk.range_in_destination.end,
                         suspect,
                         hunk.offset(),
@@ -419,7 +419,7 @@ pub fn process_change(
                     //    <--->      (blamed)
                     //         <-->  (new hunk)
 
-                    lines_blamed.push(BlameEntry::with_offset(
+                    out.push(BlameEntry::with_offset(
                         hunk.range_in_destination.start..added.end,
                         suspect,
                         hunk.offset(),
@@ -476,7 +476,7 @@ pub fn process_change(
                         // <---------->  (added)
                         //    <--->      (blamed)
 
-                        lines_blamed.push(BlameEntry::with_offset(
+                        out.push(BlameEntry::with_offset(
                             hunk.range_in_destination.clone(),
                             suspect,
                             hunk.offset(),
@@ -562,37 +562,32 @@ pub fn process_change(
 }
 
 pub fn process_changes(
-    lines_blamed: &mut Vec<BlameEntry>,
-    hunks_to_blame: Vec<UnblamedHunk>,
-    changes: Vec<Change>,
+    out: &mut Vec<BlameEntry>,
+    hunks_to_blame: &[UnblamedHunk],
+    changes: &[Change],
     suspect: ObjectId,
 ) -> Vec<UnblamedHunk> {
-    let mut hunks_iter = hunks_to_blame.iter();
-    let mut changes_iter = changes.iter();
+    let mut hunks_iter = hunks_to_blame.iter().cloned();
+    let mut changes_iter = changes.iter().cloned();
 
-    let mut hunk: Option<UnblamedHunk> = hunks_iter.next().cloned();
-    let mut change: Option<Change> = changes_iter.next().cloned();
+    let mut hunk: Option<UnblamedHunk> = hunks_iter.next();
+    let mut change: Option<Change> = changes_iter.next();
 
     let mut new_hunks_to_blame: Vec<UnblamedHunk> = vec![];
     let mut offset_in_destination: Offset = Offset::Added(0);
 
     loop {
         (hunk, change) = process_change(
-            lines_blamed,
+            out,
             &mut new_hunks_to_blame,
             &mut offset_in_destination,
             suspect,
             hunk,
             change,
         );
 
-        if hunk.is_none() {
-            hunk = hunks_iter.next().cloned();
-        }
-
-        if change.is_none() {
-            change = changes_iter.next().cloned();
-        }
+        hunk = hunk.or_else(|| hunks_iter.next());
+        change = change.or_else(|| changes_iter.next());
 
         if hunk.is_none() && change.is_none() {
             break;
@@ -603,7 +598,7 @@ pub fn process_changes(
 }
 
 /// This function merges adjacent blame entries. It merges entries that are adjacent both in the
-/// blamed file as well as in the original file that introduced them. This follows `git`’s
+/// blamed file and in the original file that introduced them. This follows `git`’s
 /// behaviour. `libgit2`, as of 2024-09-19, only checks whether two entries are adjacent in the
 /// blamed file which can result in different blames in certain edge cases. See [the commit][1]
 /// that introduced the extra check into `git` for context. See [this commit][2] for a way to test
@@ -723,7 +718,7 @@ pub fn blame_file(worktree_path: PathBuf, file_path: &BStr) -> Vec<BlameEntry> {
         0..number_of_lines.try_into().unwrap(),
         Offset::Added(0),
     )];
-    let mut lines_blamed: Vec<BlameEntry> = vec![];
+    let mut out: Vec<BlameEntry> = vec![];
 
     loop {
         let item = traverse.next().unwrap().unwrap();
@@ -738,7 +733,7 @@ pub fn blame_file(worktree_path: PathBuf, file_path: &BStr) -> Vec<BlameEntry> {
             // remaining lines to it, even though we don’t explicitly check whether that is true
             // here. We could perhaps use `needed_to_obtain` to compare `suspect` against an empty
             // tree to validate this assumption.
-            lines_blamed.extend(lines_to_blame.iter().map(|hunk| {
+            out.extend(lines_to_blame.iter().map(|hunk| {
                 BlameEntry::new(
                     hunk.range_in_blamed_file.clone(),
                     // TODO
@@ -800,7 +795,7 @@ pub fn blame_file(worktree_path: PathBuf, file_path: &BStr) -> Vec<BlameEntry> {
             if matches!(modification, gix_diff::tree::recorder::Change::Addition { .. }) {
                 // Every line that has not been blamed yet on a commit, is expected to have been
                 // added when the file was added to the repository.
-                lines_blamed.extend(lines_to_blame.iter().map(|hunk| {
+                out.extend(lines_to_blame.iter().map(|hunk| {
                     BlameEntry::new(
                         hunk.range_in_blamed_file.clone(),
                         // TODO
@@ -840,21 +835,17 @@ pub fn blame_file(worktree_path: PathBuf, file_path: &BStr) -> Vec<BlameEntry> {
 
         let outcome = resource_cache.prepare_diff().unwrap();
         let input = outcome.interned_input();
-
         let number_of_lines_in_destination = input.after.len();
-
         let change_recorder = ChangeRecorder::new(number_of_lines_in_destination.try_into().unwrap());
-
         let changes = gix_diff::blob::diff(gix_diff::blob::Algorithm::Histogram, &input, change_recorder);
-
-        lines_to_blame = process_changes(&mut lines_blamed, lines_to_blame, changes, suspect);
+        lines_to_blame = process_changes(&mut out, &lines_to_blame, &changes, suspect);
     }
 
     assert_eq!(lines_to_blame, vec![]);
 
     // I don’t know yet whether it would make sense to use a data structure instead that preserves
     // order on insertion.
-    lines_blamed.sort_by(|a, b| a.range_in_blamed_file.start.cmp(&b.range_in_blamed_file.start));
+    out.sort_by(|a, b| a.range_in_blamed_file.start.cmp(&b.range_in_blamed_file.start));
 
-    coalesce_blame_entries(lines_blamed)
+    coalesce_blame_entries(out)
 }