From 452da44dc7f167f5506071a1bb8a6bb6d501c791 Mon Sep 17 00:00:00 2001
From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com>
Date: Wed, 19 Mar 2025 15:43:07 +0000
Subject: [PATCH] refactor: extract into a `blame` module

---
 helix-term/src/handlers/blame.rs | 21 +++-----
 helix-vcs/src/git.rs             | 73 ++-----------------------
 helix-vcs/src/git/blame.rs       | 93 ++++++++++++++++++++++++++++++++
 helix-vcs/src/lib.rs             | 33 +++++++++---
 4 files changed, 130 insertions(+), 90 deletions(-)
 create mode 100644 helix-vcs/src/git/blame.rs

diff --git a/helix-term/src/handlers/blame.rs b/helix-term/src/handlers/blame.rs
index 640e3a72..64cb7e08 100644
--- a/helix-term/src/handlers/blame.rs
+++ b/helix-term/src/handlers/blame.rs
@@ -41,23 +41,14 @@ impl helix_event::AsyncHook for BlameHandler {
         // convert 0-based line numbers into 1-based line numbers
         let cursor_line = cursor_line + 1;
 
-        // the line for which we compute the blame
-        // Because gix_blame doesn't care about stuff that is not commited, we have to "normalize" the
-        // line number to account for uncommited code.
-        //
-        // You'll notice that blame_line can be 0 when, for instance we have:
-        // - removed 0 lines
-        // - added 10 lines
-        // - cursor_line is 8
-        //
-        // So when our cursor is on the 10th added line or earlier, blame_line will be 0. This means
-        // the blame will be incorrect. But that's fine, because when the cursor_line is on some hunk,
-        // we can show to the user nothing at all
-        let blame_line = cursor_line.saturating_sub(added_lines_count) + removed_lines_count;
-
         let worker = tokio::spawn(async move {
             diff_providers
-                .blame_line(&file, blame_line)
+                .blame(
+                    &file,
+                    cursor_line..cursor_line,
+                    added_lines_count,
+                    removed_lines_count,
+                )
                 .map(|s| s.to_string())
         });
         self.worker = Some(worker);
diff --git a/helix-vcs/src/git.rs b/helix-vcs/src/git.rs
index b72bd0ae..b59aefb7 100644
--- a/helix-vcs/src/git.rs
+++ b/helix-vcs/src/git.rs
@@ -1,12 +1,11 @@
 use anyhow::{bail, Context, Result};
 use arc_swap::ArcSwap;
-use core::fmt;
+use gix::bstr::ByteSlice as _;
 use gix::filter::plumbing::driver::apply::Delay;
 use std::io::Read;
 use std::path::Path;
 use std::sync::Arc;
 
-use gix::bstr::{BStr, ByteSlice};
 use gix::diff::Rewrites;
 use gix::dir::entry::Status;
 use gix::objs::tree::EntryKind;
@@ -23,6 +22,9 @@ use crate::FileChange;
 #[cfg(test)]
 mod test;
 
+mod blame;
+pub use blame::*;
+
 #[inline]
 fn get_repo_dir(file: &Path) -> Result<&Path> {
     file.parent().context("file has no parent directory")
@@ -126,73 +128,6 @@ fn open_repo(path: &Path) -> Result<ThreadSafeRepository> {
     Ok(res)
 }
 
-pub struct BlameInformation {
-    pub commit_hash: String,
-    pub author_name: String,
-    pub commit_date: String,
-    pub commit_message: String,
-}
-
-impl fmt::Display for BlameInformation {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "{}, {} • {} • {}",
-            self.author_name, self.commit_date, self.commit_message, self.commit_hash
-        )
-    }
-}
-
-/// Emulates the result of running `git blame` from the command line.
-pub fn blame_line(file: &Path, line: u32) -> Result<BlameInformation> {
-    let repo_dir = get_repo_dir(file)?;
-    let repo = open_repo(repo_dir)
-        .context("failed to open git repo")?
-        .to_thread_local();
-
-    let suspect = repo.head()?.peel_to_commit_in_place()?;
-
-    let relative_path = file
-        .strip_prefix(
-            repo.path()
-                .parent()
-                .context("Could not get parent path of repository")?,
-        )
-        .unwrap_or(file)
-        .to_str()
-        .context("Could not convert path to string")?;
-
-    let traverse_all_commits = gix::traverse::commit::topo::Builder::from_iters(
-        &repo.objects,
-        [suspect.id],
-        None::<Vec<gix::ObjectId>>,
-    )
-    .build()?;
-
-    let mut resource_cache = repo.diff_resource_cache_for_tree_diff()?;
-    let latest_commit_id = gix::blame::file(
-        &repo.objects,
-        traverse_all_commits,
-        &mut resource_cache,
-        BStr::new(relative_path),
-        Some(line..line.saturating_add(0)),
-    )?
-    .entries
-    .first()
-    .context("No commits found")?
-    .commit_id;
-
-    let commit = repo.find_commit(latest_commit_id)?;
-    let author = commit.author()?;
-
-    Ok(BlameInformation {
-        commit_hash: commit.short_id()?.to_string(),
-        author_name: author.name.to_string(),
-        commit_date: author.time.format(gix::date::time::format::SHORT),
-        commit_message: commit.message()?.title.to_string(),
-    })
-}
-
 /// Emulates the result of running `git status` from the command line.
 fn status(repo: &Repository, f: impl Fn(Result<FileChange>) -> bool) -> Result<()> {
     let work_dir = repo
diff --git a/helix-vcs/src/git/blame.rs b/helix-vcs/src/git/blame.rs
new file mode 100644
index 00000000..e0cd250b
--- /dev/null
+++ b/helix-vcs/src/git/blame.rs
@@ -0,0 +1,93 @@
+use anyhow::Context as _;
+use core::fmt;
+use gix::bstr::BStr;
+use std::{ops::Range, path::Path};
+
+use super::{get_repo_dir, open_repo};
+
+pub struct BlameInformation {
+    pub commit_hash: String,
+    pub author_name: String,
+    pub commit_date: String,
+    pub commit_message: String,
+}
+
+impl fmt::Display for BlameInformation {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "{}, {} • {} • {}",
+            self.author_name, self.commit_date, self.commit_message, self.commit_hash
+        )
+    }
+}
+
+/// `git blame` a range in a file
+pub fn blame(
+    file: &Path,
+    range: Range<u32>,
+    added_lines_count: u32,
+    removed_lines_count: u32,
+) -> anyhow::Result<BlameInformation> {
+    // Because gix_blame doesn't care about stuff that is not commited, we have to "normalize" the
+    // line number to account for uncommited code.
+    //
+    // You'll notice that blame_line can be 0 when, for instance we have:
+    // - removed 0 lines
+    // - added 10 lines
+    // - cursor_line is 8
+    //
+    // So when our cursor is on the 10th added line or earlier, blame_line will be 0. This means
+    // the blame will be incorrect. But that's fine, because when the cursor_line is on some hunk,
+    // we can show to the user nothing at all
+    let normalize = |line: u32| line.saturating_sub(added_lines_count) + removed_lines_count;
+
+    let blame_range = normalize(range.start)..normalize(range.end);
+
+    let repo_dir = get_repo_dir(file)?;
+    let repo = open_repo(repo_dir)
+        .context("failed to open git repo")?
+        .to_thread_local();
+
+    let suspect = repo.head()?.peel_to_commit_in_place()?;
+
+    let relative_path = file
+        .strip_prefix(
+            repo.path()
+                .parent()
+                .context("Could not get parent path of repository")?,
+        )
+        .unwrap_or(file)
+        .to_str()
+        .context("Could not convert path to string")?;
+
+    let traverse_all_commits = gix::traverse::commit::topo::Builder::from_iters(
+        &repo.objects,
+        [suspect.id],
+        None::<Vec<gix::ObjectId>>,
+    )
+    .build()?;
+
+    let mut resource_cache = repo.diff_resource_cache_for_tree_diff()?;
+    let latest_commit_id = gix::blame::file(
+        &repo.objects,
+        traverse_all_commits,
+        &mut resource_cache,
+        BStr::new(relative_path),
+        Some(blame_range),
+    )?
+    .entries
+    .first()
+    .context("No commits found")?
+    .commit_id;
+
+    let commit = repo.find_commit(latest_commit_id)?;
+    let author = commit.author()?;
+
+    Ok(BlameInformation {
+        commit_hash: commit.short_id()?.to_string(),
+        author_name: author.name.to_string(),
+        commit_date: author.time.format(gix::date::time::format::SHORT),
+        commit_message: commit.message()?.title.to_string(),
+    })
+}
diff --git a/helix-vcs/src/lib.rs b/helix-vcs/src/lib.rs
index a513bda4..619d18ef 100644
--- a/helix-vcs/src/lib.rs
+++ b/helix-vcs/src/lib.rs
@@ -1,6 +1,8 @@
-use anyhow::{anyhow, bail, Context, Result};
+use anyhow::Context as _;
+use anyhow::{anyhow, bail, Result};
 use arc_swap::ArcSwap;
 use git::BlameInformation;
+use std::ops::Range;
 use std::{
     path::{Path, PathBuf},
     sync::Arc,
@@ -49,11 +51,24 @@ impl DiffProviderRegistry {
             })
     }
 
-    /// Blame line in a file. Lines are 1-indexed
-    pub fn blame_line(&self, file: &Path, line: u32) -> anyhow::Result<BlameInformation> {
+    /// Blame range of lines in a file. Lines are 1-indexed
+    pub fn blame(
+        &self,
+        file: &Path,
+        range: Range<u32>,
+        added_lines_count: u32,
+        removed_lines_count: u32,
+    ) -> anyhow::Result<BlameInformation> {
         self.providers
             .iter()
-            .map(|provider| provider.blame_line(file, line))
+            .map(|provider| {
+                provider.blame(
+                    file,
+                    range.start..range.end,
+                    added_lines_count,
+                    removed_lines_count,
+                )
+            })
             .next()
             .context("No provider found")?
     }
@@ -118,10 +133,16 @@ impl DiffProvider {
         }
     }
 
-    fn blame_line(&self, file: &Path, line: u32) -> Result<BlameInformation> {
+    fn blame(
+        &self,
+        file: &Path,
+        range: Range<u32>,
+        added_lines_count: u32,
+        removed_lines_count: u32,
+    ) -> Result<BlameInformation> {
         match self {
             #[cfg(feature = "git")]
-            Self::Git => git::blame_line(file, line),
+            Self::Git => git::blame(file, range, added_lines_count, removed_lines_count),
             Self::None => bail!("No blame support compiled in"),
         }
     }