From f1890313c42d8f5b347feef1f48ec53f054dff08 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Sun, 27 Apr 2025 10:53:22 +0200 Subject: [PATCH 1/4] feat!: Add `BlameRanges` to enable multi-range blame support This update replaces single-range handling with the `BlameRanges` type, allowing multiple 1-based inclusive line ranges to be specified for blame operations. It hides some of the implementation details of the range logic, prepares for compatibility with `git` behavior, and adds tests to validate multi-range scenarios. --- gix-blame/src/file/function.rs | 33 ++--- gix-blame/src/lib.rs | 2 +- gix-blame/src/types.rs | 134 +++++++++++++++++++- gix-blame/tests/blame.rs | 78 +++++++++++- gix-blame/tests/fixtures/make_blame_repo.sh | 1 + 5 files changed, 215 insertions(+), 33 deletions(-) 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..e8f7f5fc5f9 100644 --- a/gix-blame/src/types.rs +++ b/gix-blame/src/types.rs @@ -8,16 +8,142 @@ 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. +/// +/// This type 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..41); +/// +/// // Blame multiple ranges +/// let mut ranges = BlameRanges::new(); +/// ranges.add_range(1..5); // Lines 1-4 +/// ranges.add_range(10..15); // Lines 10-14 +/// ``` +/// +/// # Line Number Representation +/// +/// This type uses 1-based inclusive ranges to mirror `git`'s behaviour: +/// - A range of `20..41` 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>, +} + +impl BlameRanges { + /// Create a new empty BlameRanges instance. + /// + /// An empty instance means to blame the entire file. + pub fn new() -> Self { + Self { ranges: Vec::new() } + } + + /// 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: Range) { + self.merge_range(new_range); + } + + /// Create from a single range. + /// + /// The range should be 1-based inclusive, similar to git's line number format. + pub fn from_range(range: Range) -> Self { + Self { ranges: vec![range] } + } + + /// Create from multiple ranges. + /// + /// All ranges should be 1-based inclusive. + /// 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 + } + + /// Attempts to merge the new range with any existing ranges. + /// If no merge is possible, adds it as a new range. + fn merge_range(&mut self, new_range: Range) { + // First 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 { + // Merge the ranges by taking the minimum start and maximum end + range.start = range.start.min(new_range.start); + range.end = range.end.max(new_range.end); + return; + } + } + // If no overlap found, add as 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..83f2ff510df 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, }, ) @@ -296,7 +297,7 @@ fn line_range() { "simple.txt".into(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: Some(1..2), + range: BlameRanges::from_range(1..2), since: None, }, ) @@ -327,7 +328,7 @@ fn since() { "simple.txt".into(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: None, + range: BlameRanges::default(), since: Some(gix_date::parse("2025-01-31", None).unwrap()), }, ) @@ -342,6 +343,75 @@ fn since() { 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 { gix_testtools::scripted_fixture_read_only("make_blame_repo.sh").unwrap() } 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 From 8143d692e51c8d1b3d8c2323e83676e1be122f13 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Sun, 27 Apr 2025 22:36:46 +0200 Subject: [PATCH 2/4] adapt to changes in `gix-blame` --- src/plumbing/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 7e858fd1d8a..68cf57afe21 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -11,7 +11,7 @@ use anyhow::{anyhow, Context, Result}; use clap::{CommandFactory, Parser}; use gitoxide_core as core; use gitoxide_core::{pack::verify, repository::PathsOrPatterns}; -use gix::bstr::{io::BufReadExt, BString}; +use gix::{bstr::{io::BufReadExt, BString}}; use crate::{ plumbing::{ @@ -1578,7 +1578,7 @@ pub fn main() -> Result<()> { &file, gix::blame::Options { diff_algorithm, - range, + range: range.map(BlameRanges::from_range).unwrap_or_default(), since, }, out, From 36a6ffeea7bbde7fb3689ddf2a107e09a50e602c Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Sun, 27 Apr 2025 11:21:48 +0200 Subject: [PATCH 3/4] feat: add support for multiple blame ranges like `gix blame -L -L ...` Update the blame subcommand to handle multiple line ranges. This allows specifying multiple `-L` options similar to the usage of git. --- src/plumbing/main.rs | 4 ++-- src/plumbing/options/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 68cf57afe21..d0d69d0ca9d 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.map(BlameRanges::from_range).unwrap_or_default(), + 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..fd8445c9b48 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, From d4461e700657d049a8cbc1552f328e35b27c92c3 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 27 Apr 2025 22:10:25 +0200 Subject: [PATCH 4/4] refactor - cargo fmt - apply some IDE suggestions - partition tests more - run doc-tests - use RangeInclusive for a less confusing representation --- gix-blame/Cargo.toml | 3 - gix-blame/src/types.rs | 71 +++++++------- gix-blame/tests/blame.rs | 179 ++++++++++++++++++------------------ src/plumbing/main.rs | 2 +- src/plumbing/options/mod.rs | 2 +- src/shared.rs | 8 +- 6 files changed, 134 insertions(+), 131 deletions(-) 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/types.rs b/gix-blame/src/types.rs index e8f7f5fc5f9..d60b723e598 100644 --- a/gix-blame/src/types.rs +++ b/gix-blame/src/types.rs @@ -1,18 +1,18 @@ +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. /// -/// This type handles the conversion between git's 1-based inclusive ranges and the internal +/// It handles the conversion between git's 1-based inclusive ranges and the internal /// 0-based exclusive ranges used by the blame algorithm. /// /// # Examples @@ -21,18 +21,18 @@ use crate::Error; /// use gix_blame::BlameRanges; /// /// // Blame lines 20 through 40 (inclusive) -/// let range = BlameRanges::from_range(20..41); +/// let range = BlameRanges::from_range(20..=40); /// /// // Blame multiple ranges /// let mut ranges = BlameRanges::new(); -/// ranges.add_range(1..5); // Lines 1-4 -/// ranges.add_range(10..15); // Lines 10-14 +/// 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..41` represents 21 lines, spanning from line 20 up to and including line 40 +/// - 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 @@ -43,59 +43,60 @@ use crate::Error; pub struct BlameRanges { /// The ranges to blame, stored as 1-based inclusive ranges /// An empty Vec means blame the entire file - ranges: Vec>, + 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 { ranges: Vec::new() } - } - - /// 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: Range) { - self.merge_range(new_range); + Self::default() } /// Create from a single range. /// - /// The range should be 1-based inclusive, similar to git's line number format. - pub fn from_range(range: Range) -> Self { + /// 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 should be 1-based inclusive. + /// All ranges are 1-based. /// Overlapping or adjacent ranges will be merged. - pub fn from_ranges(ranges: Vec>) -> Self { + 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, adds it as a new range. - fn merge_range(&mut self, new_range: Range) { - // First check if this range can be merged with any existing range + /// 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 { - // Merge the ranges by taking the minimum start and maximum end - range.start = range.start.min(new_range.start); - range.end = range.end.max(new_range.end); + 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 as new range + // If no overlap found, add it as a new range self.ranges.push(new_range); } @@ -118,11 +119,11 @@ impl BlameRanges { let mut result = Vec::with_capacity(self.ranges.len()); for range in &self.ranges { - if range.start == 0 { + if *range.start() == 0 { return Err(Error::InvalidLineRange); } - let start = range.start - 1; - let end = range.end; + let start = range.start() - 1; + let end = *range.end(); if start >= max_lines || end > max_lines || start == end { return Err(Error::InvalidLineRange); } diff --git a/gix-blame/tests/blame.rs b/gix-blame/tests/blame.rs index 83f2ff510df..99e5105f6df 100644 --- a/gix-blame/tests/blame.rs +++ b/gix-blame/tests/blame.rs @@ -281,37 +281,6 @@ fn diff_disparity() { } } -#[test] -fn line_range() { - let Fixture { - odb, - mut resource_cache, - suspect, - } = Fixture::new().unwrap(); - - 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; - - 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); -} - #[test] fn since() { let Fixture { @@ -343,73 +312,109 @@ fn since() { assert_eq!(lines_blamed, baseline); } -#[test] -fn multiple_ranges_using_add_range() { - let Fixture { - odb, - mut resource_cache, - suspect, - } = Fixture::new().unwrap(); +mod blame_ranges { + use crate::{fixture_path, Baseline, Fixture}; + use gix_blame::BlameRanges; - 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 + #[test] + fn line_range() { + let Fixture { + odb, + mut resource_cache, + suspect, + } = Fixture::new().unwrap(); - 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; + 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; - assert_eq!(lines_blamed.len(), 3); // Should have 3 lines total (2 from first range + 1 from second range) + assert_eq!(lines_blamed.len(), 2); - let git_dir = fixture_path().join(".git"); - let baseline = Baseline::collect(git_dir.join("simple-lines-multiple-1-2-and-4.baseline")).unwrap(); + 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); -} + assert_eq!(lines_blamed, baseline); + } -#[test] -fn multiple_ranges_usingfrom_ranges() { - let Fixture { - odb, - mut resource_cache, - suspect, - } = Fixture::new().unwrap(); + #[test] + fn multiple_ranges_using_add_range() { + let Fixture { + odb, + mut resource_cache, + suspect, + } = Fixture::new().unwrap(); - let ranges = BlameRanges::from_ranges(vec![1..2, 1..1, 4..4]); + 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; + 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) + 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(); + 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); + 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/src/plumbing/main.rs b/src/plumbing/main.rs index d0d69d0ca9d..894b4e2031a 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -11,7 +11,7 @@ use anyhow::{anyhow, Context, Result}; use clap::{CommandFactory, Parser}; use gitoxide_core as core; use gitoxide_core::{pack::verify, repository::PathsOrPatterns}; -use gix::{bstr::{io::BufReadExt, BString}}; +use gix::bstr::{io::BufReadExt, BString}; use crate::{ plumbing::{ diff --git a/src/plumbing/options/mod.rs b/src/plumbing/options/mod.rs index fd8445c9b48..beab5743928 100644 --- a/src/plumbing/options/mod.rs +++ b/src/plumbing/options/mod.rs @@ -169,7 +169,7 @@ pub enum Subcommands { file: std::ffi::OsString, /// Only blame lines in the given 1-based inclusive range ',', e.g. '20,40'. #[clap(short='L', value_parser=AsRange, action=clap::ArgAction::Append)] - ranges: Vec>, + 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]