Skip to content
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
77 changes: 77 additions & 0 deletions .config/jp/tools/src/git/hunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,83 @@ pub fn diff_header(diff_stdout: &str) -> Option<&str> {
diff_stdout.split_once("\n@@ ").map(|(header, _)| header)
}

/// Parsed counts and start positions extracted from a `@@ ... @@` header.
#[derive(Debug, Clone, Copy)]
pub struct HunkCounts {
pub old_start: usize,
pub old_count: usize,
pub new_count: usize,
}

/// Parse the `-OS,N` and `+Y,M` ranges from a hunk header line.
///
/// Accepts both forms with explicit count (`-5,2`) and the implicit
/// single-line form (`-5`, equivalent to `-5,1`). Anything after the second
/// `@@` is ignored.
pub fn parse_hunk_counts(header_line: &str) -> Option<HunkCounts> {
let mut parts = header_line.split_whitespace();
parts.next()?; // leading `@@`
let old_part = parts.next()?.strip_prefix('-')?;
let new_part = parts.next()?.strip_prefix('+')?;

let (old_start, old_count) = parse_range(old_part)?;
let (_new_start, new_count) = parse_range(new_part)?;

Some(HunkCounts {
old_start,
old_count,
new_count,
})
}

fn parse_range(s: &str) -> Option<(usize, usize)> {
let mut it = s.split(',');
let start: usize = it.next()?.parse().ok()?;
let count: usize = it.next().map_or(Some(1), |c| c.parse().ok())?;
Some((start, count))
}

/// Rewrite a hunk's header to use a canonical `+Y` line number.
///
/// Hunks emitted by `git diff-files --unified=0` carry `+Y` values that
/// reflect the cumulative line shift of every preceding unstaged hunk in
/// the same file. When staging only a subset of those hunks, the `+Y` of
/// each selected hunk must be recomputed: `git apply --cached
/// --unidiff-zero` positions changes by `+Y`, so a stale offset places
/// the patch at the wrong line.
///
/// `cumulative_offset` is the net `(additions - removals)` line change
/// produced by preceding *selected* hunks in the assembled patch, so that
/// the rewritten `+Y` reflects the source state at the point this hunk
/// is applied.
#[allow(clippy::cast_possible_wrap, clippy::cast_sign_loss)]
pub fn rewrite_hunk_y(hunk: &str, cumulative_offset: isize) -> Option<(String, HunkCounts)> {
let (header_line, body) = hunk.split_once('\n')?;
let counts = parse_hunk_counts(header_line)?;

// Canonical `+Y` assuming the hunk applies at its old position to the
// source state implied by `cumulative_offset`.
let canonical_y = if counts.old_count > 0 {
counts.old_start as isize
} else {
counts.old_start as isize + 1
};
let new_y = (canonical_y + cumulative_offset).max(0) as usize;

// Preserve any trailing context after the second `@@`.
let trailing = header_line
.splitn(4, ' ')
.nth(3)
.map_or(String::new(), |t| format!(" {t}"));

let new_header = format!(
"@@ -{},{} +{},{} @@{}",
counts.old_start, counts.old_count, new_y, counts.new_count, trailing
);

Some((format!("{new_header}\n{body}"), counts))
}

#[cfg(test)]
#[path = "hunk_tests.rs"]
mod tests;
29 changes: 23 additions & 6 deletions .config/jp/tools/src/git/stage_patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use serde_json::{Map, Value};

use super::{
apply::apply_patch_to_index,
hunk::{diff_header, hunk_id, split_hunks},
hunk::{diff_header, hunk_id, rewrite_hunk_y, split_hunks},
};
use crate::util::{
OneOrMany, ToolResult,
Expand Down Expand Up @@ -189,13 +189,30 @@ fn build_file_patch<R: ProcessRunner>(

// Deduplicate requested IDs and emit hunks in file order to satisfy
// `git apply`'s monotonic-line requirement for multi-hunk patches.
//
// Each selected hunk's `+Y` is recomputed: the working-tree diff bakes
// in the cumulative line shift of every preceding unstaged hunk, but a
// partial stage skips some of those shifts. We track the net effect of
// preceding *selected* hunks and rewrite each header accordingly so
// `git apply --cached --unidiff-zero` lands the change at the right
// line.
let requested: std::collections::HashSet<&str> =
requested_ids.iter().map(String::as_str).collect();
let selected: Vec<&str> = available_hunks
.iter()
.filter(|h| requested.contains(hunk_id(h).as_str()))
.map(String::as_str)
.collect();

let mut cumulative_offset: isize = 0;
let mut selected: Vec<String> = vec![];
for hunk in &available_hunks {
if !requested.contains(hunk_id(hunk).as_str()) {
continue;
}
let (rewritten, counts) = rewrite_hunk_y(hunk, cumulative_offset)
.ok_or_else(|| format!("Malformed hunk header in diff: {hunk}"))?;
#[allow(clippy::cast_possible_wrap)]
{
cumulative_offset += counts.new_count as isize - counts.old_count as isize;
}
selected.push(rewritten);
}

// Ensure the patch ends with a newline. Hunks emitted by `split_hunks`
// may have lost their trailing newline during splitting (the newline
Expand Down
43 changes: 25 additions & 18 deletions .config/jp/tools/src/git/stage_patch_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,13 @@ enum DiffLineKind {
}

/// Parsed hunk header coordinates.
///
/// Only `old_start` is tracked: `new_start` from the original full-diff hunk
/// header is intentionally discarded, since `build_sub_hunk` recomputes the
/// `+Y` line number from scratch (see the offset-correction comment there).
#[derive(Debug)]
struct HunkHeader {
old_start: usize,
new_start: usize,
}

fn parse_hunk_header(header: &str) -> Result<HunkHeader, String> {
Expand All @@ -68,20 +71,11 @@ fn parse_hunk_header(header: &str) -> Result<HunkHeader, String> {
let old_part = parts
.get(1)
.ok_or("Invalid hunk header: missing old range")?;
let new_part = parts
.get(2)
.ok_or("Invalid hunk header: missing new range")?;

let old_range = old_part.trim_start_matches('-');
let (old_start, _old_count) = parse_range(old_range)?;

let new_range = new_part.trim_start_matches('+');
let (new_start, _new_count) = parse_range(new_range)?;

Ok(HunkHeader {
old_start,
new_start,
})
Ok(HunkHeader { old_start })
}

fn parse_range(range: &str) -> Result<(usize, usize), String> {
Expand Down Expand Up @@ -239,13 +233,26 @@ fn build_sub_hunk(hunk: &str, selected: &[usize]) -> Result<String, String> {
body.push('\n');
}

// Preserve the offset between old and new positions from the original
// hunk header. This is critical for pure insertions where new_start
// differs from old_start (e.g. @@ -3,0 +4,1 @@ inserts AFTER line 3).
#[allow(clippy::cast_possible_wrap, clippy::cast_sign_loss)]
let new_start = {
let offset = header.new_start as isize - header.old_start as isize;
(old_start as isize + offset).max(0) as usize
// Compute `new_start` from the sub-hunk's own contents, NOT by
// preserving the offset of the original full-diff hunk header.
//
// The header we received comes from `git diff-files --unified=0` against
// the entire working tree, so its `+Y` reflects the cumulative line
// shift of every unstaged hunk earlier in the file. Applying a sub-hunk
// in isolation against a clean index via `git apply --cached
// --unidiff-zero` uses `+Y` to position the change — carrying that
// cumulative offset over would land the patch at the wrong line
// whenever other unstaged hunks (not part of this staging) shifted
// line numbers.
let new_start = if found_removal {
// Replacement or pure removal: new content starts at the same
// position as the first removed line.
old_start
} else {
// Pure insertion: the first new line lands at `old_start + 1`. For
// `old_start = 0` (insertion at file start) this naturally yields
// `1`.
old_start.saturating_add(1)
};

let hunk_header = format!("@@ -{old_start},{removals} +{new_start},{additions} @@");
Expand Down
4 changes: 0 additions & 4 deletions .config/jp/tools/src/git/stage_patch_lines_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,24 @@ use crate::util::runner::MockProcessRunner;
fn parse_hunk_header_simple() {
let h = parse_hunk_header("@@ -5 +5 @@").unwrap();
assert_eq!(h.old_start, 5);
assert_eq!(h.new_start, 5);
}

#[test]
fn parse_hunk_header_with_counts() {
let h = parse_hunk_header("@@ -5,3 +5,2 @@").unwrap();
assert_eq!(h.old_start, 5);
assert_eq!(h.new_start, 5);
}

#[test]
fn parse_hunk_header_zero_count() {
let h = parse_hunk_header("@@ -5,0 +5,3 @@").unwrap();
assert_eq!(h.old_start, 5);
assert_eq!(h.new_start, 5);
}

#[test]
fn parse_hunk_header_different_old_new_start() {
let h = parse_hunk_header("@@ -3,0 +4,1 @@").unwrap();
assert_eq!(h.old_start, 3);
assert_eq!(h.new_start, 4);
}

#[test]
Expand Down
107 changes: 107 additions & 0 deletions .config/jp/tools/tests/git_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ fn commit_then_modify(root: &Utf8Path, path: &str, original: &str, modified: &st
fs::write(root.join(path), modified).unwrap();
}

/// 20-line fixture used by the offset-correction regression tests.
fn lines_1_to_20() -> String {
let mut s = String::new();
for i in 1..=20 {
use std::fmt::Write as _;
writeln!(s, "line{i}").unwrap();
}
s
}

/// Extract content-addressed patch IDs from a `git_list_patches` XML response.
fn extract_patch_ids(xml: &str) -> Vec<String> {
xml.lines()
Expand Down Expand Up @@ -1341,6 +1351,103 @@ async fn sequential_staging_across_tools() {
assert_eq!(staged_content(&root, "mix.rs"), "A\nb\nc\nd\nE\n");
}

#[tokio::test]
async fn stage_patch_addition_with_unrelated_unstaged_removal() {
if !has_git() {
return;
}

let (_dir, root) = init_repo();
let original = lines_1_to_20();
let modified: String = {
let mut lines: Vec<String> = original.lines().map(String::from).collect();
for _ in 0..9 {
lines.remove(2);
}
lines.insert(9, "INSERTED".to_string());
lines.join("\n") + "\n"
};

fs::write(root.join("sp.rs"), &original).unwrap();
git(&root, &["add", "sp.rs"]);
git(&root, &["commit", "-m", "init sp"]);
fs::write(root.join("sp.rs"), &modified).unwrap();

let ids = patch_ids(&root, "sp.rs").await;
assert_eq!(ids.len(), 2, "expected 2 hunks, got {}", ids.len());

// Stage only the addition hunk (the second one).
run_ok(
ctx(&root),
tool_with_answers(
"git_stage_patch",
&json!({"patches": [{"path": "sp.rs", "ids": [&ids[1]]}]}),
&json!({"stage_changes": true}),
),
)
.await;

let expected: String = {
let mut lines: Vec<String> = original.lines().map(String::from).collect();
lines.insert(18, "INSERTED".to_string());
lines.join("\n") + "\n"
};
assert_eq!(staged_content(&root, "sp.rs"), expected);
}

#[tokio::test]
async fn stage_patch_lines_addition_with_unrelated_unstaged_removal() {
if !has_git() {
return;
}

let (_dir, root) = init_repo();

// Original file: 20 lines.
let original = lines_1_to_20();

// Working tree: remove lines 3-11 (9 lines) AND insert "INSERTED" after
// old line 18. Two distant unstaged hunks in a single file.
let modified: String = {
let mut lines: Vec<String> = original.lines().map(String::from).collect();
for _ in 0..9 {
lines.remove(2);
}
// After removals: indices 0..=10 hold line1, line2, line12..line20.
// Old line 18 is now at index 8. Insert "INSERTED" right after it.
lines.insert(9, "INSERTED".to_string());
lines.join("\n") + "\n"
};

fs::write(root.join("two_hunk.rs"), &original).unwrap();
git(&root, &["add", "two_hunk.rs"]);
git(&root, &["commit", "-m", "init two_hunk"]);
fs::write(root.join("two_hunk.rs"), &modified).unwrap();

// Expect two distinct hunks in the listing.
let ids = patch_ids(&root, "two_hunk.rs").await;
assert_eq!(ids.len(), 2, "expected 2 hunks, got {}", ids.len());

// Stage just the insertion hunk's only line.
run_ok(
ctx(&root),
tool(
"git_stage_patch_lines",
&json!({"path": "two_hunk.rs", "patch_id": &ids[1], "lines": [0]}),
),
)
.await;

// The index should match HEAD plus exactly one inserted line after
// old line 18 — the unrelated removal must remain unstaged.
let expected: String = {
let mut lines: Vec<String> = original.lines().map(String::from).collect();
lines.insert(18, "INSERTED".to_string());
lines.join("\n") + "\n"
};
assert_eq!(staged_content(&root, "two_hunk.rs"), expected);
}

#[tokio::test]
async fn stage_patch_multiple_files_single_call() {
if !has_git() {
Expand Down
Loading