Merge pull request #1766 from cruessler/add-range-to-gix-blame

Add `gix blame -L start,end`

Author: Byron

gitoxide-core/src/repository/blame.rs

@@ -5,6 +5,7 @@ use std::ffi::OsStr;
 pub fn blame_file(
     mut repo: gix::Repository,
     file: &OsStr,
+    range: Option<std::ops::Range<u32>>,
     out: impl std::io::Write,
     err: Option<&mut dyn std::io::Write>,
 ) -> anyhow::Result<()> {
@@ -40,7 +41,7 @@ pub fn blame_file(
             .with_commit_graph(repo.commit_graph_if_enabled()?)
             .build()?;
     let mut resource_cache = repo.diff_resource_cache_for_tree_diff()?;
-    let outcome = gix::blame::file(&repo.objects, traverse, &mut resource_cache, file.as_bstr())?;
+    let outcome = gix::blame::file(&repo.objects, traverse, &mut resource_cache, file.as_bstr(), range)?;
     let statistics = outcome.statistics;
     write_blame_entries(out, outcome)?;
 

gix-blame/src/error.rs

@@ -27,4 +27,6 @@ pub enum Error {
     Traverse(#[source] Box<dyn std::error::Error + Send + Sync>),
     #[error(transparent)]
     DiffTree(#[from] gix_diff::tree::Error),
+    #[error("Invalid line range was given, line range is expected to be a 1-based inclusive range in the format '<start>,<end>'")]
+    InvalidLineRange,
 }

gix-blame/src/file/function.rs

@@ -25,6 +25,10 @@ use std::ops::Range;
 ///    - The first commit returned here is the first eligible commit to be responsible for parts of `file_path`.
 /// * `file_path`
 ///    - A *slash-separated* worktree-relative path to the file to blame.
+/// * `range`
+///    - A 1-based inclusive range, in order to mirror `git`’s behaviour. `Some(20..40)` represents
+///      21 lines, spanning from line 20 up to and including line 40. This will be converted to
+///      `19..40` internally as the algorithm uses 0-based ranges that are exclusive at the end.
 /// * `resource_cache`
 ///    - Used for diffing trees.
 ///
@@ -61,6 +65,7 @@ pub fn file<E>(
     traverse: impl IntoIterator<Item = Result<gix_traverse::commit::Info, E>>,
     resource_cache: &mut gix_diff::blob::Platform,
     file_path: &BStr,
+    range: Option<Range<u32>>,
 ) -> Result<Outcome, Error>
 where
     E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
@@ -85,19 +90,17 @@ where
             .tokenize()
             .map(|token| interner.intern(token))
             .count()
-    };
+    } as u32;
 
     // Binary or otherwise empty?
     if num_lines_in_blamed == 0 {
         return Ok(Outcome::default());
     }
 
-    let mut hunks_to_blame = vec![{
-        let range_in_blamed_file = 0..num_lines_in_blamed as u32;
-        UnblamedHunk {
-            range_in_blamed_file: range_in_blamed_file.clone(),
-            suspects: [(suspect, range_in_blamed_file)].into(),
-        }
+    let range_in_blamed_file = one_based_inclusive_to_zero_based_exclusive_range(range, num_lines_in_blamed)?;
+    let mut hunks_to_blame = vec![UnblamedHunk {
+        range_in_blamed_file: range_in_blamed_file.clone(),
+        suspects: [(suspect, range_in_blamed_file)].into(),
     }];
 
     let mut out = Vec::new();
@@ -260,6 +263,25 @@ where
     })
 }
 
+/// This function assumes that `range` has 1-based inclusive line numbers and converts it to the
+/// format internally used: 0-based line numbers stored in ranges that are exclusive at the
+/// end.
+fn one_based_inclusive_to_zero_based_exclusive_range(
+    range: Option<Range<u32>>,
+    max_lines: u32,
+) -> Result<Range<u32>, Error> {
+    let Some(range) = range else { return Ok(0..max_lines) };
+    if range.start == 0 {
+        return Err(Error::InvalidLineRange);
+    }
+    let start = range.start - 1;
+    let end = range.end;
+    if start >= max_lines || end > max_lines || start == end {
+        return Err(Error::InvalidLineRange);
+    }
+    Ok(start..end)
+}
+
 /// Pass ownership of each unblamed hunk of `from` to `to`.
 ///
 /// This happens when `from` didn't actually change anything in the blamed file.

gix-blame/tests/blame.rs

@@ -194,6 +194,7 @@ macro_rules! mktest {
                 commits,
                 &mut resource_cache,
                 format!("{}.txt", $case).as_str().into(),
+                None,
             )?
             .entries;
 
@@ -254,6 +255,7 @@ fn diff_disparity() {
             commits,
             &mut resource_cache,
             format!("{case}.txt").as_str().into(),
+            None,
         )
         .unwrap()
         .entries;
@@ -267,6 +269,26 @@ fn diff_disparity() {
     }
 }
 
+#[test]
+fn line_range() {
+    let Fixture {
+        odb,
+        mut resource_cache,
+        commits,
+    } = Fixture::new().unwrap();
+
+    let lines_blamed = gix_blame::file(&odb, commits, &mut resource_cache, "simple.txt".into(), Some(1..2))
+        .unwrap()
+        .entries;
+
+    assert_eq!(lines_blamed.len(), 2);
+
+    let git_dir = fixture_path().join(".git");
+    let baseline = Baseline::collect(git_dir.join("simple-lines-1-2.baseline")).unwrap();
+
+    assert_eq!(lines_blamed, baseline);
+}
+
 fn fixture_path() -> PathBuf {
     gix_testtools::scripted_fixture_read_only("make_blame_repo.sh").unwrap()
 }

gix-blame/tests/fixtures/make_blame_repo.sh

@@ -199,6 +199,7 @@ git commit -q -m c14.2
 git merge branch-that-has-one-of-the-changes || true
 
 git blame --porcelain simple.txt > .git/simple.baseline
+git blame --porcelain -L 1,2 simple.txt > .git/simple-lines-1-2.baseline
 git blame --porcelain multiline-hunks.txt > .git/multiline-hunks.baseline
 git blame --porcelain deleted-lines.txt > .git/deleted-lines.baseline
 git blame --porcelain deleted-lines-multiple-hunks.txt > .git/deleted-lines-multiple-hunks.baseline

src/plumbing/main.rs

@@ -1542,15 +1542,25 @@ pub fn main() -> Result<()> {
                 },
             ),
         },
-        Subcommands::Blame { statistics, file } => prepare_and_run(
+        Subcommands::Blame {
+            statistics,
+            file,
+            range,
+        } => prepare_and_run(
             "blame",
             trace,
             verbose,
             progress,
             progress_keep_open,
             None,
             move |_progress, out, err| {
-                core::repository::blame::blame_file(repository(Mode::Lenient)?, &file, out, statistics.then_some(err))
+                core::repository::blame::blame_file(
+                    repository(Mode::Lenient)?,
+                    &file,
+                    range,
+                    out,
+                    statistics.then_some(err),
+                )
             },
         ),
         Subcommands::Completions { shell, out_dir } => {

src/plumbing/options/mod.rs

@@ -4,6 +4,8 @@ use clap_complete::Shell;
 use gitoxide_core as core;
 use gix::bstr::BString;
 
+use crate::shared::AsRange;
+
 #[derive(Debug, clap::Parser)]
 #[clap(name = "gix", about = "The git underworld", version = option_env!("GIX_VERSION"))]
 #[clap(subcommand_required = true)]
@@ -162,6 +164,9 @@ pub enum Subcommands {
         statistics: bool,
         /// The file to create the blame information for.
         file: std::ffi::OsString,
+        /// Only blame lines in the given 1-based inclusive range '<start>,<end>', e.g. '20,40'.
+        #[clap(short='L', value_parser=AsRange)]
+        range: Option<std::ops::Range<u32>>,
     },
     /// Generate shell completions to stdout or a directory.
     #[clap(visible_alias = "generate-completions", visible_alias = "shell-completions")]

src/shared.rs

@@ -408,14 +408,40 @@ mod clap {
                 .parse_ref(cmd, arg, value)
         }
     }
+
+    #[derive(Clone)]
+    pub struct AsRange;
+
+    impl TypedValueParser for AsRange {
+        type Value = std::ops::Range<u32>;
+
+        fn parse_ref(&self, cmd: &Command, arg: Option<&Arg>, value: &OsStr) -> Result<Self::Value, Error> {
+            StringValueParser::new()
+                .try_map(|arg| -> Result<_, Box<dyn std::error::Error + Send + Sync>> {
+                    let parts = arg.split_once(',');
+                    if let Some((start, end)) = parts {
+                        let start = u32::from_str(start)?;
+                        let end = u32::from_str(end)?;
+
+                        if start <= end {
+                            return Ok(start..end);
+                        }
+                    }
+
+                    Err(Box::new(Error::new(ErrorKind::ValueValidation)))
+                })
+                .parse_ref(cmd, arg, value)
+        }
+    }
 }
 pub use self::clap::{
-    AsBString, AsHashKind, AsOutputFormat, AsPartialRefName, AsPathSpec, AsTime, CheckPathSpec, ParseRenameFraction,
+    AsBString, AsHashKind, AsOutputFormat, AsPartialRefName, AsPathSpec, AsRange, AsTime, CheckPathSpec,
+    ParseRenameFraction,
 };
 
 #[cfg(test)]
 mod value_parser_tests {
-    use super::ParseRenameFraction;
+    use super::{AsRange, ParseRenameFraction};
     use clap::Parser;
 
     #[test]
@@ -441,4 +467,16 @@ mod value_parser_tests {
         let c = Cmd::parse_from(["cmd", "-a=75"]);
         assert_eq!(c.arg, Some(Some(0.75)));
     }
+
+    #[test]
+    fn range() {
+        #[derive(Debug, clap::Parser)]
+        pub struct Cmd {
+            #[clap(long, short='l', value_parser = AsRange)]
+            pub arg: Option<std::ops::Range<u32>>,
+        }
+
+        let c = Cmd::parse_from(["cmd", "-l=1,10"]);
+        assert_eq!(c.arg, Some(1..10));
+    }
 }