25504610947

Committed 07 May 2026 03:13PM UTC coverage: 66.25% (+0.07%) from 66.18%

Build # 25504610947

Build Type

Pull #622

github

Committed by

web-flow

Commit Message

Merge 52f3f696a into 4d6432dd3

Pull Request Pull Request #622: chore(tools): Add `start_line`/`end_line` paging to diff tools

Coverage Stats

100 of 107 new or added lines in 4 files covered. (93.46%)

25760 of 38883 relevant lines covered (66.25%)

200.27 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

98.65

/.config/jp/tools/src/git/diff_commit.rs

use std::fmt::Write;

use camino::{Utf8Path, Utf8PathBuf};
use serde_json::{Map, Value};

use super::diff_filter::{
    add_slice_markers, grep_diff, slice_diff, truncate_diff, validate_line_range,
};
use crate::util::{
    OneOrMany, ToolResult, error,
    runner::{DuctProcessRunner, ProcessRunner},
};

/// Maximum lines of diff output before truncation kicks in.
const MAX_LINES: usize = 500;

pub(crate) async fn git_diff_commit(
    root: Utf8PathBuf,
    revision: String,
    paths: OneOrMany<String>,
    pattern: Option<String>,
    context: Option<usize>,
    start_line: Option<usize>,
    end_line: Option<usize>,
    options: &Map<String, Value>,
) -> ToolResult {
    let env = super::env_from_options(options);
    let paths = paths.iter().map(AsRef::as_ref).collect::<Vec<_>>();

    // An empty `paths` array still deserializes successfully (the schema's
    // `required` only checks presence). Without this guard the tool would
    // run `git show <rev> --` with no pathspec and dump the entire commit
    // diff, defeating the drill-down purpose of the `paths` argument.
    if paths.is_empty() {
        return error(
            "`paths` must contain at least one entry. `git_diff_commit` requires explicit paths \
             to prevent dumping the whole commit diff; use `git_show` for an overview.",
        );
    }

    if let Err(msg) = validate_line_range(start_line, end_line) {
        return error(msg);
    }

    git_diff_commit_impl(
        &root,
        &revision,
        &paths,
        pattern.as_deref(),
        context,
        start_line,
        end_line,
        &DuctProcessRunner,
        &env,
    )
}

fn git_diff_commit_impl<R: ProcessRunner>(
    root: &Utf8Path,
    revision: &str,
    paths: &[&str],
    pattern: Option<&str>,
    context: Option<usize>,
    start_line: Option<usize>,
    end_line: Option<usize>,
    runner: &R,
    env: &[(&str, &str)],
) -> ToolResult {
    // `git show <rev> --format= -- <paths>` gives us just the diff for
    // specific files, with an empty format to suppress the commit header.
    let mut args: Vec<&str> = vec!["show", "--format=", revision, "--"];
    args.extend(paths);

    let output = runner.run_with_env("git", &args, root, env)?;

    if !output.status.is_success() {
        return error(format!("git show failed: {}", output.stderr.trim()));
    }

    let diff = output.stdout.trim_start().to_string();

    if diff.is_empty() {
        return Ok("No diff found for the specified revision and paths.".into());
    }

    let total_lines = diff.lines().count();
    if let Some(s) = start_line
        && s > total_lines
    {
        return error(format!(
            "`start_line` is greater than the number of diff output lines ({total_lines})."
        ));
    }

    let has_range = start_line.is_some() || end_line.is_some();

    // An explicit range bypasses the truncation cap — the user is paginating
    // and owns their window size. Three modes:
    //
    // - `pattern` (with or without `range`): grep walks the full diff so
    //   structural headers and `@@` line counters stay accurate. When a
    //   range is also set, `grep_diff` restricts matches to that window
    //   instead of pre-slicing, which would hide preceding `@@` headers and
    //   produce zero-based synthesized hunk headers.
    // - `range` only: a plain text slice of the rendered diff.
    // - neither: fall back to the default truncation cap.
    let (mut content, note): (String, Option<String>) = if let Some(pat) = pattern {
        let bounds = has_range.then(|| (start_line.unwrap_or(1), end_line.unwrap_or(total_lines)));
        let (c, n) = grep_diff(&diff, pat, context.unwrap_or(3), bounds)?;
        (c.into_owned(), n)
    } else if has_range {
        (slice_diff(&diff, start_line, end_line), None)
    } else {
        let (c, n) = truncate_diff(&diff, MAX_LINES);
        (c.into_owned(), n)
    };

    // Slice markers are added last so they survive grep filtering.
    if has_range {
        add_slice_markers(&mut content, start_line, end_line);
    }

    let mut result = String::new();
    write!(result, "```diff\n{}\n```", content.trim_end())?;
    if let Some(note) = note {
        writeln!(result, "\n\n{note}\n")?;
    }
    Ok(result.into())
}

#[cfg(test)]
#[path = "diff_commit_tests.rs"]
mod tests;

1	use std::fmt::Write;
2
3	use camino::{Utf8Path, Utf8PathBuf};
4	use serde_json::{Map, Value};
5
6	use super::diff_filter::{
7	add_slice_markers, grep_diff, slice_diff, truncate_diff, validate_line_range,
8	};
9	use crate::util::{
10	OneOrMany, ToolResult, error,
11	runner::{DuctProcessRunner, ProcessRunner},
12	};
13
14	/// Maximum lines of diff output before truncation kicks in.
15	const MAX_LINES: usize = 500;
16
17	pub(crate) async fn git_diff_commit(	5✔
18	root: Utf8PathBuf,	5✔
19	revision: String,	5✔
20	paths: OneOrMany<String>,	5✔
21	pattern: Option<String>,	5✔
22	context: Option<usize>,	5✔
23	start_line: Option<usize>,	5✔
24	end_line: Option<usize>,	5✔
25	options: &Map<String, Value>,	5✔
26	) -> ToolResult {	5✔
27	let env = super::env_from_options(options);	5✔
28	let paths = paths.iter().map(AsRef::as_ref).collect::<Vec<_>>();	5✔
29
30	// An empty `paths` array still deserializes successfully (the schema's
31	// `required` only checks presence). Without this guard the tool would
32	// run `git show <rev> --` with no pathspec and dump the entire commit
33	// diff, defeating the drill-down purpose of the `paths` argument.
34	if paths.is_empty() {	5✔
35	return error(	1✔
36	"`paths` must contain at least one entry. `git_diff_commit` requires explicit paths \
37	to prevent dumping the whole commit diff; use `git_show` for an overview.",
38	);
39	}	4✔
40
41	if let Err(msg) = validate_line_range(start_line, end_line) {	4✔
NEW 42	return error(msg);	×
43	}	4✔
44
45	git_diff_commit_impl(	4✔
46	&root,	4✔
47	&revision,	4✔
48	&paths,	4✔
49	pattern.as_deref(),	4✔
50	context,	4✔
51	start_line,	4✔
52	end_line,	4✔
53	&DuctProcessRunner,	4✔
54	&env,	4✔
55	)
56	}	5✔
57
58	fn git_diff_commit_impl<R: ProcessRunner>(	12✔
59	root: &Utf8Path,	12✔
60	revision: &str,	12✔
61	paths: &[&str],	12✔
62	pattern: Option<&str>,	12✔
63	context: Option<usize>,	12✔
64	start_line: Option<usize>,	12✔
65	end_line: Option<usize>,	12✔
66	runner: &R,	12✔
67	env: &[(&str, &str)],	12✔
68	) -> ToolResult {	12✔
69	// `git show <rev> --format= -- <paths>` gives us just the diff for
70	// specific files, with an empty format to suppress the commit header.
71	let mut args: Vec<&str> = vec!["show", "--format=", revision, "--"];	12✔
72	args.extend(paths);	12✔
73
74	let output = runner.run_with_env("git", &args, root, env)?;	12✔
75
76	if !output.status.is_success() {	12✔
77	return error(format!("git show failed: {}", output.stderr.trim()));	1✔
78	}	11✔
79
80	let diff = output.stdout.trim_start().to_string();	11✔
81
82	if diff.is_empty() {	11✔
83	return Ok("No diff found for the specified revision and paths.".into());	2✔
84	}	9✔
85
86	let total_lines = diff.lines().count();	9✔
87	if let Some(s) = start_line	9✔
88	&& s > total_lines	3✔
89	{
90	return error(format!(	1✔
91	"`start_line` is greater than the number of diff output lines ({total_lines})."
92	));
93	}	8✔
94
95	let has_range = start_line.is_some() \|\| end_line.is_some();	8✔
96
97	// An explicit range bypasses the truncation cap — the user is paginating
98	// and owns their window size. Three modes:
99	//
100	// - `pattern` (with or without `range`): grep walks the full diff so
101	// structural headers and `@@` line counters stay accurate. When a
102	// range is also set, `grep_diff` restricts matches to that window
103	// instead of pre-slicing, which would hide preceding `@@` headers and
104	// produce zero-based synthesized hunk headers.
105	// - `range` only: a plain text slice of the rendered diff.
106	// - neither: fall back to the default truncation cap.
107	let (mut content, note): (String, Option<String>) = if let Some(pat) = pattern {	8✔
108	let bounds = has_range.then(\|\| (start_line.unwrap_or(1), end_line.unwrap_or(total_lines)));	3✔
109	let (c, n) = grep_diff(&diff, pat, context.unwrap_or(3), bounds)?;	3✔
110	(c.into_owned(), n)	3✔
111	} else if has_range {	5✔
112	(slice_diff(&diff, start_line, end_line), None)	1✔
113	} else {
114	let (c, n) = truncate_diff(&diff, MAX_LINES);	4✔
115	(c.into_owned(), n)	4✔
116	};
117
118	// Slice markers are added last so they survive grep filtering.
119	if has_range {	8✔
120	add_slice_markers(&mut content, start_line, end_line);	2✔
121	}	6✔
122
123	let mut result = String::new();	8✔
124	write!(result, "```diff\n{}\n```", content.trim_end())?;	8✔
125	if let Some(note) = note {	8✔
126	writeln!(result, "\n\n{note}\n")?;	4✔
127	}	4✔
128	Ok(result.into())	8✔
129	}	12✔
130
131	#[cfg(test)]
132	#[path = "diff_commit_tests.rs"]
133	mod tests;

dcdpr / jp / 25504610947

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous