Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gix blame -L start,end #1766

Merged
merged 3 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion gitoxide-core/src/repository/blame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down Expand Up @@ -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)?;

Expand Down
2 changes: 2 additions & 0 deletions gix-blame/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
36 changes: 29 additions & 7 deletions gix-blame/src/file/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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>>,
Byron marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<Outcome, Error>
where
E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
Expand All @@ -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();
Expand Down Expand Up @@ -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.
Expand Down
22 changes: 22 additions & 0 deletions gix-blame/tests/blame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ macro_rules! mktest {
commits,
&mut resource_cache,
format!("{}.txt", $case).as_str().into(),
None,
)?
.entries;

Expand Down Expand Up @@ -254,6 +255,7 @@ fn diff_disparity() {
commits,
&mut resource_cache,
format!("{case}.txt").as_str().into(),
None,
)
.unwrap()
.entries;
Expand All @@ -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()
}
1 change: 1 addition & 0 deletions gix-blame/tests/fixtures/make_blame_repo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions src/plumbing/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 } => {
Expand Down
5 changes: 5 additions & 0 deletions src/plumbing/options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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")]
Expand Down
42 changes: 40 additions & 2 deletions src/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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 {
Byron marked this conversation as resolved.
Show resolved Hide resolved
#[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));
}
}
Loading