refactor

- assure the previous path is passed when diffing, and when available.
- make rewrites configurable as part of `Options`.

Author: Byron

gitoxide-core/src/repository/blame.rs

@@ -47,23 +47,19 @@ pub fn blame_file(
         options,
     )?;
     let statistics = outcome.statistics;
-    write_blame_entries(out, outcome, file)?;
+    show_blame_entries(out, outcome, file)?;
 
     if let Some(err) = err {
         writeln!(err, "{statistics:#?}")?;
     }
     Ok(())
 }
 
-fn write_blame_entries(
+fn show_blame_entries(
     mut out: impl std::io::Write,
     outcome: gix::blame::Outcome,
     source_file_name: gix::bstr::BString,
 ) -> Result<(), std::io::Error> {
-    let show_file_names = outcome
-        .entries_with_lines()
-        .any(|(entry, _)| entry.source_file_name.is_some());
-
     for (entry, lines_in_hunk) in outcome.entries_with_lines() {
         for ((actual_lno, source_lno), line) in entry
             .range_in_blamed_file()
@@ -77,13 +73,8 @@ fn write_blame_entries(
                 line_no = actual_lno + 1,
             )?;
 
-            if show_file_names {
-                if let Some(ref source_file_name) = entry.source_file_name {
-                    write!(out, "{source_file_name} ")?;
-                } else {
-                    write!(out, "{source_file_name} ")?;
-                }
-            }
+            let source_file_name = entry.source_file_name.as_ref().unwrap_or(&source_file_name);
+            write!(out, "{source_file_name} ")?;
 
             write!(out, "{src_line_no} {line}", src_line_no = source_lno + 1)?;
         }

gix-blame/src/file/function.rs

@@ -122,17 +122,15 @@ pub fn file(
         }
 
         let first_hunk_for_suspect = hunks_to_blame.iter().find(|hunk| hunk.has_suspect(&suspect));
-        let is_still_suspect = first_hunk_for_suspect.is_some();
-        if !is_still_suspect {
+        let Some(first_hunk_for_suspect) = first_hunk_for_suspect else {
             // There are no `UnblamedHunk`s associated with this `suspect`, so we can continue with
             // the next one.
             continue 'outer;
-        }
+        };
 
         // We know `first_hunk_for_suspect` can’t be `None` here because we check `is_some()`
         // above.
-        let current_file_path: BString = first_hunk_for_suspect
-            .unwrap()
+        let current_file_path = first_hunk_for_suspect
             .source_file_name
             .clone()
             .unwrap_or_else(|| file_path.to_owned());
@@ -259,6 +257,7 @@ pub fn file(
                 &mut buf,
                 &mut buf2,
                 &mut buf3,
+                options.rewrites,
             )?;
             let Some(modification) = changes_for_file_path else {
                 if more_than_one_parent {
@@ -293,6 +292,7 @@ pub fn file(
                         id,
                         previous_id,
                         file_path,
+                        file_path,
                         options.diff_algorithm,
                         &mut stats,
                     )?;
@@ -309,6 +309,7 @@ pub fn file(
                         id,
                         source_id,
                         file_path,
+                        source_location.as_ref(),
                         options.diff_algorithm,
                         &mut stats,
                     )?;
@@ -433,6 +434,8 @@ fn coalesce_blame_entries(lines_blamed: Vec<BlameEntry>) -> Vec<BlameEntry> {
         })
 }
 
+/// The union of [`gix_diff::tree::recorder::Change`] and [`gix_diff::tree_with_rewrites::Change`],
+/// keeping only the blame-relevant information.
 enum TreeDiffChange {
     Addition,
     Deletion,
@@ -497,6 +500,7 @@ fn tree_diff_at_file_path(
     commit_buf: &mut Vec<u8>,
     lhs_tree_buf: &mut Vec<u8>,
     rhs_tree_buf: &mut Vec<u8>,
+    rewrites: Option<gix_diff::Rewrites>,
 ) -> Result<Option<TreeDiffChange>, Error> {
     let parent_tree_id = find_commit(cache, &odb, &parent_id, commit_buf)?.tree_id()?;
 
@@ -513,9 +517,15 @@ fn tree_diff_at_file_path(
     // Here, we follow git’s behaviour. We return when we’ve found a `Modification`. We try a
     // second time with rename tracking when the change is either an `Addition` or a `Deletion`
     // because those can turn out to have been a `Rewrite`.
+    // TODO(perf): renames are usually rare enough to not care about the work duplication done here.
+    //             But in theory, a rename tracker could be used by us, on demand, and we could stuff the
+    //             changes in there and have it find renames, without repeating the diff.
     if matches!(result, Some(TreeDiffChange::Modification { .. })) {
         return Ok(result);
     }
+    let Some(rewrites) = rewrites else {
+        return Ok(result);
+    };
 
     let result = tree_diff_with_rewrites_at_file_path(
         &odb,
@@ -525,6 +535,7 @@ fn tree_diff_at_file_path(
         resource_cache,
         parent_tree_iter,
         tree_iter,
+        rewrites,
     )?;
 
     Ok(result)
@@ -626,7 +637,7 @@ fn tree_diff_without_rewrites_at_file_path(
     stats.trees_diffed += 1;
 
     match result {
-        Ok(_) | Err(gix_diff::tree::Error::Cancelled) => Ok(recorder.change.map(std::convert::Into::into)),
+        Ok(_) | Err(gix_diff::tree::Error::Cancelled) => Ok(recorder.change.map(Into::into)),
         Err(error) => Err(Error::DiffTree(error)),
     }
 }
@@ -640,12 +651,13 @@ fn tree_diff_with_rewrites_at_file_path(
     resource_cache: &mut gix_diff::blob::Platform,
     parent_tree_iter: gix_object::TreeRefIter<'_>,
     tree_iter: gix_object::TreeRefIter<'_>,
+    rewrites: gix_diff::Rewrites,
 ) -> Result<Option<TreeDiffChange>, Error> {
     let mut change: Option<gix_diff::tree_with_rewrites::Change> = None;
 
     let options: gix_diff::tree_with_rewrites::Options = gix_diff::tree_with_rewrites::Options {
         location: Some(gix_diff::tree::recorder::Location::Path),
-        rewrites: Some(gix_diff::Rewrites::default()),
+        rewrites: Some(rewrites),
     };
     let result = gix_diff::tree_with_rewrites(
         parent_tree_iter,
@@ -667,18 +679,20 @@ fn tree_diff_with_rewrites_at_file_path(
 
     match result {
         Ok(_) | Err(gix_diff::tree_with_rewrites::Error::Diff(gix_diff::tree::Error::Cancelled)) => {
-            Ok(change.map(std::convert::Into::into))
+            Ok(change.map(Into::into))
         }
         Err(error) => Err(Error::DiffTreeWithRewrites(error)),
     }
 }
 
+#[allow(clippy::too_many_arguments)]
 fn blob_changes(
     odb: impl gix_object::Find + gix_object::FindHeader,
     resource_cache: &mut gix_diff::blob::Platform,
     oid: ObjectId,
     previous_oid: ObjectId,
     file_path: &BStr,
+    previous_file_path: &BStr,
     diff_algorithm: gix_diff::blob::Algorithm,
     stats: &mut Statistics,
 ) -> Result<Vec<Change>, Error> {
@@ -738,7 +752,7 @@ fn blob_changes(
     resource_cache.set_resource(
         previous_oid,
         gix_object::tree::EntryKind::Blob,
-        file_path,
+        previous_file_path,
         gix_diff::blob::ResourceKind::OldOrSource,
         &odb,
     )?;

gix-blame/src/types.rs

@@ -147,6 +147,8 @@ pub struct Options {
     pub range: BlameRanges,
     /// Don't consider commits before the given date.
     pub since: Option<gix_date::Time>,
+    /// Determine if rename tracking should be performed, and how.
+    pub rewrites: Option<gix_diff::Rewrites>,
 }
 
 /// The outcome of [`file()`](crate::file()).
@@ -280,6 +282,7 @@ pub struct BlameEntry {
     /// The commit that introduced the section into the *Source File*.
     pub commit_id: ObjectId,
     /// The *Source File*'s name, in case it differs from *Blamed File*'s name.
+    /// This happens when the file was renamed.
     pub source_file_name: Option<BString>,
 }
 

gix-blame/tests/blame.rs

@@ -231,6 +231,7 @@ macro_rules! mktest {
                     diff_algorithm: gix_diff::blob::Algorithm::Histogram,
                     range: BlameRanges::default(),
                     since: None,
+                    rewrites: Some(gix_diff::Rewrites::default()),
                 },
             )?
             .entries;
@@ -315,6 +316,7 @@ fn diff_disparity() {
                 diff_algorithm: gix_diff::blob::Algorithm::Histogram,
                 range: BlameRanges::default(),
                 since: None,
+                rewrites: Some(gix_diff::Rewrites::default()),
             },
         )
         .unwrap()
@@ -349,6 +351,7 @@ fn since() {
             diff_algorithm: gix_diff::blob::Algorithm::Histogram,
             range: BlameRanges::default(),
             since: Some(gix_date::parse("2025-01-31", None).unwrap()),
+            rewrites: Some(gix_diff::Rewrites::default()),
         },
     )
     .unwrap()
@@ -386,6 +389,7 @@ mod blame_ranges {
                 diff_algorithm: gix_diff::blob::Algorithm::Histogram,
                 range: BlameRanges::from_range(1..=2),
                 since: None,
+                rewrites: Some(gix_diff::Rewrites::default()),
             },
         )
         .unwrap()
@@ -424,6 +428,7 @@ mod blame_ranges {
                 diff_algorithm: gix_diff::blob::Algorithm::Histogram,
                 range: ranges,
                 since: None,
+                rewrites: None,
             },
         )
         .unwrap()
@@ -463,6 +468,7 @@ mod blame_ranges {
                 diff_algorithm: gix_diff::blob::Algorithm::Histogram,
                 range: ranges,
                 since: None,
+                rewrites: None,
             },
         )
         .unwrap()

src/plumbing/main.rs

@@ -1590,6 +1590,7 @@ pub fn main() -> Result<()> {
                         diff_algorithm,
                         range: gix::blame::BlameRanges::from_ranges(ranges),
                         since,
+                        rewrites: Some(gix::diff::Rewrites::default()),
                     },
                     out,
                     statistics.then_some(err),