diff --git a/gix-blame/Cargo.toml b/gix-blame/Cargo.toml index 9aaf563cbf9..6731854c123 100644 --- a/gix-blame/Cargo.toml +++ b/gix-blame/Cargo.toml @@ -10,9 +10,6 @@ authors = ["Christoph Rüßler ", "Sebastian Thi edition = "2021" rust-version = "1.70" -[lib] -doctest = false - [dependencies] gix-commitgraph = { version = "^0.28.0", path = "../gix-commitgraph" } gix-revwalk = { version = "^0.20.1", path = "../gix-revwalk" } diff --git a/gix-blame/src/file/function.rs b/gix-blame/src/file/function.rs index 12ef57d374d..05293053231 100644 --- a/gix-blame/src/file/function.rs +++ b/gix-blame/src/file/function.rs @@ -94,11 +94,15 @@ pub fn file( return Ok(Outcome::default()); } - let range_in_blamed_file = one_based_inclusive_to_zero_based_exclusive_range(options.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 ranges = options.range.to_zero_based_exclusive(num_lines_in_blamed)?; + let mut hunks_to_blame = Vec::with_capacity(ranges.len()); + + for range in ranges { + hunks_to_blame.push(UnblamedHunk { + range_in_blamed_file: range.clone(), + suspects: [(suspect, range)].into(), + }); + } let (mut buf, mut buf2) = (Vec::new(), Vec::new()); let commit = find_commit(cache.as_ref(), &odb, &suspect, &mut buf)?; @@ -342,25 +346,6 @@ pub fn file( }) } -/// 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>, - max_lines: u32, -) -> Result, 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. diff --git a/gix-blame/src/lib.rs b/gix-blame/src/lib.rs index d2c7a5243d7..aca4299d41c 100644 --- a/gix-blame/src/lib.rs +++ b/gix-blame/src/lib.rs @@ -17,7 +17,7 @@ mod error; pub use error::Error; mod types; -pub use types::{BlameEntry, Options, Outcome, Statistics}; +pub use types::{BlameEntry, BlameRanges, Options, Outcome, Statistics}; mod file; pub use file::function::file; diff --git a/gix-blame/src/types.rs b/gix-blame/src/types.rs index bc01e4d8bec..d60b723e598 100644 --- a/gix-blame/src/types.rs +++ b/gix-blame/src/types.rs @@ -1,23 +1,150 @@ +use gix_hash::ObjectId; +use gix_object::bstr::BString; +use smallvec::SmallVec; +use std::ops::RangeInclusive; use std::{ num::NonZeroU32, ops::{AddAssign, Range, SubAssign}, }; -use gix_hash::ObjectId; -use gix_object::bstr::BString; -use smallvec::SmallVec; - use crate::file::function::tokens_for_diffing; +use crate::Error; + +/// A type to represent one or more line ranges to blame in a file. +/// +/// It handles the conversion between git's 1-based inclusive ranges and the internal +/// 0-based exclusive ranges used by the blame algorithm. +/// +/// # Examples +/// +/// ```rust +/// use gix_blame::BlameRanges; +/// +/// // Blame lines 20 through 40 (inclusive) +/// let range = BlameRanges::from_range(20..=40); +/// +/// // Blame multiple ranges +/// let mut ranges = BlameRanges::new(); +/// ranges.add_range(1..=4); // Lines 1-4 +/// ranges.add_range(10..=14); // Lines 10-14 +/// ``` +/// +/// # Line Number Representation +/// +/// This type uses 1-based inclusive ranges to mirror `git`'s behaviour: +/// - A range of `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 +/// +/// # Empty Ranges +/// +/// An empty `BlameRanges` (created via `BlameRanges::new()` or `BlameRanges::default()`) means +/// to blame the entire file, similar to running `git blame` without line number arguments. +#[derive(Debug, Clone, Default)] +pub struct BlameRanges { + /// The ranges to blame, stored as 1-based inclusive ranges + /// An empty Vec means blame the entire file + ranges: Vec>, +} + +/// Lifecycle +impl BlameRanges { + /// Create a new empty BlameRanges instance. + /// + /// An empty instance means to blame the entire file. + pub fn new() -> Self { + Self::default() + } + + /// Create from a single range. + /// + /// The range is 1-based, similar to git's line number format. + pub fn from_range(range: RangeInclusive) -> Self { + Self { ranges: vec![range] } + } + + /// Create from multiple ranges. + /// + /// All ranges are 1-based. + /// Overlapping or adjacent ranges will be merged. + pub fn from_ranges(ranges: Vec>) -> Self { + let mut result = Self::new(); + for range in ranges { + result.merge_range(range); + } + result + } +} + +impl BlameRanges { + /// Add a single range to blame. + /// + /// The range should be 1-based inclusive. + /// If the new range overlaps with or is adjacent to an existing range, + /// they will be merged into a single range. + pub fn add_range(&mut self, new_range: RangeInclusive) { + self.merge_range(new_range); + } + + /// Attempts to merge the new range with any existing ranges. + /// If no merge is possible, add it as a new range. + fn merge_range(&mut self, new_range: RangeInclusive) { + // Check if this range can be merged with any existing range + for range in &mut self.ranges { + // Check if ranges overlap or are adjacent + if new_range.start() <= range.end() && range.start() <= new_range.end() { + *range = *range.start().min(new_range.start())..=*range.end().max(new_range.end()); + return; + } + } + // If no overlap found, add it as a new range + self.ranges.push(new_range); + } + + /// Convert the 1-based inclusive ranges to 0-based exclusive ranges. + /// + /// This is used internally by the blame algorithm to convert from git's line number format + /// to the internal format used for processing. + /// + /// # Errors + /// + /// Returns `Error::InvalidLineRange` if: + /// - Any range starts at 0 (must be 1-based) + /// - Any range extends beyond the file's length + /// - Any range has the same start and end + pub fn to_zero_based_exclusive(&self, max_lines: u32) -> Result>, Error> { + if self.ranges.is_empty() { + let range = 0..max_lines; + return Ok(vec![range]); + } + + let mut result = Vec::with_capacity(self.ranges.len()); + for range in &self.ranges { + 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); + } + result.push(start..end); + } + Ok(result) + } + + /// Returns true if no specific ranges are set (meaning blame entire file) + pub fn is_empty(&self) -> bool { + self.ranges.is_empty() + } +} /// Options to be passed to [`file()`](crate::file()). #[derive(Default, Debug, Clone)] pub struct Options { /// The algorithm to use for diffing. pub diff_algorithm: gix_diff::blob::Algorithm, - /// 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. - pub range: Option>, + /// The ranges to blame in the file. + pub range: BlameRanges, /// Don't consider commits before the given date. pub since: Option, } diff --git a/gix-blame/tests/blame.rs b/gix-blame/tests/blame.rs index 2956d59b933..99e5105f6df 100644 --- a/gix-blame/tests/blame.rs +++ b/gix-blame/tests/blame.rs @@ -1,5 +1,6 @@ use std::path::PathBuf; +use gix_blame::BlameRanges; use gix_hash::ObjectId; use gix_object::bstr; @@ -193,7 +194,7 @@ macro_rules! mktest { format!("{}.txt", $case).as_str().into(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: None, + range: BlameRanges::default(), since: None, }, )? @@ -264,7 +265,7 @@ fn diff_disparity() { format!("{case}.txt").as_str().into(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: None, + range: BlameRanges::default(), since: None, }, ) @@ -281,7 +282,7 @@ fn diff_disparity() { } #[test] -fn line_range() { +fn since() { let Fixture { odb, mut resource_cache, @@ -296,50 +297,124 @@ fn line_range() { "simple.txt".into(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: Some(1..2), - since: None, + range: BlameRanges::default(), + since: Some(gix_date::parse("2025-01-31", None).unwrap()), }, ) .unwrap() .entries; - assert_eq!(lines_blamed.len(), 2); + assert_eq!(lines_blamed.len(), 1); let git_dir = fixture_path().join(".git"); - let baseline = Baseline::collect(git_dir.join("simple-lines-1-2.baseline")).unwrap(); + let baseline = Baseline::collect(git_dir.join("simple-since.baseline")).unwrap(); assert_eq!(lines_blamed, baseline); } -#[test] -fn since() { - let Fixture { - odb, - mut resource_cache, - suspect, - } = Fixture::new().unwrap(); +mod blame_ranges { + use crate::{fixture_path, Baseline, Fixture}; + use gix_blame::BlameRanges; - let lines_blamed = gix_blame::file( - &odb, - suspect, - None, - &mut resource_cache, - "simple.txt".into(), - gix_blame::Options { - diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: None, - since: Some(gix_date::parse("2025-01-31", None).unwrap()), - }, - ) - .unwrap() - .entries; + #[test] + fn line_range() { + let Fixture { + odb, + mut resource_cache, + suspect, + } = Fixture::new().unwrap(); - assert_eq!(lines_blamed.len(), 1); + let lines_blamed = gix_blame::file( + &odb, + suspect, + None, + &mut resource_cache, + "simple.txt".into(), + gix_blame::Options { + diff_algorithm: gix_diff::blob::Algorithm::Histogram, + range: BlameRanges::from_range(1..=2), + since: None, + }, + ) + .unwrap() + .entries; - let git_dir = fixture_path().join(".git"); - let baseline = Baseline::collect(git_dir.join("simple-since.baseline")).unwrap(); + assert_eq!(lines_blamed.len(), 2); - assert_eq!(lines_blamed, baseline); + 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); + } + + #[test] + fn multiple_ranges_using_add_range() { + let Fixture { + odb, + mut resource_cache, + suspect, + } = Fixture::new().unwrap(); + + let mut ranges = BlameRanges::new(); + ranges.add_range(1..=2); // Lines 1-2 + ranges.add_range(1..=1); // Duplicate range, should be ignored + ranges.add_range(4..=4); // Line 4 + + let lines_blamed = gix_blame::file( + &odb, + suspect, + None, + &mut resource_cache, + "simple.txt".into(), + gix_blame::Options { + diff_algorithm: gix_diff::blob::Algorithm::Histogram, + range: ranges, + since: None, + }, + ) + .unwrap() + .entries; + + assert_eq!(lines_blamed.len(), 3); // Should have 3 lines total (2 from first range + 1 from second range) + + let git_dir = fixture_path().join(".git"); + let baseline = Baseline::collect(git_dir.join("simple-lines-multiple-1-2-and-4.baseline")).unwrap(); + + assert_eq!(lines_blamed, baseline); + } + + #[test] + fn multiple_ranges_usingfrom_ranges() { + let Fixture { + odb, + mut resource_cache, + suspect, + } = Fixture::new().unwrap(); + + let ranges = BlameRanges::from_ranges(vec![1..=2, 1..=1, 4..=4]); + + let lines_blamed = gix_blame::file( + &odb, + suspect, + None, + &mut resource_cache, + "simple.txt".into(), + gix_blame::Options { + diff_algorithm: gix_diff::blob::Algorithm::Histogram, + range: ranges, + since: None, + }, + ) + .unwrap() + .entries; + + assert_eq!(lines_blamed.len(), 3); // Should have 3 lines total (2 from first range + 1 from second range) + + let git_dir = fixture_path().join(".git"); + let baseline = Baseline::collect(git_dir.join("simple-lines-multiple-1-2-and-4.baseline")).unwrap(); + + assert_eq!(lines_blamed, baseline); + } } fn fixture_path() -> PathBuf { diff --git a/gix-blame/tests/fixtures/make_blame_repo.sh b/gix-blame/tests/fixtures/make_blame_repo.sh index aa366230fff..fe5ca4f1af0 100755 --- a/gix-blame/tests/fixtures/make_blame_repo.sh +++ b/gix-blame/tests/fixtures/make_blame_repo.sh @@ -227,6 +227,7 @@ git merge branch-that-has-earlier-commit || 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 -L 1,2 -L 4 simple.txt > .git/simple-lines-multiple-1-2-and-4.baseline git blame --porcelain --since 2025-01-31 simple.txt > .git/simple-since.baseline git blame --porcelain multiline-hunks.txt > .git/multiline-hunks.baseline git blame --porcelain deleted-lines.txt > .git/deleted-lines.baseline diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 7e858fd1d8a..894b4e2031a 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -1560,7 +1560,7 @@ pub fn main() -> Result<()> { Subcommands::Blame { statistics, file, - range, + ranges, since, } => prepare_and_run( "blame", @@ -1578,7 +1578,7 @@ pub fn main() -> Result<()> { &file, gix::blame::Options { diff_algorithm, - range, + range: gix::blame::BlameRanges::from_ranges(ranges), since, }, out, diff --git a/src/plumbing/options/mod.rs b/src/plumbing/options/mod.rs index ab12f3f691b..beab5743928 100644 --- a/src/plumbing/options/mod.rs +++ b/src/plumbing/options/mod.rs @@ -168,8 +168,8 @@ pub enum Subcommands { /// The file to create the blame information for. file: std::ffi::OsString, /// Only blame lines in the given 1-based inclusive range ',', e.g. '20,40'. - #[clap(short='L', value_parser=AsRange)] - range: Option>, + #[clap(short='L', value_parser=AsRange, action=clap::ArgAction::Append)] + ranges: Vec>, /// Don't consider commits before the given date. #[clap(long, value_parser=AsTime, value_name = "DATE")] since: Option, diff --git a/src/shared.rs b/src/shared.rs index 765e9df6936..9dbbfc77790 100644 --- a/src/shared.rs +++ b/src/shared.rs @@ -413,7 +413,7 @@ mod clap { pub struct AsRange; impl TypedValueParser for AsRange { - type Value = std::ops::Range; + type Value = std::ops::RangeInclusive; fn parse_ref(&self, cmd: &Command, arg: Option<&Arg>, value: &OsStr) -> Result { StringValueParser::new() @@ -424,7 +424,7 @@ mod clap { let end = u32::from_str(end)?; if start <= end { - return Ok(start..end); + return Ok(start..=end); } } @@ -474,11 +474,11 @@ mod value_parser_tests { #[derive(Debug, clap::Parser)] pub struct Cmd { #[clap(long, short='l', value_parser = AsRange)] - pub arg: Option>, + pub arg: Option>, } let c = Cmd::parse_from(["cmd", "-l=1,10"]); - assert_eq!(c.arg, Some(1..10)); + assert_eq!(c.arg, Some(1..=10)); } #[test]