Revert "Store UnblamedHunks by ObjectId"

This reverts commit c28bd5a.

Author: cruessler

gix-blame/src/lib.rs

@@ -3,7 +3,6 @@
 #![forbid(unsafe_code)]
 
 use std::{
-    collections::BTreeMap,
     ops::{Add, AddAssign, Range, SubAssign},
     path::PathBuf,
 };
@@ -223,30 +222,6 @@ impl UnblamedHunk {
     }
 }
 
-struct UnblamedHunkRecorder {
-    hunks: BTreeMap<ObjectId, Vec<UnblamedHunk>>,
-}
-
-impl UnblamedHunkRecorder {
-    fn new() -> Self {
-        Self { hunks: BTreeMap::new() }
-    }
-
-    fn add_hunk(&mut self, object_id: ObjectId, unblamed_hunk: UnblamedHunk) {
-        self.hunks
-            .entry(object_id)
-            .and_modify(|unblamed_hunks| unblamed_hunks.push(unblamed_hunk.clone()))
-            .or_insert_with(|| vec![unblamed_hunk.clone()]);
-    }
-
-    fn add_hunks(&mut self, object_id: ObjectId, new_unblamed_hunks: Vec<UnblamedHunk>) {
-        self.hunks
-            .entry(object_id)
-            .and_modify(|unblamed_hunks| unblamed_hunks.extend(new_unblamed_hunks.clone()))
-            .or_insert_with(|| new_unblamed_hunks);
-    }
-}
-
 #[derive(Clone, Debug, PartialEq)]
 pub enum Change {
     Unchanged(Range<u32>),
@@ -767,7 +742,6 @@ pub fn blame_file<E>(
     traverse: impl IntoIterator<Item = Result<gix_traverse::commit::Info, E>>,
     resource_cache: &mut gix_diff::blob::Platform,
     worktree_path: PathBuf,
-    start_id: ObjectId,
     file_path: &BStr,
 ) -> Result<Vec<BlameEntry>, E> {
     // TODO
@@ -796,33 +770,24 @@ pub fn blame_file<E>(
     // TODO Verify that `imara-diff` tokenizes lines the same way `lines` does.
     let number_of_lines = std::fs::read_to_string(absolute_path).unwrap().lines().count();
 
-    let mut unblamed_hunk_recorder = UnblamedHunkRecorder::new();
-    unblamed_hunk_recorder.add_hunk(
-        start_id,
-        UnblamedHunk::new(0..number_of_lines.try_into().unwrap(), Offset::Added(0)),
-    );
+    let mut lines_to_blame: Vec<UnblamedHunk> = vec![UnblamedHunk::new(
+        0..number_of_lines.try_into().unwrap(),
+        Offset::Added(0),
+    )];
     let mut out: Vec<BlameEntry> = vec![];
 
     for item in traverse {
         let item = item?;
         let suspect = item.id;
 
-        // TODO
-        // When we call `.remove(&suspect)`, we need to make sure that all commits that have
-        // `suspect` as their parent have already been processed. `git` accomplishes that by
-        // keeping unprocessed commits in a priority queue sorted by commit date.
-        let Some(hunks_to_blame) = unblamed_hunk_recorder.hunks.remove(&suspect) else {
-            continue;
-        };
-
         let parent_ids = item.parent_ids;
         if parent_ids.is_empty() {
             // 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
             // tree to validate this assumption.
-            out.extend(hunks_to_blame.iter().map(|hunk| {
+            out.extend(lines_to_blame.iter().map(|hunk| {
                 BlameEntry::new(
                     hunk.range_in_blamed_file.clone(),
                     // TODO
@@ -833,6 +798,8 @@ pub fn blame_file<E>(
                 )
             }));
 
+            lines_to_blame = vec![];
+
             break;
         }
 
@@ -846,16 +813,14 @@ pub fn blame_file<E>(
 
         let [ref modification]: [gix_diff::tree::recorder::Change] = changes_for_file_path[..] else {
             // None of the changes affected the file we’re currently blaming.
-            unblamed_hunk_recorder.add_hunks(last_parent_id, hunks_to_blame);
-
             continue;
         };
 
         match 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.
-                out.extend(hunks_to_blame.iter().map(|hunk| {
+                out.extend(lines_to_blame.iter().map(|hunk| {
                     BlameEntry::new(
                         hunk.range_in_blamed_file.clone(),
                         // TODO
@@ -866,20 +831,20 @@ pub fn blame_file<E>(
                     )
                 }));
 
+                lines_to_blame = vec![];
+
                 break;
             }
             gix_diff::tree::recorder::Change::Deletion { .. } => todo!(),
             gix_diff::tree::recorder::Change::Modification { previous_oid, oid, .. } => {
                 let changes = get_changes(&odb, resource_cache, *oid, *previous_oid, file_path);
 
-                let new_hunks_to_blame = process_changes(&mut out, &hunks_to_blame, &changes, suspect);
-
-                unblamed_hunk_recorder.add_hunks(last_parent_id, new_hunks_to_blame.clone());
+                lines_to_blame = process_changes(&mut out, &lines_to_blame, &changes, suspect);
             }
         }
     }
 
-    assert!(unblamed_hunk_recorder.hunks.is_empty());
+    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.

gix-blame/tests/blame.rs

@@ -111,7 +111,6 @@ struct Fixture {
     worktree_path: PathBuf,
     odb: gix_odb::Handle,
     resource_cache: gix_diff::blob::Platform,
-    start_id: ObjectId,
     commits: Vec<Result<gix_traverse::commit::Info, gix_traverse::commit::simple::Error>>,
 }
 
@@ -174,7 +173,6 @@ impl Fixture {
         Ok(Fixture {
             odb,
             worktree_path,
-            start_id: head_id,
             commits,
             resource_cache,
         })
@@ -189,7 +187,6 @@ macro_rules! mktest {
                 worktree_path,
                 odb,
                 mut resource_cache,
-                start_id,
                 commits,
             } = Fixture::new().unwrap();
 
@@ -198,7 +195,6 @@ macro_rules! mktest {
                 commits,
                 &mut resource_cache,
                 worktree_path,
-                start_id,
                 format!("{}.txt", $case).as_str().into(),
             )
             .unwrap();
@@ -242,15 +238,13 @@ fn diff_disparity() {
             worktree_path,
             odb,
             mut resource_cache,
-            start_id,
             commits,
         } = Fixture::new().unwrap();
         let lines_blamed = blame_file(
             &odb,
             commits,
             &mut resource_cache,
             worktree_path,
-            start_id,
             format!("{case}.txt").as_str().into(),
         )
         .unwrap();