refactor

* use items so that abstracting the traversal is easier
* remove `gix-ref`

Author: Byron

gix-blame/Cargo.toml

@@ -18,12 +18,10 @@ gix-diff = { version = "^0.46.0", path = "../gix-diff", default-features = false
 gix-object = { version = "^0.44.0", path = "../gix-object" }
 gix-hash = { version = "^0.14.2", path = "../gix-hash" }
 gix-worktree = { version = "^0.36.0", path = "../gix-worktree", default-features = false, features = ["attributes"] }
-
-# TODO: remove dependencies below this comment by abstracting them away, or passing 'big' types as argument
-gix-ref = { version = "^0.47.0", path = "../gix-ref" }
 gix-traverse = { version = "^0.41.0", path = "../gix-traverse" }
 
 [dev-dependencies]
+gix-ref = { version = "^0.47.0", path = "../gix-ref" }
 gix-filter = { version = "^0.13.0", path = "../gix-filter" }
 gix-fs = { version = "^0.11.3", path = "../gix-fs" }
 gix-index = { version = "^0.35.0", path = "../gix-index" }

gix-blame/src/lib.rs

@@ -7,11 +7,9 @@ use std::{
     path::PathBuf,
 };
 
-use gix_diff::blob::{Platform, Sink};
-use gix_hash::{oid, ObjectId};
+use gix_hash::ObjectId;
+use gix_object::bstr::BStr;
 use gix_object::FindExt;
-use gix_ref::bstr::BStr;
-use gix_traverse::commit::Simple;
 
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum Offset {
@@ -247,7 +245,7 @@ impl ChangeRecorder {
     }
 }
 
-impl Sink for ChangeRecorder {
+impl gix_diff::blob::Sink for ChangeRecorder {
     type Out = Vec<Change>;
 
     // “imara-diff will compute a line diff by default”, so each `start` and `end` represents a
@@ -661,13 +659,13 @@ fn coalesce_blame_entries(lines_blamed: Vec<BlameEntry>) -> Vec<BlameEntry> {
 }
 
 // TODO: do not instantiate anything, get everything passed as argument.
-pub fn blame_file<T: gix_object::Find + gix_object::FindHeader>(
-    odb: &T,
-    traverse: &mut Simple<&T, fn(&oid) -> bool>,
-    resource_cache: &mut Platform,
+pub fn blame_file<E>(
+    odb: impl gix_object::Find + gix_object::FindHeader,
+    traverse: impl IntoIterator<Item = Result<gix_traverse::commit::Info, E>>,
+    resource_cache: &mut gix_diff::blob::Platform,
     worktree_path: PathBuf,
     file_path: &BStr,
-) -> Vec<BlameEntry> {
+) -> Result<Vec<BlameEntry>, E> {
     // TODO
     // At a high level, what we want to do is the following:
     //
@@ -700,15 +698,13 @@ pub fn blame_file<T: gix_object::Find + gix_object::FindHeader>(
     )];
     let mut out: Vec<BlameEntry> = vec![];
 
-    loop {
-        let item = traverse.next().unwrap().unwrap();
+    for item in traverse {
+        let item = item?;
         let suspect = item.id;
 
-        let iter = traverse.commit_iter();
-        let parent_ids = iter.parent_ids().collect::<Vec<_>>();
-
+        let parent_ids = item.parent_ids;
         if parent_ids.is_empty() {
-            // I’m not entirely sure this is correct yet. `suspect`, at this point, is the `id` of
+            // I’m not entirely sure if this is correct yet. `suspect`, at this point, is the `id` of
             // the last `item` that was yielded by `traverse`, so it makes sense to assign the
             // 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
@@ -831,5 +827,5 @@ pub fn blame_file<T: gix_object::Find + gix_object::FindHeader>(
     // order on insertion.
     out.sort_by(|a, b| a.range_in_blamed_file.start.cmp(&b.range_in_blamed_file.start));
 
-    coalesce_blame_entries(out)
+    Ok(coalesce_blame_entries(out))
 }

gix-blame/tests/blame.rs

@@ -150,7 +150,8 @@ macro_rules! mktest {
                 &mut resource_cache,
                 worktree_path,
                 format!("{}.txt", $case).as_str().into(),
-            );
+            )
+            .unwrap();
 
             assert_eq!(lines_blamed.len(), $number_of_lines);