diff --git a/src/diff/hunk.rs b/src/diff/hunk.rs index 81389b0..9c2c44c 100644 --- a/src/diff/hunk.rs +++ b/src/diff/hunk.rs @@ -80,9 +80,12 @@ pub struct FilteredContent { /// Each deletion references a specific line in the old file. pub deletions: Vec<(u32, String)>, - /// Kept additions (content only - position is implicit via insertion_point). - /// All additions go to the same place, so we don't need individual positions. - pub additions: Vec, + /// Kept additions with their NEW line positions. + /// + /// Pure additions all share `insertion_point` — the line number is only + /// meaningful when paired with deletions in a dense replacement, where it + /// lets `into_hunks` split a non-contiguous selection into separate hunks. + pub additions: Vec<(u32, String)>, /// Whether the original old content's last line lacked a trailing newline pub old_missing_newline: bool, @@ -118,7 +121,7 @@ impl FilteredContent { }, new: ModifiedLines { start: new_start, - lines: self.additions, + lines: self.additions.into_iter().map(|(_, c)| c).collect(), missing_final_newline: self.new_missing_newline, }, }]; @@ -165,8 +168,57 @@ impl FilteredContent { } // Case 3: Mixed (both deletions and additions) - // For now, keep as single hunk - more complex splitting could be added later if has_deletions && has_additions { + let deletion_groups = group_contiguous_lines(&self.deletions); + let addition_groups = group_contiguous_lines(&self.additions); + + // Sub-case 3a: Non-contiguous dense replacement. + // + // When deletions split into multiple non-contiguous groups, the + // single-hunk patch would not apply because git apply expects the + // deleted lines to be consecutive in HEAD. + // + // If addition groups are paired 1:1 with deletion groups (typical + // dense `-X,X,-Y,Y` selections), produce one hunk per pair. + // + // No-newline tracking does not apply here — that case keeps a + // single hunk via the fallback below. + if deletion_groups.len() > 1 + && deletion_groups.len() == addition_groups.len() + && !self.old_missing_newline + && !self.new_missing_newline + { + let mut hunks = Vec::with_capacity(deletion_groups.len()); + let mut local_delta = cumulative_delta; + for (del_group, add_group) in + deletion_groups.into_iter().zip(addition_groups.into_iter()) + { + let old_start = del_group.first_line_num; + let new_start = (old_start as i32 + local_delta) as u32; + let num_del = del_group.lines.len() as i32; + let num_add = add_group.lines.len() as i32; + + hunks.push(Hunk { + old: ModifiedLines { + start: old_start, + lines: del_group.lines.into_iter().map(|(_, c)| c).collect(), + missing_final_newline: false, + }, + new: ModifiedLines { + start: new_start, + lines: add_group.lines.into_iter().map(|(_, c)| c).collect(), + missing_final_newline: false, + }, + }); + + local_delta += num_add - num_del; + } + return hunks; + } + + // Sub-case 3b: Single hunk fallback. + // Used when the selection is contiguous, asymmetric (deletion and + // addition group counts differ), or involves no-newline state. let old_start = self .deletions .first() @@ -182,7 +234,7 @@ impl FilteredContent { }, new: ModifiedLines { start: new_start, - lines: self.additions, + lines: self.additions.into_iter().map(|(_, c)| c).collect(), missing_final_newline: self.new_missing_newline, }, }]; @@ -278,7 +330,7 @@ impl Hunk { Some(FilteredContent { insertion_point: self.old.start, deletions: old_filtered.lines, - additions: new_filtered.lines.into_iter().map(|(_, c)| c).collect(), + additions: new_filtered.lines, old_missing_newline, new_missing_newline, }) @@ -740,7 +792,7 @@ mod tests { // When filtering to only additions, deletions should be empty // and additions should contain only the selected line assert!(filtered.deletions.is_empty()); - assert_eq!(filtered.additions, vec!["added three".to_string()]); + assert_eq!(filtered.additions, vec![(12, "added three".to_string())]); assert_eq!(filtered.insertion_point, 10); } @@ -811,7 +863,10 @@ mod tests { assert!(filtered.deletions.is_empty()); assert_eq!( filtered.additions, - vec!["line eleven".to_string(), "line twelve".to_string()] + vec![ + (11, "line eleven".to_string()), + (12, "line twelve".to_string()) + ] ); assert_eq!(filtered.insertion_point, 9); } @@ -946,7 +1001,7 @@ mod tests { assert!(filtered.deletions.is_empty()); assert_eq!( filtered.additions, - vec!["ten".to_string(), "twelve".to_string()] + vec![(10, "ten".to_string()), (12, "twelve".to_string())] ); // Insertion point is the original old.start assert_eq!(filtered.insertion_point, 9); @@ -979,7 +1034,7 @@ mod tests { // Should have one deletion at position 11 and one addition assert_eq!(filtered.deletions, vec![(11, "old two".to_string())]); - assert_eq!(filtered.additions, vec!["new three".to_string()]); + assert_eq!(filtered.additions, vec![(12, "new three".to_string())]); assert_eq!(filtered.insertion_point, 10); } @@ -1085,7 +1140,7 @@ mod tests { let filtered = hunk.filter(|_| false, |n| n == 7).unwrap(); // Should preserve the no-newline flag since we kept the last line - assert_eq!(filtered.additions, vec!["second addition".to_string()]); + assert_eq!(filtered.additions, vec![(7, "second addition".to_string())]); assert!(filtered.new_missing_newline); } @@ -1100,7 +1155,7 @@ mod tests { let filtered = hunk.filter(|_| false, |n| n == 6).unwrap(); // Should NOT have no-newline flag since we didn't keep the last line - assert_eq!(filtered.additions, vec!["first addition".to_string()]); + assert_eq!(filtered.additions, vec![(6, "first addition".to_string())]); assert!(!filtered.new_missing_newline); } @@ -1119,7 +1174,7 @@ mod tests { assert!(!filtered.old_missing_newline); assert_eq!( filtered.additions, - vec!["new content with newline".to_string()] + vec![(10, "new content with newline".to_string())] ); } } @@ -1361,7 +1416,7 @@ mod proptests { // First addition should be the bridge content (same as the deletion) let bridge_content = &filtered.deletions.last().unwrap().1; prop_assert!( - filtered.additions.first() == Some(bridge_content), + filtered.additions.first().map(|(_, c)| c) == Some(bridge_content), "Bridge content must match: deletions={:?}, additions={:?}", filtered.deletions, filtered.additions ); @@ -1388,7 +1443,7 @@ mod proptests { } // Every addition must exist in hunk.new OR hunk.old (bridge synthesis) - for line in &filtered.additions { + for (_, line) in &filtered.additions { prop_assert!( hunk.new.lines.contains(line) || hunk.old.lines.contains(line), "Filtered addition {:?} not in original new {:?} or old {:?}", diff --git a/tests/e2e_test.rs b/tests/e2e_test.rs index ba0fe85..722518b 100644 --- a/tests/e2e_test.rs +++ b/tests/e2e_test.rs @@ -769,6 +769,35 @@ mod replacement { f.git_diff_cached() ); } + + /// 3.10: Non-Contiguous Replacements in Dense Hunk + /// + /// Three contiguous lines are all replaced, producing a single dense hunk + /// of `-X,-Y,-Z,+X,+Y,+Z`. Stage two of the three replacements (skip the + /// middle one) — this requires splitting the dense hunk into two output + /// hunks, each carrying one delete+add pair. + #[test] + fn non_contiguous_in_dense_hunk() { + let f = Fixture::new(); + let initial = "a\nb\nc\nd\ne\n"; + f.write_file("file.txt", initial); + f.stage_file("file.txt"); + f.commit("initial"); + + // Replace b, c, d → all contiguous in one hunk + f.write_file("file.txt", "a\nB_changed\nC_changed\nD_changed\ne\n"); + + insta::assert_snapshot!( + "replacement__non_contiguous_in_dense_hunk__diff", + f.stager.diff(&["file.txt".to_string()]).unwrap() + ); + // Stage b→B and d→D, skip c→C + f.stager.stage("file.txt:-2,2,-4,4").unwrap(); + insta::assert_snapshot!( + "replacement__non_contiguous_in_dense_hunk__staged", + f.git_diff_cached() + ); + } } // ============================================================================= diff --git a/tests/snapshots/e2e_test__replacement__replacement__non_contiguous_in_dense_hunk__diff.snap b/tests/snapshots/e2e_test__replacement__replacement__non_contiguous_in_dense_hunk__diff.snap new file mode 100644 index 0000000..e65e18f --- /dev/null +++ b/tests/snapshots/e2e_test__replacement__replacement__non_contiguous_in_dense_hunk__diff.snap @@ -0,0 +1,12 @@ +--- +source: tests/e2e_test.rs +assertion_line: 790 +expression: "f.stager.diff(&[\"file.txt\".to_string()]).unwrap()" +--- +file.txt: + -2: b + -3: c + -4: d + +2: B_changed + +3: C_changed + +4: D_changed diff --git a/tests/snapshots/e2e_test__replacement__replacement__non_contiguous_in_dense_hunk__staged.snap b/tests/snapshots/e2e_test__replacement__replacement__non_contiguous_in_dense_hunk__staged.snap new file mode 100644 index 0000000..9fd6284 --- /dev/null +++ b/tests/snapshots/e2e_test__replacement__replacement__non_contiguous_in_dense_hunk__staged.snap @@ -0,0 +1,15 @@ +--- +source: tests/e2e_test.rs +assertion_line: 796 +expression: f.git_diff_cached() +--- +diff --git a/file.txt b/file.txt +index 9405325..add706c 100644 +--- a/file.txt ++++ b/file.txt +@@ -2 +2 @@ a +-b ++B_changed +@@ -4 +4 @@ c +-d ++D_changed