From 9072c7d6258e32cf2bf84ba85ce047bcdc5f50e0 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Tue, 25 Nov 2025 18:34:42 -0800 Subject: [PATCH 1/2] paths: make `line_shift` calculation correct The `line_shift` value is used when combining diff hunks from two or more stacks together, and has an important invariant: at any point where a diff hunk from another stack could be inserted, the cumulative `line_shift` value must be the net lines (lines added less lines removed) of all the diff hunks prior to that point. This is so that the combiner knows how to shift the diff hunks. For example, suppose the combiner needs to combine two stacks; the second stack has a diff that adds a line at line 100. Suppose the combined effect of all diff hunks in the first stack prior to line 100 is a net reduction of 42 lines: the total `line_shift` value of all those diff hunks must thus be -42, so that the combiner knows that the change that the second stack has must be added at line 58 instead of line 100. Point A: Note that this invariant only needs to apply at any point where a diff hunk from another stack could be inserted. If, in a stack, there are two hunks adjacent to each other, no diff hunk may be inserted between them, so as long as their total `line_shift` value is correct, they may have any `line_shift` value they want. (In fact, it is sometimes not possible to determine what each `line_shift` value should be.) This invariant is not met by the current algorithm, so I switched the calculation for something that does. The main principles are: - If applying a hunk causes other hunks to completely disappear, the incoming hunk must bear responsibility for the `line_shift` values of the hunks that disappear by adding their values to itself. - If a hunk splits into two (only possible when applying a hunk in the middle of an existing hunk), the two resulting hunks must split the original `line_shift` value between them. Due to Point A above, the exact proportion does not matter (the two resulting hunks sandwich the hunk that split them, and all three are adjacent to each other), so I have chosen to distribute the `line_shift` value based on their sizes. - If applying a hunk causes another hunk to be reduced in size, but not completely disappear, the exact distribution of `line_shift` values in between these two hunks does not really matter, since they are adjacent (and thus Point A applies). But I have chosen to take some `line_shift` from the reduced-size hunk to give to the incoming hunk, analogous to how a completely disappearing hunk cedes all its `line_shift` to the incoming hunk, to make the `line_shift` values more reasonable (well, reasonable to me, at least). There is some code duplication due to how the diff hunks of an individual stack are combined. I thought of rewriting the combining algorithm before writing this commit (to reduce or eliminate the code duplication needed), but there were some inconsistencies in how zero-line hunks were handled, so I thought it best to correct the `line_shift` issue before making further changes. --- .../but-hunk-dependency/src/ranges/paths.rs | 303 ++++++++++++------ .../src/ranges/tests/path.rs | 54 ++-- .../src/ranges/tests/workspace.rs | 90 ++++++ .../hunk_dependency/workspace_dependencies.rs | 70 ++-- 4 files changed, 353 insertions(+), 164 deletions(-) diff --git a/crates/but-hunk-dependency/src/ranges/paths.rs b/crates/but-hunk-dependency/src/ranges/paths.rs index 7ce5886482..2245000b62 100644 --- a/crates/but-hunk-dependency/src/ranges/paths.rs +++ b/crates/but-hunk-dependency/src/ranges/paths.rs @@ -333,6 +333,10 @@ impl PathRanges { self.get_shifted_old_start(incoming_hunk.old_start), incoming_hunk.old_lines, ) { + let line_shift = self.sum_line_shift( + *first_intersecting_hunk_index, + *last_intersecting_hunk_index + 1, + ) + net_lines; let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_between( *first_intersecting_hunk_index, *last_intersecting_hunk_index + 1, @@ -342,7 +346,7 @@ impl PathRanges { commit_id, start: incoming_hunk.new_start, lines: incoming_hunk.new_lines, - line_shift: net_lines, + line_shift, }], 0, ); @@ -360,6 +364,26 @@ impl PathRanges { self.get_shifted_old_start(incoming_hunk.old_start), incoming_hunk.old_lines, ) { + let trimmed_hunk_lines = self.calculate_lines_of_trimmed_hunk( + last_intersecting_hunk, + change_type, + incoming_hunk, + "While calculating the lines of bottom trimmed hunk", + )?; + let amount_trimmed = last_intersecting_hunk + .lines + .sub_or_err(trimmed_hunk_lines)?; + + // The trimmed hunk now has fewer lines than before, so decrease + // its line shift. To compensate, increase the line shift of the + // incoming hunk by the amount of decrease. + let trimmed_hunk_line_shift = + last_intersecting_hunk.line_shift - i32::try_from(amount_trimmed)?; + let incoming_hunk_line_shift = self.sum_line_shift( + *first_intersecting_hunk_index, + *last_intersecting_hunk_index, + ) + net_lines + + i32::try_from(amount_trimmed)?; let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_between( *first_intersecting_hunk_index, *last_intersecting_hunk_index + 1, @@ -370,22 +394,15 @@ impl PathRanges { commit_id, start: incoming_hunk.new_start, lines: incoming_hunk.new_lines, - line_shift: net_lines, + line_shift: incoming_hunk_line_shift, }, HunkRange { change_type: last_intersecting_hunk.change_type, stack_id: last_intersecting_hunk.stack_id, commit_id: last_intersecting_hunk.commit_id, start: incoming_hunk.new_start + incoming_hunk.new_lines, - lines: self - .calculate_lines_of_trimmed_hunk( - last_intersecting_hunk, - change_type, - incoming_hunk, - "While calculating the lines of the bottom hunk range when incoming hunk overlaps the beginning of the second intersecting hunk range." - - )?, - line_shift: last_intersecting_hunk.line_shift, + lines: trimmed_hunk_lines, + line_shift: trimmed_hunk_line_shift, }, ], 0, @@ -402,6 +419,24 @@ impl PathRanges { self.get_shifted_old_start(incoming_hunk.old_start), incoming_hunk.old_lines, ) { + let trimmed_hunk_lines = incoming_hunk + .new_start + .sub_or_err(first_intersecting_hunk.start) + .context("While calculating the lines of top trimmed hunk")?; + let amount_trimmed = first_intersecting_hunk + .lines + .sub_or_err(trimmed_hunk_lines)?; + + // The trimmed hunk now has fewer lines than before, so decrease + // its line shift. To compensate, increase the line shift of the + // incoming hunk by the amount of decrease. + let trimmed_hunk_line_shift = + first_intersecting_hunk.line_shift - i32::try_from(amount_trimmed)?; + let incoming_hunk_line_shift = self.sum_line_shift( + *first_intersecting_hunk_index + 1, + *last_intersecting_hunk_index + 1, + ) + net_lines + + i32::try_from(amount_trimmed)?; let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_between( *first_intersecting_hunk_index, *last_intersecting_hunk_index + 1, @@ -411,11 +446,8 @@ impl PathRanges { stack_id: first_intersecting_hunk.stack_id, commit_id: first_intersecting_hunk.commit_id, start: first_intersecting_hunk.start, - lines: incoming_hunk - .new_start - .sub_or_err(first_intersecting_hunk.start) - .context("While calculating the lines when incoming hunk overlaps the end of the first intersecting hunk range.")?, - line_shift: first_intersecting_hunk.line_shift, + lines: trimmed_hunk_lines, + line_shift: trimmed_hunk_line_shift, }, HunkRange { change_type, @@ -423,7 +455,7 @@ impl PathRanges { commit_id, start: incoming_hunk.new_start, lines: incoming_hunk.new_lines, - line_shift: net_lines, + line_shift: incoming_hunk_line_shift, }, ], 1, @@ -435,6 +467,36 @@ impl PathRanges { } // 2.3. The incoming hunk is contained in the intersecting hunk ranges + let top_trimmed_hunk_lines = incoming_hunk + .new_start + .sub_or_err(first_intersecting_hunk.start) + .context("While calculating the lines of top trimmed hunk")?; + let top_amount_trimmed = first_intersecting_hunk + .lines + .sub_or_err(top_trimmed_hunk_lines)?; + let bottom_trimmed_hunk_lines = self.calculate_lines_of_trimmed_hunk( + last_intersecting_hunk, + change_type, + incoming_hunk, + "While calculating the lines of bottom trimmed hunk", + )?; + let bottom_amount_trimmed = last_intersecting_hunk + .lines + .sub_or_err(bottom_trimmed_hunk_lines)?; + + // The trimmed hunks now have fewer lines than before, so decrease their + // line shift. To compensate, increase the line shift of the incoming + // hunk by the amount of decrease. + let top_trimmed_hunk_line_shift = + first_intersecting_hunk.line_shift - i32::try_from(top_amount_trimmed)?; + let bottom_trimmed_hunk_line_shift = + last_intersecting_hunk.line_shift - i32::try_from(bottom_amount_trimmed)?; + let incoming_hunk_line_shift = self.sum_line_shift( + *first_intersecting_hunk_index + 1, + *last_intersecting_hunk_index, + ) + net_lines + + i32::try_from(top_amount_trimmed)? + + i32::try_from(bottom_amount_trimmed)?; let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_between( *first_intersecting_hunk_index, *last_intersecting_hunk_index + 1, @@ -444,11 +506,8 @@ impl PathRanges { stack_id: first_intersecting_hunk.stack_id, commit_id: first_intersecting_hunk.commit_id, start: first_intersecting_hunk.start, - lines: incoming_hunk - .new_start - .sub_or_err(first_intersecting_hunk.start) - .context("While calculating the lines of the top hunk range when incoming hunk is contained in the intersecting hunk ranges.")?, - line_shift: first_intersecting_hunk.line_shift, + lines: top_trimmed_hunk_lines, + line_shift: top_trimmed_hunk_line_shift, }, HunkRange { change_type, @@ -456,21 +515,15 @@ impl PathRanges { commit_id, start: incoming_hunk.new_start, lines: incoming_hunk.new_lines, - line_shift: net_lines, + line_shift: incoming_hunk_line_shift, }, HunkRange { change_type: last_intersecting_hunk.change_type, stack_id: last_intersecting_hunk.stack_id, commit_id: last_intersecting_hunk.commit_id, start: incoming_hunk.new_start + incoming_hunk.new_lines, - lines: self - .calculate_lines_of_trimmed_hunk( - last_intersecting_hunk, - change_type, - incoming_hunk, - "While calculating the lines of the bottom hunk range when incoming hunk is contained in the intersecting hunk ranges." - )?, - line_shift: last_intersecting_hunk.line_shift, + lines: bottom_trimmed_hunk_lines, + line_shift: bottom_trimmed_hunk_line_shift, }, ], 1, @@ -509,7 +562,7 @@ impl PathRanges { commit_id, start: incoming_hunk.new_start, lines: incoming_hunk.new_lines, - line_shift: net_lines, + line_shift: net_lines + hunk.line_shift, }], 0, ); @@ -525,6 +578,34 @@ impl PathRanges { self.get_shifted_old_start(incoming_hunk.old_start), incoming_hunk.old_lines, ) { + let top_trimmed_hunk_lines = incoming_hunk + .new_start + .sub_or_err(hunk.start) + .context("When calculating the top lines of the hunk range being split.")?; + let bottom_trimmed_hunk_lines = self.calculate_lines_of_trimmed_hunk( + &hunk, + change_type, + incoming_hunk, + "When calculating the bottom lines of the hunk range being split.", + )?; + let total_trimmed_hunk_lines = top_trimmed_hunk_lines + bottom_trimmed_hunk_lines; + let amount_trimmed = hunk.lines.sub_or_err(total_trimmed_hunk_lines)?; + + // The hunk is trimmed in the middle, splitting it into a top and a + // bottom hunk. They now have fewer lines than before, so decrease their + // line shift. To compensate, increase the line shift of the incoming + // hunk by the amount of decrease. + // + // There is no definitive way to determine how much should be + // attributed to each section. Therefore, attribute based on the + // hunk line proportion. + let total_trimmed_hunk_line_shift = hunk.line_shift - i32::try_from(amount_trimmed)?; + let top_trimmed_hunk_line_shift = total_trimmed_hunk_line_shift + * i32::try_from(top_trimmed_hunk_lines)? + / i32::try_from(total_trimmed_hunk_lines)?; + let bottom_trimmed_hunk_line_shift = + total_trimmed_hunk_line_shift - top_trimmed_hunk_line_shift; + let incoming_hunk_line_shift = net_lines + i32::try_from(amount_trimmed)?; let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_at( index, vec![ @@ -533,10 +614,8 @@ impl PathRanges { stack_id: hunk.stack_id, commit_id: hunk.commit_id, start: hunk.start, - lines: incoming_hunk.new_start.sub_or_err(hunk.start).context( - "When calculating the top lines of the hunk range being split.", - )?, - line_shift: hunk.line_shift, + lines: top_trimmed_hunk_lines, + line_shift: top_trimmed_hunk_line_shift, }, HunkRange { change_type, @@ -544,20 +623,15 @@ impl PathRanges { commit_id, start: incoming_hunk.new_start, lines: incoming_hunk.new_lines, - line_shift: net_lines, + line_shift: incoming_hunk_line_shift, }, HunkRange { change_type: hunk.change_type, stack_id: hunk.stack_id, commit_id: hunk.commit_id, start: incoming_hunk.new_start + incoming_hunk.new_lines, - lines: self.calculate_lines_of_trimmed_hunk( - &hunk, - change_type, - incoming_hunk, - "When calculating the bottom lines of the hunk range being split.", - )?, - line_shift: hunk.line_shift, + lines: bottom_trimmed_hunk_lines, + line_shift: bottom_trimmed_hunk_line_shift, }, ], 1, @@ -569,65 +643,84 @@ impl PathRanges { } // 3. The incoming hunk partially overwrites the intersecting hunk range. - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = if self - .get_shifted_old_start(incoming_hunk.old_start) - <= hunk.start - { - // The incoming hunk overlaps the beginning of the intersecting hunk range. - self.replace_hunk_ranges_at( - index, - vec![ - HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: net_lines, - }, - HunkRange { - change_type: hunk.change_type, - stack_id: hunk.stack_id, - commit_id: hunk.commit_id, - start: incoming_hunk.new_start + incoming_hunk.new_lines, - lines: self.calculate_lines_of_trimmed_hunk( - &hunk, + let (i_next_hunk_to_visit, i_first_hunk_to_shift) = + if self.get_shifted_old_start(incoming_hunk.old_start) <= hunk.start { + let bottom_trimmed_hunk_lines = self.calculate_lines_of_trimmed_hunk( + &hunk, + change_type, + incoming_hunk, + "When calculating the lines of the hunk range's beginning being trimmed.", + )?; + let bottom_amount_trimmed = hunk.lines.sub_or_err(bottom_trimmed_hunk_lines)?; + + // The hunk now has fewer lines than before, so decrease its line + // shift. To compensate, increase the line shift of the incoming + // hunk by the amount of decrease. + let bottom_trimmed_hunk_line_shift = + hunk.line_shift - i32::try_from(bottom_amount_trimmed)?; + let incoming_hunk_line_shift = net_lines + i32::try_from(bottom_amount_trimmed)?; + + // The incoming hunk overlaps the beginning of the intersecting hunk range. + self.replace_hunk_ranges_at( + index, + vec![ + HunkRange { change_type, - incoming_hunk, - "When calculating the lines of the hunk range's beginning being trimmed.", - )?, - line_shift: net_lines, - }, - ], - 0, - ) - } else { - // The incoming hunk overlaps the end of the intersecting hunk range. - self.replace_hunk_ranges_at( - index, - vec![ - HunkRange { - change_type: hunk.change_type, - stack_id: hunk.stack_id, - commit_id: hunk.commit_id, - start: hunk.start, - lines: incoming_hunk.new_start.sub_or_err(hunk.start).context( - "When calculating the lines of the hunk range's end being trimmed.", - )?, - line_shift: hunk.line_shift, - }, - HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: net_lines, - }, - ], - 1, - ) - }; + stack_id, + commit_id, + start: incoming_hunk.new_start, + lines: incoming_hunk.new_lines, + line_shift: incoming_hunk_line_shift, + }, + HunkRange { + change_type: hunk.change_type, + stack_id: hunk.stack_id, + commit_id: hunk.commit_id, + start: incoming_hunk.new_start + incoming_hunk.new_lines, + lines: bottom_trimmed_hunk_lines, + line_shift: bottom_trimmed_hunk_line_shift, + }, + ], + 0, + ) + } else { + let top_trimmed_hunk_lines = incoming_hunk + .new_start + .sub_or_err(hunk.start) + .context("When calculating the lines of the hunk range's end being trimmed.")?; + let top_amount_trimmed = hunk.lines.sub_or_err(top_trimmed_hunk_lines)?; + + // The hunk now has fewer lines than before, so decrease its line + // shift. To compensate, increase the line shift of the incoming + // hunk by the amount of decrease. + let top_trimmed_hunk_line_shift = + hunk.line_shift - i32::try_from(top_amount_trimmed)?; + let incoming_hunk_line_shift = net_lines + i32::try_from(top_amount_trimmed)?; + + // The incoming hunk overlaps the end of the intersecting hunk range. + self.replace_hunk_ranges_at( + index, + vec![ + HunkRange { + change_type: hunk.change_type, + stack_id: hunk.stack_id, + commit_id: hunk.commit_id, + start: hunk.start, + lines: top_trimmed_hunk_lines, + line_shift: top_trimmed_hunk_line_shift, + }, + HunkRange { + change_type, + stack_id, + commit_id, + start: incoming_hunk.new_start, + lines: incoming_hunk.new_lines, + line_shift: incoming_hunk_line_shift, + }, + ], + 1, + ) + }; self.track_commit_dependency(commit_id, vec![hunk.commit_id])?; *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); @@ -804,6 +897,12 @@ impl PathRanges { ) -> (usize, usize) { insert_hunk_ranges(&mut self.hunk_ranges, start, end, hunks, index_of_interest) } + + fn sum_line_shift(&self, start: usize, end: usize) -> i32 { + self.hunk_ranges[start..end] + .iter() + .fold(0, |acc, hunk_range| acc + hunk_range.line_shift) + } } /// Update the hunk ranges by inserting the new hunks at the given start and end indices. diff --git a/crates/but-hunk-dependency/src/ranges/tests/path.rs b/crates/but-hunk-dependency/src/ranges/tests/path.rs index f5c5adc4d5..381c3a0faf 100644 --- a/crates/but-hunk-dependency/src/ranges/tests/path.rs +++ b/crates/but-hunk-dependency/src/ranges/tests/path.rs @@ -142,7 +142,7 @@ a commit_id: commit_a_id, start: 1, lines: 3, - line_shift: 7 + line_shift: 3 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -150,7 +150,7 @@ a commit_id: commit_b_id, start: 4, lines: 1, - line_shift: 0 + line_shift: 1 }, HunkRange { change_type: TreeStatusKind::Addition, @@ -158,7 +158,7 @@ a commit_id: commit_a_id, start: 5, lines: 3, - line_shift: 7 + line_shift: 3 } ] ); @@ -694,7 +694,7 @@ a commit_id: commit3_id, start: 1, lines: 1, - line_shift: -1 + line_shift: 0 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -725,7 +725,7 @@ a commit_id: commit3_id, start: 1, lines: 1, - line_shift: -1 + line_shift: 0 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -764,7 +764,7 @@ a commit_id: commit3_id, start: 1, lines: 1, - line_shift: -1 + line_shift: 0 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -772,7 +772,7 @@ a commit_id: commit5_id, start: 2, lines: 3, - line_shift: 1 + line_shift: 3 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -780,7 +780,7 @@ a commit_id: commit4_id, start: 5, lines: 1, - line_shift: 2 + line_shift: 1 } ] ); @@ -810,7 +810,7 @@ a commit_id: commit5_id, start: 1, lines: 3, - line_shift: 1 + line_shift: 3 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -818,7 +818,7 @@ a commit_id: commit4_id, start: 4, lines: 1, - line_shift: 2 + line_shift: 1 } ] ); @@ -1100,7 +1100,7 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { commit_id: commit1_id, start: 1, lines: 6, - line_shift: 9 + line_shift: 6 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -1108,7 +1108,7 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { commit_id: commit2_id, start: 7, lines: 0, - line_shift: -3 + line_shift: 0 }, HunkRange { change_type: TreeStatusKind::Addition, @@ -1116,7 +1116,7 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { commit_id: commit1_id, start: 7, lines: 0, - line_shift: 9 + line_shift: 0 } ] ); @@ -1143,7 +1143,7 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { commit_id: commit3_id, start: 1, lines: 1, - line_shift: 0 + line_shift: 1 }, HunkRange { change_type: TreeStatusKind::Addition, @@ -1151,7 +1151,7 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { commit_id: commit1_id, start: 2, lines: 5, - line_shift: 0 + line_shift: 5 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -1159,7 +1159,7 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { commit_id: commit2_id, start: 7, lines: 0, - line_shift: -3 + line_shift: 0 }, HunkRange { change_type: TreeStatusKind::Addition, @@ -1167,7 +1167,7 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { commit_id: commit1_id, start: 7, lines: 0, - line_shift: 9 + line_shift: 0 } ] ); @@ -1452,7 +1452,7 @@ fn removing_line_updates_range() -> anyhow::Result<()> { commit_id: commit1_id, start: 2, lines: 0, - line_shift: 1 + line_shift: 0 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -1460,7 +1460,7 @@ fn removing_line_updates_range() -> anyhow::Result<()> { commit_id: commit2_id, start: 2, lines: 0, - line_shift: -1 + line_shift: 0 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -1468,7 +1468,7 @@ fn removing_line_updates_range() -> anyhow::Result<()> { commit_id: commit1_id, start: 2, lines: 2, - line_shift: 1 + line_shift: 0 } ] ); @@ -1703,7 +1703,7 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { commit_id: commit1_id, start: 1, lines: 2, - line_shift: 10 + line_shift: 2 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -1711,7 +1711,7 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { commit_id: commit2_id, start: 3, lines: 4, - line_shift: 3 + line_shift: 4 }, HunkRange { change_type: TreeStatusKind::Addition, @@ -1719,7 +1719,7 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { commit_id: commit1_id, start: 7, lines: 0, - line_shift: 10 + line_shift: 0 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -1727,7 +1727,7 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { commit_id: commit2_id, start: 7, lines: 0, - line_shift: -1 + line_shift: 0, }, HunkRange { change_type: TreeStatusKind::Addition, @@ -1735,7 +1735,7 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { commit_id: commit1_id, start: 7, lines: 2, - line_shift: 10 + line_shift: 2, }, HunkRange { change_type: TreeStatusKind::Modification, @@ -1743,7 +1743,7 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { commit_id: commit2_id, start: 9, lines: 2, - line_shift: 1 + line_shift: 2 }, HunkRange { change_type: TreeStatusKind::Addition, @@ -1751,7 +1751,7 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { commit_id: commit1_id, start: 11, lines: 3, - line_shift: 10 + line_shift: 3 }, HunkRange { change_type: TreeStatusKind::Modification, diff --git a/crates/but-hunk-dependency/src/ranges/tests/workspace.rs b/crates/but-hunk-dependency/src/ranges/tests/workspace.rs index abcc86194c..eb54b19839 100644 --- a/crates/but-hunk-dependency/src/ranges/tests/workspace.rs +++ b/crates/but-hunk-dependency/src/ranges/tests/workspace.rs @@ -93,6 +93,96 @@ fn workspace_simple() -> anyhow::Result<()> { Ok(()) } +#[test] +fn overlapping_commits_in_a_stack() -> anyhow::Result<()> { + let path = BString::from("/test.txt"); + + let commit1_id = id_from_hex_char('1'); + let commit2_id = id_from_hex_char('2'); + let stack1_id = StackId::generate(); + + let commit3_id = id_from_hex_char('3'); + let stack2_id = StackId::generate(); + + let workspace_ranges = WorkspaceRanges::try_from_stacks(vec![ + InputStack { + stack_id: stack1_id, + commits_from_base_to_tip: vec![ + InputCommit { + commit_id: commit1_id, + files: vec![InputFile { + path: path.clone(), + change_type: TreeStatusKind::Modification, + hunks: vec![input_hunk_from_unified_diff( + "@@ -1,4 +1,9 @@ +1 ++P1 ++P2 ++P3 ++P4 ++P5 +2 +3 +4 +", + )?], + }], + }, + InputCommit { + commit_id: commit2_id, + files: vec![InputFile { + path: path.clone(), + change_type: TreeStatusKind::Modification, + hunks: vec![input_hunk_from_unified_diff( + "@@ -1,6 +1,7 @@ +1 +P1 +P2 ++Q1 +P3 +P4 +P5 +", + )?], + }], + }, + ], + }, + InputStack { + stack_id: stack2_id, + commits_from_base_to_tip: vec![InputCommit { + commit_id: commit3_id, + files: vec![InputFile { + change_type: TreeStatusKind::Modification, + path: path.clone(), + hunks: vec![input_hunk_from_unified_diff( + "@@ -3,6 +3,7 @@ +3 +4 +5 ++R1 +6 +7 +8 +", + )?], + }], + }], + }, + ])?; + + { + // According to stack2, R1 is on line 6. Then, stack1 added 6 lines + // before that, so R1 should now be on line 12. + let dependencies = workspace_ranges.intersection(&path, 12, 1).unwrap(); + assert_eq!(dependencies.len(), 1); + assert_eq!(dependencies[0].commit_id, commit3_id); + assert_eq!(dependencies[0].stack_id, stack2_id); + } + + Ok(()) +} + #[test] fn gracefully_handle_invalid_input_commits() -> anyhow::Result<()> { let path = BString::from("/test.txt"); diff --git a/crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs b/crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs index 08013d1415..8e696a9c4b 100644 --- a/crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs +++ b/crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs @@ -304,7 +304,7 @@ fn every_commit_is_sequentially_dependent_multi_stack() -> anyhow::Result<()> { commit_id: Sha1(517d4baf97701f602df4b3672618b4199d4b1a71), start: 1, lines: 1, - line_shift: 0, + line_shift: 1, }, ], ), @@ -371,7 +371,7 @@ fn complex_file_manipulation() -> anyhow::Result<()> { commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 1, lines: 0, - line_shift: 7, + line_shift: 0, }, StableHunkRange { change_type: Modification, @@ -385,21 +385,21 @@ fn complex_file_manipulation() -> anyhow::Result<()> { commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 3, lines: 6, - line_shift: 7, + line_shift: 6, }, StableHunkRange { change_type: Modification, commit_id: Sha1(4f6e1f7479f83a4c669f45e2381d69c02b2c5e1f), start: 9, lines: 3, - line_shift: 2, + line_shift: 3, }, StableHunkRange { change_type: Addition, commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 12, lines: 0, - line_shift: 7, + line_shift: 0, }, ], ), @@ -411,28 +411,28 @@ fn complex_file_manipulation() -> anyhow::Result<()> { commit_id: Sha1(b2510a88eab2cf2f7dde23202c7a8e536905f4f7), start: 1, lines: 1, - line_shift: 0, + line_shift: 1, }, StableHunkRange { change_type: Addition, commit_id: Sha1(98231f63bb3539acf42356c886c07b140d42b68c), start: 2, lines: 5, - line_shift: 0, + line_shift: 5, }, StableHunkRange { change_type: Modification, commit_id: Sha1(dfb757b02a06e28a9d6822376b2f8829f90d46bd), start: 7, lines: 0, - line_shift: -3, + line_shift: 0, }, StableHunkRange { change_type: Addition, commit_id: Sha1(98231f63bb3539acf42356c886c07b140d42b68c), start: 7, lines: 1, - line_shift: 10, + line_shift: 1, }, ], ), @@ -460,7 +460,7 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 1, lines: 0, - line_shift: 7, + line_shift: 0, }, StableHunkRange { change_type: Modification, @@ -474,21 +474,21 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 3, lines: 6, - line_shift: 7, + line_shift: 6, }, StableHunkRange { change_type: Modification, commit_id: Sha1(4f6e1f7479f83a4c669f45e2381d69c02b2c5e1f), start: 9, lines: 3, - line_shift: 2, + line_shift: 3, }, StableHunkRange { change_type: Addition, commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 12, lines: 0, - line_shift: 7, + line_shift: 0, }, ], ), @@ -500,28 +500,28 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { commit_id: Sha1(b2510a88eab2cf2f7dde23202c7a8e536905f4f7), start: 1, lines: 1, - line_shift: 0, + line_shift: 1, }, StableHunkRange { change_type: Addition, commit_id: Sha1(98231f63bb3539acf42356c886c07b140d42b68c), start: 2, lines: 5, - line_shift: 0, + line_shift: 5, }, StableHunkRange { change_type: Modification, commit_id: Sha1(dfb757b02a06e28a9d6822376b2f8829f90d46bd), start: 7, lines: 0, - line_shift: -3, + line_shift: 0, }, StableHunkRange { change_type: Addition, commit_id: Sha1(98231f63bb3539acf42356c886c07b140d42b68c), start: 7, lines: 1, - line_shift: 10, + line_shift: 1, }, ], ), @@ -553,7 +553,7 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 3, lines: 6, - line_shift: 7, + line_shift: 6, }, ], }, @@ -573,7 +573,7 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { commit_id: Sha1(98231f63bb3539acf42356c886c07b140d42b68c), start: 2, lines: 5, - line_shift: 0, + line_shift: 5, }, ], }, @@ -601,7 +601,7 @@ fn complex_file_manipulation_multiple_hunks() -> anyhow::Result<()> { commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), start: 1, lines: 0, - line_shift: 9, + line_shift: 0, }, StableHunkRange { change_type: Modification, @@ -615,63 +615,63 @@ fn complex_file_manipulation_multiple_hunks() -> anyhow::Result<()> { commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), start: 2, lines: 2, - line_shift: 9, + line_shift: 1, }, StableHunkRange { change_type: Modification, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), start: 4, lines: 0, - line_shift: -2, + line_shift: 1, }, StableHunkRange { change_type: Modification, commit_id: Sha1(e954269ca7be71d09da50ec389b13f268a779c27), start: 5, lines: 1, - line_shift: 0, + line_shift: 1, }, StableHunkRange { change_type: Modification, commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), start: 6, lines: 1, - line_shift: 9, + line_shift: 1, }, StableHunkRange { change_type: Modification, commit_id: Sha1(e954269ca7be71d09da50ec389b13f268a779c27), start: 7, lines: 0, - line_shift: -1, + line_shift: 0, }, StableHunkRange { change_type: Modification, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), start: 7, lines: 1, - line_shift: 0, + line_shift: 1, }, StableHunkRange { change_type: Modification, commit_id: Sha1(250b92ba3b6451781f6632cd34be70db814ec4ac), start: 8, lines: 1, - line_shift: 0, + line_shift: 1, }, StableHunkRange { change_type: Modification, commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), start: 9, lines: 1, - line_shift: 9, + line_shift: 1, }, StableHunkRange { change_type: Modification, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), start: 10, lines: 1, - line_shift: 1, + line_shift: 2, }, ], ), @@ -707,7 +707,7 @@ fn complex_file_manipulation_multiple_hunks_with_uncommitted_changes() -> anyhow commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), start: 2, lines: 2, - line_shift: 9, + line_shift: 1, }, ], }, @@ -723,21 +723,21 @@ fn complex_file_manipulation_multiple_hunks_with_uncommitted_changes() -> anyhow commit_id: Sha1(e954269ca7be71d09da50ec389b13f268a779c27), start: 7, lines: 0, - line_shift: -1, + line_shift: 0, }, StableHunkRange { change_type: Modification, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), start: 7, lines: 1, - line_shift: 0, + line_shift: 1, }, StableHunkRange { change_type: Modification, commit_id: Sha1(250b92ba3b6451781f6632cd34be70db814ec4ac), start: 8, lines: 1, - line_shift: 0, + line_shift: 1, }, ], }, @@ -753,7 +753,7 @@ fn complex_file_manipulation_multiple_hunks_with_uncommitted_changes() -> anyhow commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), start: 10, lines: 1, - line_shift: 1, + line_shift: 2, }, ], }, @@ -827,7 +827,7 @@ fn dependencies_handle_complex_branch_checkout() -> anyhow::Result<()> { commit_id: Sha1(29e20dfbf0130dde207c6e3c704469a869db920e), start: 1, lines: 1, - line_shift: 0, + line_shift: 1, }, ], ), From 2435e41c007d0ff109260d3cc74d717a4a4f09a3 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Wed, 26 Nov 2025 10:04:55 -0800 Subject: [PATCH 2/2] ranges: simplify calculations, fixing bugs This is a WIP. I need to make another pass to make the identifier names consistent, add documentation, and so on, but uploading this first to check if I'm on the right track. Second most noteworthy is the fix in input_hunk_from_unified_diff. `old_start` has a special meaning when `old_lines` is 0 (same for `new_start` and `new_lines`). Most noteworthy are the bug fixes as can be seen in some tests. In crates/but-hunk-dependency/src/ranges/tests/path.rs: In removing_line_updates_range, the file goes from ? 1 1 to (in commit 1) ? a b c to (in commit 2) ? a c It can be readily seen why the existing results are wrong. In shift_is_correct_after_multiple_changes, the file ends up being 1 2 update 3 add line 1 add line 2 add line 4 4 6 update 7 add line 8 9 10 added lines at the bottom Once again, it can be readily seen why the existing results are wrong. DO NOT SUBMIT There are some insta tests that I blindly accepted the review for that I need to look into. There might be some special handling regarding deletion and recreation of files. --- crates/but-hunk-dependency/src/ranges/hunk.rs | 156 ++- .../but-hunk-dependency/src/ranges/paths.rs | 941 +---------------- .../src/ranges/tests/mod.rs | 21 +- .../src/ranges/tests/path.rs | 32 +- .../src/ranges/tests/path_utilities.rs | 957 ------------------ .../src/ranges/tests/workspace.rs | 76 +- .../tests/hunk_dependency/main.rs | 4 +- .../tests/hunk_dependency/ui.rs | 16 +- .../hunk_dependency/workspace_dependencies.rs | 77 +- 9 files changed, 215 insertions(+), 2065 deletions(-) delete mode 100644 crates/but-hunk-dependency/src/ranges/tests/path_utilities.rs diff --git a/crates/but-hunk-dependency/src/ranges/hunk.rs b/crates/but-hunk-dependency/src/ranges/hunk.rs index 03b265f423..e1599080b2 100644 --- a/crates/but-hunk-dependency/src/ranges/hunk.rs +++ b/crates/but-hunk-dependency/src/ranges/hunk.rs @@ -1,11 +1,11 @@ -use anyhow::{Context as _, Result}; +use anyhow::{Context as _, Result, bail}; use but_core::{TreeStatusKind, ref_metadata::StackId}; use crate::utils::PaniclessSubtraction; /// A struct for tracking what stack and commit a hunk belongs to as its line numbers shift with /// new changes come in from other commits and/or stacks. -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct HunkRange { /// The kind of change that was performed on the path of the parent-diff. pub change_type: TreeStatusKind, @@ -21,7 +21,117 @@ pub struct HunkRange { pub line_shift: i32, } +fn get_left_right(start: u32, lines: u32) -> Result<(i32, i32)> { + let left = if lines == 0 { start } else { start - 1 }; + Ok((i32::try_from(left)?, i32::try_from(left + lines)?)) +} + +fn set_left_right(hunk_range: &mut HunkRange, left: i32, right: i32) -> Result<()> { + hunk_range.lines = u32::try_from(right - left)?; + hunk_range.start = u32::try_from(if hunk_range.lines == 0 { + left + } else { + left + 1 + })?; + Ok(()) +} + +pub(crate) struct ReceiveResult { + pub left: Option, + pub incoming_line_shift_change: i32, + pub right: Option, +} + impl HunkRange { + pub(crate) fn receive( + mut self, + incoming_start: u32, + incoming_lines: u32, + ) -> Result { + let (incoming_left, incoming_right) = get_left_right(incoming_start, incoming_lines)?; + let (existing_left, existing_right) = get_left_right(self.start, self.lines)?; + let existing_lines = self.lines; + let existing_line_shift = self.line_shift; + + // Calculate if part or all of self will remain to the left of incoming + // existing L R + // incoming left 1-><-2-><-3 + let mut left_existing = if incoming_left <= existing_left { + // 1. There will be no hunk to the left of incoming. + None + } else if incoming_left < existing_right { + // 2. There will be a trimmed hunk to the left of incoming. + let mut left_existing = self.clone(); + set_left_right(&mut left_existing, existing_left, incoming_left)?; + Some(left_existing) + } else { + // 3. There will be an untrimmed hunk to the left of incoming. + Some(self.clone()) + }; + + // Calculate if part or all of self will remain to the right of incoming + // existing L R + // incoming right 3-><-2-><-1 + let mut right_existing = if incoming_right >= existing_right { + // 1. There will be no hunk to the right of incoming. + None + } else if incoming_right > existing_left { + // 2. There will be a trimmed hunk to the right of incoming. + set_left_right(&mut self, incoming_right, existing_right)?; + Some(self) + } else { + // 3. There will be an untrimmed hunk to the right of incoming. + Some(self) + }; + + let incoming_line_shift_change = if let (None, None) = (&left_existing, &right_existing) { + // No trace of existing hunk range, so attribute all its line shift + // to the incoming hunk range. + existing_line_shift + } else { + // Deduct trimmed lines from the `line_shift` of existing hunk(s) and + // add them to the `line_shift` of incoming + fn get_lines(hunk_range_option: &Option) -> u32 { + hunk_range_option + .as_ref() + .map_or(0, |hunk_range| hunk_range.lines) + } + let existing_lines_after_trimming = + get_lines(&left_existing) + get_lines(&right_existing); + let Some(trimmed_lines) = existing_lines.checked_sub(existing_lines_after_trimming) + else { + bail!("when calculating trimmed lines") + }; + let trimmed_lines = i32::try_from(trimmed_lines)?; + if let (Some(left_existing), Some(right_existing)) = + (&mut left_existing, &mut right_existing) + { + // Here, both `left_existing` and `right_existing` will have the original + // `line_shift` from `existing_hunk_range`. + // Distribute proportionally according to their `lines`. + let total_lines = i32::try_from(left_existing.lines + right_existing.lines)?; + let total_net_line_shift = left_existing.line_shift - trimmed_lines; + left_existing.line_shift = if total_lines == 0 { + // Arbitrarily split equally to avoid a division by zero. + total_net_line_shift / 2 + } else { + total_net_line_shift * i32::try_from(left_existing.lines)? / total_lines + }; + right_existing.line_shift = total_net_line_shift - left_existing.line_shift; + } else if let Some(left_existing) = &mut left_existing { + left_existing.line_shift -= trimmed_lines; + } else if let Some(right_existing) = &mut right_existing { + right_existing.line_shift -= trimmed_lines; + } + trimmed_lines + }; + Ok(ReceiveResult { + left: left_existing, + incoming_line_shift_change, + right: right_existing, + }) + } + /// See if this range intersects with the hunk identified with `start` and `lines`. pub(crate) fn intersects(&self, start: u32, lines: u32) -> Result { if self.change_type == TreeStatusKind::Deletion { @@ -64,46 +174,4 @@ impl HunkRange { Ok(self.start <= incoming_last_line && last_line >= start) } - - pub(crate) fn contains(&self, start: u32, lines: u32) -> bool { - if lines == 0 { - // Special case when only adding lines. - return self.start <= start && self.start + self.lines > start + 1; - } - start > self.start && start + lines <= self.start + self.lines - } - - pub(crate) fn covered_by(&self, start: u32, lines: u32) -> bool { - if start == 0 && lines == 0 { - // Special when adding lines at the top of the file. - return false; - } - self.start >= start && self.start + self.lines <= start + lines - } - - pub(crate) fn precedes(&self, start: u32) -> Result { - let last_line = (self.start + self.lines) - .sub_or_err(1) - .context("While calculating the last line")?; - - Ok(last_line < start) - } - - pub(crate) fn follows(&self, start: u32, lines: u32) -> Result { - if start == 0 && lines == 0 { - // Special case when adding lines at the top of the file. - return Ok(true); - } - - if lines == 0 { - // Special case when only adding lines. - return Ok(self.start > start); - } - - let incoming_last_line = (start + lines) - .sub_or_err(1) - .context("While calculating the last line of the incoming hunk")?; - - Ok(self.start > incoming_last_line) - } } diff --git a/crates/but-hunk-dependency/src/ranges/paths.rs b/crates/but-hunk-dependency/src/ranges/paths.rs index 2245000b62..6b446435ac 100644 --- a/crates/but-hunk-dependency/src/ranges/paths.rs +++ b/crates/but-hunk-dependency/src/ranges/paths.rs @@ -1,12 +1,7 @@ -use std::{ - collections::{HashMap, HashSet}, - vec, -}; - -use anyhow::{Context as _, bail}; +use anyhow::bail; use but_core::{TreeStatusKind, ref_metadata::StackId}; -use crate::{HunkRange, InputDiffHunk, utils::PaniclessSubtraction}; +use crate::{HunkRange, InputDiffHunk, ranges::hunk::ReceiveResult}; /// Adds sequential diffs from sequential commits for a specific path, and shifts line numbers /// with additions and deletions. It is expected that diffs are added one commit at a time, @@ -14,9 +9,7 @@ use crate::{HunkRange, InputDiffHunk, utils::PaniclessSubtraction}; #[derive(Debug, Default)] pub(crate) struct PathRanges { pub hunk_ranges: Vec, - pub commit_dependencies: HashMap>, commit_ids: Vec, - line_shift: i32, } impl PathRanges { @@ -32,915 +25,59 @@ impl PathRanges { bail!("Commit ID already in stack: {}", commit_id) } - let mut index_next_hunk_to_visit: Option = None; - let incoming_hunks_count = incoming_hunks.len(); - self.line_shift = 0; + let mut existing_hunk_ranges_iter = + itertools::put_back(std::mem::take(&mut self.hunk_ranges)); + let mut line_shift: i32 = 0; - // This is the main loop that processes all diff hunks in a commit, - // turning them into hunk ranges and inserting them in order. for incoming_hunk in &incoming_hunks { - // Handle existing hunk range is a file deletion. - if self.hunk_ranges.len() == 1 - && self.hunk_ranges[0].change_type == TreeStatusKind::Deletion - { - self.handle_file_recreation( - commit_id, - stack_id, - change_type, - incoming_hunks, - self.hunk_ranges[0], - )?; - break; - } - - // Assume that an incoming hunk deleting a file is the only diff in the commit. - if change_type == TreeStatusKind::Deletion { - self.handle_file_deletion( - incoming_hunks_count, - change_type, - incoming_hunk, - stack_id, - commit_id, - )?; - break; - } - - // If no existing hunk ranges, add all incoming hunks. - if self.hunk_ranges.is_empty() { - self.handle_add_all_hunks(stack_id, commit_id, change_type, incoming_hunks)?; - break; - } - - // Find all existing hunks that intersect with the incoming hunk. - // -- - - // If we already added a hunk, we need to check **only** the hunk ranges after that. - // -> hunks are expected to be added in top to bottom and not overlapping. - - if let Some(i) = index_next_hunk_to_visit { - // If the last hunk was added at the end, there are no more hunks to compare against. - // -> we can just append the incoming hunk - if i >= self.hunk_ranges.len() { - // Append the incoming hunk depends only of the commit that created the file (if any) - let file_creation_commit = self.find_file_creation_commit(); - - if let Some(file_creation_commit) = file_creation_commit { - self.track_commit_dependency(commit_id, vec![file_creation_commit])?; - } - - if incoming_hunk.new_lines > 0 { - self.hunk_ranges.push(HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: incoming_hunk.net_lines()?, - }); - } - - index_next_hunk_to_visit = Some(self.hunk_ranges.len()); - continue; - } - } - - // Start looking for intersecting hunks ranges after the last added hunk if there is one, - // otherwise start from the beginning. - let mut i = index_next_hunk_to_visit.unwrap_or_default(); - - // Find all intersecting hunk ranges. - let mut intersecting_hunks = vec![]; - while i < self.hunk_ranges.len() { - let current_hunk = self.hunk_ranges[i]; - - if current_hunk.lines == 0 { - i += 1; - continue; - } - - // Current hunk range starts after the end of the incoming hunk. - // -> we can stop looking for intersecting hunks - if current_hunk.follows( - self.get_shifted_old_start(incoming_hunk.old_start), - incoming_hunk.old_lines, - )? { + let mut incoming_hunk_line_shift = incoming_hunk.net_lines()?; + loop { + let Some(existing_hunk_range) = existing_hunk_ranges_iter.next() else { break; + }; + let ReceiveResult { + left, + incoming_line_shift_change, + right, + } = existing_hunk_range + .receive(incoming_hunk.old_start, incoming_hunk.old_lines)?; + if let Some(mut left) = left { + left.start = { + let Some(start) = left.start.checked_add_signed(line_shift) else { + bail!("when calculating left.start") + }; + start + }; + self.hunk_ranges.push(left); } - - // Current hunk range is ends before the start of the incoming hunk. - if current_hunk.precedes(self.get_shifted_old_start(incoming_hunk.old_start))? { - i += 1; - continue; - } - - if current_hunk.intersects( - self.get_shifted_old_start(incoming_hunk.old_start), - incoming_hunk.old_lines, - )? { - intersecting_hunks.push((i, current_hunk)); + incoming_hunk_line_shift += incoming_line_shift_change; + if let Some(right) = right { + if existing_hunk_ranges_iter.put_back(right).is_some() { + bail!("putting back too many"); + }; + break; } - - i += 1; - } - - // If there are no intersecting hunk ranges, we just add the incoming hunk. - if intersecting_hunks.is_empty() { - self.handle_no_intersecting_hunks( - commit_id, - i, - change_type, - incoming_hunk, - stack_id, - &mut index_next_hunk_to_visit, - )?; - continue; } - - // Handle multiple a single intersecting hunk. - if intersecting_hunks.len() == 1 { - self.handle_single_intersecting_hunk( - intersecting_hunks[0], - change_type, - incoming_hunk, - stack_id, - commit_id, - &mut index_next_hunk_to_visit, - )?; - continue; - } - - self.handle_multiple_intersecting_hunks( - intersecting_hunks, - change_type, - incoming_hunk, - stack_id, - commit_id, - &mut index_next_hunk_to_visit, - )?; - } - - self.commit_ids.push(commit_id); - - Ok(()) - } - - fn handle_file_recreation( - &mut self, - commit_id: gix::ObjectId, - stack_id: StackId, - change_type: TreeStatusKind, - incoming_hunks: Vec, - existing_hunk_range: HunkRange, - ) -> Result<(), anyhow::Error> { - if incoming_hunks.len() > 1 { - bail!("File recreation must be the only diff in a commit"); - } - if change_type != TreeStatusKind::Addition { - bail!("File recreation must be an addition"); - } - - self.track_commit_dependency(commit_id, vec![existing_hunk_range.commit_id])?; - self.hunk_ranges.clear(); - self.handle_add_all_hunks(stack_id, commit_id, change_type, incoming_hunks)?; - Ok(()) - } - - fn handle_file_deletion( - &mut self, - incoming_hunks_count: usize, - change_type: TreeStatusKind, - incoming_hunk: &InputDiffHunk, - stack_id: StackId, - commit_id: gix::ObjectId, - ) -> Result<(), anyhow::Error> { - // Incoming hunk is a file deletion. - // This overrides all existing hunk ranges. - if incoming_hunks_count > 1 { - bail!("File deletion must be the only diff in a commit"); - } - self.hunk_ranges = vec![HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: 0, - }]; - - // The commit that deletes a file depends on the last commit that touched it. - if let Some(previous_commit_added) = self.commit_ids.last().copied() { - self.track_commit_dependency(commit_id, vec![previous_commit_added])?; - } - - Ok(()) - } - - /// Incoming hunk affects no hunk ranges. - fn handle_no_intersecting_hunks( - &mut self, - commit_id: gix::ObjectId, - index: usize, - change_type: TreeStatusKind, - incoming_hunk: &InputDiffHunk, - stack_id: StackId, - index_next_hunk_to_visit: &mut Option, - ) -> Result<(), anyhow::Error> { - // The incoming hunk does not intersect with anything. - // The only commit that this depends on is the one that created the file. - // That commit may or may not be available in the hunk list. - let file_creation_commit = self.find_file_creation_commit(); - - if let Some(file_creation_commit) = file_creation_commit { - self.track_commit_dependency(commit_id, vec![file_creation_commit])?; - } - - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.insert_hunk_ranges_at( - index, - vec![HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: incoming_hunk.net_lines()?, - }], - 0, - ); - *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); - self.update_start_lines(i_first_hunk_to_shift, incoming_hunk.net_lines()?)?; - Ok(()) - } - - /// Look for the commit that created the file. - fn find_file_creation_commit(&mut self) -> Option { - self.hunk_ranges - .iter() - .find(|h| h.change_type == TreeStatusKind::Addition) - .map(|h| h.commit_id) - } - - fn handle_add_all_hunks( - &mut self, - stack_id: StackId, - commit_id: gix::ObjectId, - change_type: TreeStatusKind, - incoming_hunks: Vec, - ) -> anyhow::Result<()> { - for incoming_hunk in incoming_hunks { self.hunk_ranges.push(HunkRange { change_type, stack_id, commit_id, start: incoming_hunk.new_start, lines: incoming_hunk.new_lines, - line_shift: incoming_hunk.net_lines()?, + line_shift: incoming_hunk_line_shift, }); - } - Ok(()) - } - - /// Incoming hunk affects multiple hunk ranges. - fn handle_multiple_intersecting_hunks( - &mut self, - intersecting_hunk_ranges: Vec<(usize, HunkRange)>, - change_type: TreeStatusKind, - incoming_hunk: &InputDiffHunk, - stack_id: StackId, - commit_id: gix::ObjectId, - index_next_hunk_to_visit: &mut Option, - ) -> anyhow::Result<()> { - // If there are multiple intersecting hunks, we can ignore all the intersecting hunk ranges - // in the middle as they are considered to be completely overwritten by the incoming hunk. - let net_lines = incoming_hunk.net_lines()?; - let affected_commits = intersecting_hunk_ranges - .iter() - .map(|(_, hunk)| hunk.commit_id) - .collect::>(); - - // There are two possibilities: - let (first_intersecting_hunk_index, first_intersecting_hunk) = intersecting_hunk_ranges - .first() - .ok_or(anyhow::anyhow!("No first intersecting hunk"))?; - let (last_intersecting_hunk_index, last_intersecting_hunk) = intersecting_hunk_ranges - .last() - .ok_or(anyhow::anyhow!("No last intersecting hunk"))?; - - // 1. The incoming hunk completely overwrites the intersecting hunk ranges. - if first_intersecting_hunk.covered_by( - self.get_shifted_old_start(incoming_hunk.old_start), - incoming_hunk.old_lines, - ) && last_intersecting_hunk.covered_by( - self.get_shifted_old_start(incoming_hunk.old_start), - incoming_hunk.old_lines, - ) { - let line_shift = self.sum_line_shift( - *first_intersecting_hunk_index, - *last_intersecting_hunk_index + 1, - ) + net_lines; - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_between( - *first_intersecting_hunk_index, - *last_intersecting_hunk_index + 1, - vec![HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift, - }], - 0, - ); - - self.track_commit_dependency(commit_id, affected_commits.into_iter().collect())?; - *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); - self.update_start_lines(i_first_hunk_to_shift, net_lines)?; - return Ok(()); - } - - // 2. The incoming hunk partially overwrites the intersecting hunk ranges. - // 2.1. The incoming hunk overlaps the beginning of the second intersecting hunk range - // -> we can tell because the first intersecting hunk range is completely covered by the incoming hunk. - if first_intersecting_hunk.covered_by( - self.get_shifted_old_start(incoming_hunk.old_start), - incoming_hunk.old_lines, - ) { - let trimmed_hunk_lines = self.calculate_lines_of_trimmed_hunk( - last_intersecting_hunk, - change_type, - incoming_hunk, - "While calculating the lines of bottom trimmed hunk", - )?; - let amount_trimmed = last_intersecting_hunk - .lines - .sub_or_err(trimmed_hunk_lines)?; - - // The trimmed hunk now has fewer lines than before, so decrease - // its line shift. To compensate, increase the line shift of the - // incoming hunk by the amount of decrease. - let trimmed_hunk_line_shift = - last_intersecting_hunk.line_shift - i32::try_from(amount_trimmed)?; - let incoming_hunk_line_shift = self.sum_line_shift( - *first_intersecting_hunk_index, - *last_intersecting_hunk_index, - ) + net_lines - + i32::try_from(amount_trimmed)?; - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_between( - *first_intersecting_hunk_index, - *last_intersecting_hunk_index + 1, - vec![ - HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: incoming_hunk_line_shift, - }, - HunkRange { - change_type: last_intersecting_hunk.change_type, - stack_id: last_intersecting_hunk.stack_id, - commit_id: last_intersecting_hunk.commit_id, - start: incoming_hunk.new_start + incoming_hunk.new_lines, - lines: trimmed_hunk_lines, - line_shift: trimmed_hunk_line_shift, - }, - ], - 0, - ); - self.track_commit_dependency(commit_id, affected_commits.into_iter().collect())?; - *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); - self.update_start_lines(i_first_hunk_to_shift, net_lines)?; - return Ok(()); - } - - // 2.2. The incoming hunk overlaps the end of the first intersecting hunk range - // -> we can tell because the last intersecting hunk range is completely covered by the incoming hunk. - if last_intersecting_hunk.covered_by( - self.get_shifted_old_start(incoming_hunk.old_start), - incoming_hunk.old_lines, - ) { - let trimmed_hunk_lines = incoming_hunk - .new_start - .sub_or_err(first_intersecting_hunk.start) - .context("While calculating the lines of top trimmed hunk")?; - let amount_trimmed = first_intersecting_hunk - .lines - .sub_or_err(trimmed_hunk_lines)?; - - // The trimmed hunk now has fewer lines than before, so decrease - // its line shift. To compensate, increase the line shift of the - // incoming hunk by the amount of decrease. - let trimmed_hunk_line_shift = - first_intersecting_hunk.line_shift - i32::try_from(amount_trimmed)?; - let incoming_hunk_line_shift = self.sum_line_shift( - *first_intersecting_hunk_index + 1, - *last_intersecting_hunk_index + 1, - ) + net_lines - + i32::try_from(amount_trimmed)?; - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_between( - *first_intersecting_hunk_index, - *last_intersecting_hunk_index + 1, - vec![ - HunkRange { - change_type: first_intersecting_hunk.change_type, - stack_id: first_intersecting_hunk.stack_id, - commit_id: first_intersecting_hunk.commit_id, - start: first_intersecting_hunk.start, - lines: trimmed_hunk_lines, - line_shift: trimmed_hunk_line_shift, - }, - HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: incoming_hunk_line_shift, - }, - ], - 1, - ); - self.track_commit_dependency(commit_id, affected_commits.into_iter().collect())?; - *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); - self.update_start_lines(i_first_hunk_to_shift, net_lines)?; - return Ok(()); - } - - // 2.3. The incoming hunk is contained in the intersecting hunk ranges - let top_trimmed_hunk_lines = incoming_hunk - .new_start - .sub_or_err(first_intersecting_hunk.start) - .context("While calculating the lines of top trimmed hunk")?; - let top_amount_trimmed = first_intersecting_hunk - .lines - .sub_or_err(top_trimmed_hunk_lines)?; - let bottom_trimmed_hunk_lines = self.calculate_lines_of_trimmed_hunk( - last_intersecting_hunk, - change_type, - incoming_hunk, - "While calculating the lines of bottom trimmed hunk", - )?; - let bottom_amount_trimmed = last_intersecting_hunk - .lines - .sub_or_err(bottom_trimmed_hunk_lines)?; - - // The trimmed hunks now have fewer lines than before, so decrease their - // line shift. To compensate, increase the line shift of the incoming - // hunk by the amount of decrease. - let top_trimmed_hunk_line_shift = - first_intersecting_hunk.line_shift - i32::try_from(top_amount_trimmed)?; - let bottom_trimmed_hunk_line_shift = - last_intersecting_hunk.line_shift - i32::try_from(bottom_amount_trimmed)?; - let incoming_hunk_line_shift = self.sum_line_shift( - *first_intersecting_hunk_index + 1, - *last_intersecting_hunk_index, - ) + net_lines - + i32::try_from(top_amount_trimmed)? - + i32::try_from(bottom_amount_trimmed)?; - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_between( - *first_intersecting_hunk_index, - *last_intersecting_hunk_index + 1, - vec![ - HunkRange { - change_type: first_intersecting_hunk.change_type, - stack_id: first_intersecting_hunk.stack_id, - commit_id: first_intersecting_hunk.commit_id, - start: first_intersecting_hunk.start, - lines: top_trimmed_hunk_lines, - line_shift: top_trimmed_hunk_line_shift, - }, - HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: incoming_hunk_line_shift, - }, - HunkRange { - change_type: last_intersecting_hunk.change_type, - stack_id: last_intersecting_hunk.stack_id, - commit_id: last_intersecting_hunk.commit_id, - start: incoming_hunk.new_start + incoming_hunk.new_lines, - lines: bottom_trimmed_hunk_lines, - line_shift: bottom_trimmed_hunk_line_shift, - }, - ], - 1, - ); - self.track_commit_dependency(commit_id, affected_commits.into_iter().collect())?; - *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); - self.update_start_lines(i_first_hunk_to_shift, net_lines)?; - - Ok(()) - } - - /// Incoming hunk only affects a single hunk range. - fn handle_single_intersecting_hunk( - &mut self, - intersecting_hunk_range: (usize, HunkRange), - change_type: TreeStatusKind, - incoming_hunk: &InputDiffHunk, - stack_id: StackId, - commit_id: gix::ObjectId, - index_next_hunk_to_visit: &mut Option, - ) -> anyhow::Result<()> { - // If there is only one intersecting hunk range there are three possibilities: - let (index, hunk) = intersecting_hunk_range; - let net_lines = incoming_hunk.net_lines()?; - - // 1. The incoming hunk completely overwrites the intersecting hunk. - if hunk.covered_by( - self.get_shifted_old_start(incoming_hunk.old_start), - incoming_hunk.old_lines, - ) { - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_at( - index, - vec![HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: net_lines + hunk.line_shift, - }], - 0, - ); - - self.track_commit_dependency(commit_id, vec![hunk.commit_id])?; - *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); - self.update_start_lines(i_first_hunk_to_shift, net_lines)?; - return Ok(()); - } - - // 2. The incoming hunk is contained in the intersecting hunk range. - if hunk.contains( - self.get_shifted_old_start(incoming_hunk.old_start), - incoming_hunk.old_lines, - ) { - let top_trimmed_hunk_lines = incoming_hunk - .new_start - .sub_or_err(hunk.start) - .context("When calculating the top lines of the hunk range being split.")?; - let bottom_trimmed_hunk_lines = self.calculate_lines_of_trimmed_hunk( - &hunk, - change_type, - incoming_hunk, - "When calculating the bottom lines of the hunk range being split.", - )?; - let total_trimmed_hunk_lines = top_trimmed_hunk_lines + bottom_trimmed_hunk_lines; - let amount_trimmed = hunk.lines.sub_or_err(total_trimmed_hunk_lines)?; - - // The hunk is trimmed in the middle, splitting it into a top and a - // bottom hunk. They now have fewer lines than before, so decrease their - // line shift. To compensate, increase the line shift of the incoming - // hunk by the amount of decrease. - // - // There is no definitive way to determine how much should be - // attributed to each section. Therefore, attribute based on the - // hunk line proportion. - let total_trimmed_hunk_line_shift = hunk.line_shift - i32::try_from(amount_trimmed)?; - let top_trimmed_hunk_line_shift = total_trimmed_hunk_line_shift - * i32::try_from(top_trimmed_hunk_lines)? - / i32::try_from(total_trimmed_hunk_lines)?; - let bottom_trimmed_hunk_line_shift = - total_trimmed_hunk_line_shift - top_trimmed_hunk_line_shift; - let incoming_hunk_line_shift = net_lines + i32::try_from(amount_trimmed)?; - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = self.replace_hunk_ranges_at( - index, - vec![ - HunkRange { - change_type: hunk.change_type, - stack_id: hunk.stack_id, - commit_id: hunk.commit_id, - start: hunk.start, - lines: top_trimmed_hunk_lines, - line_shift: top_trimmed_hunk_line_shift, - }, - HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: incoming_hunk_line_shift, - }, - HunkRange { - change_type: hunk.change_type, - stack_id: hunk.stack_id, - commit_id: hunk.commit_id, - start: incoming_hunk.new_start + incoming_hunk.new_lines, - lines: bottom_trimmed_hunk_lines, - line_shift: bottom_trimmed_hunk_line_shift, - }, - ], - 1, - ); - self.track_commit_dependency(commit_id, vec![hunk.commit_id])?; - *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); - self.update_start_lines(i_first_hunk_to_shift, net_lines)?; - return Ok(()); - } - - // 3. The incoming hunk partially overwrites the intersecting hunk range. - let (i_next_hunk_to_visit, i_first_hunk_to_shift) = - if self.get_shifted_old_start(incoming_hunk.old_start) <= hunk.start { - let bottom_trimmed_hunk_lines = self.calculate_lines_of_trimmed_hunk( - &hunk, - change_type, - incoming_hunk, - "When calculating the lines of the hunk range's beginning being trimmed.", - )?; - let bottom_amount_trimmed = hunk.lines.sub_or_err(bottom_trimmed_hunk_lines)?; - - // The hunk now has fewer lines than before, so decrease its line - // shift. To compensate, increase the line shift of the incoming - // hunk by the amount of decrease. - let bottom_trimmed_hunk_line_shift = - hunk.line_shift - i32::try_from(bottom_amount_trimmed)?; - let incoming_hunk_line_shift = net_lines + i32::try_from(bottom_amount_trimmed)?; - - // The incoming hunk overlaps the beginning of the intersecting hunk range. - self.replace_hunk_ranges_at( - index, - vec![ - HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: incoming_hunk_line_shift, - }, - HunkRange { - change_type: hunk.change_type, - stack_id: hunk.stack_id, - commit_id: hunk.commit_id, - start: incoming_hunk.new_start + incoming_hunk.new_lines, - lines: bottom_trimmed_hunk_lines, - line_shift: bottom_trimmed_hunk_line_shift, - }, - ], - 0, - ) - } else { - let top_trimmed_hunk_lines = incoming_hunk - .new_start - .sub_or_err(hunk.start) - .context("When calculating the lines of the hunk range's end being trimmed.")?; - let top_amount_trimmed = hunk.lines.sub_or_err(top_trimmed_hunk_lines)?; - - // The hunk now has fewer lines than before, so decrease its line - // shift. To compensate, increase the line shift of the incoming - // hunk by the amount of decrease. - let top_trimmed_hunk_line_shift = - hunk.line_shift - i32::try_from(top_amount_trimmed)?; - let incoming_hunk_line_shift = net_lines + i32::try_from(top_amount_trimmed)?; - - // The incoming hunk overlaps the end of the intersecting hunk range. - self.replace_hunk_ranges_at( - index, - vec![ - HunkRange { - change_type: hunk.change_type, - stack_id: hunk.stack_id, - commit_id: hunk.commit_id, - start: hunk.start, - lines: top_trimmed_hunk_lines, - line_shift: top_trimmed_hunk_line_shift, - }, - HunkRange { - change_type, - stack_id, - commit_id, - start: incoming_hunk.new_start, - lines: incoming_hunk.new_lines, - line_shift: incoming_hunk_line_shift, - }, - ], - 1, - ) + line_shift += incoming_hunk.net_lines()?; + } + for mut existing_hunk_range in existing_hunk_ranges_iter { + existing_hunk_range.start = { + let Some(start) = existing_hunk_range.start.checked_add_signed(line_shift) else { + bail!("when calculating existing_hunk_range.start") + }; + start }; - - self.track_commit_dependency(commit_id, vec![hunk.commit_id])?; - *index_next_hunk_to_visit = Some(i_next_hunk_to_visit); - self.update_start_lines(i_first_hunk_to_shift, net_lines)?; - - Ok(()) - } - - /// Calculate the number of lines of a hunk range that was trimmed from the top. - /// - /// Will handle the case where the incoming hunk is a modification and only adds or only deletes lines. - fn calculate_lines_of_trimmed_hunk( - &self, - hunk: &HunkRange, - change_type: TreeStatusKind, - incoming_hunk: &InputDiffHunk, - context: &'static str, - ) -> anyhow::Result { - let old_start = self.get_shifted_old_start(incoming_hunk.old_start); - let addition_shift = if self.is_addition_only_hunk(change_type, incoming_hunk) { - // If the incoming hunk is an addition, we need to subtract one more line. - 1 - } else { - 0 - }; - - let deletion_shift = if self.is_deletion_only_hunk(change_type, incoming_hunk) { - // If the incoming hunk is a deletion, we need to add one more line. - 1 - } else { - 0 - }; - - let result = hunk.start + hunk.lines; - let result = result.sub_or_err(old_start).context(context)?; - let result = result - .sub_or_err(incoming_hunk.old_lines) - .context(context)?; - let result = result.sub_or_err(addition_shift).context(context)?; - Ok(result + deletion_shift) - } - - /// Determine whether the incoming hunk is of modification type and only adds lines. - fn is_addition_only_hunk( - &self, - change_type: TreeStatusKind, - incoming_hunk: &InputDiffHunk, - ) -> bool { - let old_start = self.get_shifted_old_start(incoming_hunk.old_start); - change_type == TreeStatusKind::Modification - && (old_start + 1) == incoming_hunk.new_start - && incoming_hunk.old_lines == 0 - && incoming_hunk.new_lines > 0 - } - - /// Determine whether the incoming hunk is of modification type and only deletes lines. - fn is_deletion_only_hunk( - &self, - change_type: TreeStatusKind, - incoming_hunk: &InputDiffHunk, - ) -> bool { - let old_start = self.get_shifted_old_start(incoming_hunk.old_start); - change_type == TreeStatusKind::Modification - && old_start == (incoming_hunk.new_start + 1) - && incoming_hunk.old_lines > 0 - && incoming_hunk.new_lines == 0 - } - - fn track_commit_dependency( - &mut self, - commit_id: gix::ObjectId, - parent_ids: Vec, - ) -> anyhow::Result<()> { - for parent_id in parent_ids { - if commit_id == parent_id { - bail!("Commit ID cannot be a parent ID"); - } - self.commit_dependencies - .entry(commit_id) - .or_default() - .insert(parent_id); + self.hunk_ranges.push(existing_hunk_range); } Ok(()) } - - /// Shift the start lines of the hunk ranges starting at the given index. - fn update_start_lines( - &mut self, - index_of_first_hunk: usize, - line_shift: i32, - ) -> anyhow::Result<()> { - self.line_shift += line_shift; - - if index_of_first_hunk >= self.hunk_ranges.len() { - return Ok(()); - } - for hunk in &mut self.hunk_ranges[index_of_first_hunk..] { - let new_start = hunk.start.checked_add_signed(line_shift).ok_or_else(|| { - anyhow::anyhow!( - "Line shift overflow. Start: {} - Shift: {}", - hunk.start, - line_shift - ) - })?; - hunk.start = new_start; - } - Ok(()) - } - - /// Returns the shifted old start line number of an incoming hunk. - fn get_shifted_old_start(&self, old_start: u32) -> u32 { - // Everytime that we that an incoming hunk is added - // and it adds or subtracts lines, - // we need to shift the line numbers of the hunks that come after it. - - // This method allows us to compare the old start line number of the incoming hunk - // with the shifted start line number of the existing hunk ranges. - - old_start.checked_add_signed(self.line_shift).unwrap_or(0) - } - - /// Inserts the new hunks at the given index. - /// - /// Returns: - /// - The index of the next hunk after the last added hunk. - /// - The index of the next hunk after the hunk of interest. - fn insert_hunk_ranges_at( - &mut self, - index: usize, - hunks: Vec, - index_of_interest: usize, - ) -> (usize, usize) { - insert_hunk_ranges( - &mut self.hunk_ranges, - index, - index, - hunks, - index_of_interest, - ) - } - - /// Replaces the hunk at the given index with the new hunks. - /// - /// Returns: - /// - The index of the next hunk after the last added hunk. - /// - The index of the next hunk after the hunk of interest. - fn replace_hunk_ranges_at( - &mut self, - index: usize, - hunks: Vec, - index_of_interest: usize, - ) -> (usize, usize) { - insert_hunk_ranges( - &mut self.hunk_ranges, - index, - index + 1, - hunks, - index_of_interest, - ) - } - - /// Replaces the hunks between the given start and end indices with the new hunks. - /// - /// Returns: - /// - The index of the next hunk after the last added hunk. - /// - The index of the next hunk after the hunk of interest. - fn replace_hunk_ranges_between( - &mut self, - start: usize, - end: usize, - hunks: Vec, - index_of_interest: usize, - ) -> (usize, usize) { - insert_hunk_ranges(&mut self.hunk_ranges, start, end, hunks, index_of_interest) - } - - fn sum_line_shift(&self, start: usize, end: usize) -> i32 { - self.hunk_ranges[start..end] - .iter() - .fold(0, |acc, hunk_range| acc + hunk_range.line_shift) - } -} - -/// Update the hunk ranges by inserting the new hunks at the given start and end indices. -/// -/// Existing hunk ranges between the start and end indices are replaced by the new hunks. -/// Added hunk ranges that have 0 lines are ignored. -/// Returns: -/// - The index of the next hunk after the last added hunk. -/// - The index of the next hunk after the hunk of interest. -pub(crate) fn insert_hunk_ranges( - hunk_ranges: &mut Vec, - start: usize, - end: usize, - hunks: Vec, - index_of_interest: usize, -) -> (usize, usize) { - let mut new_hunks = vec![]; - new_hunks.extend_from_slice(&hunk_ranges[..start]); - - let mut index_after_last_added = start; - let mut index_after_interest = start; - for (i, hunk) in hunks.iter().enumerate() { - // if hunk.lines > 0 { - // } - // Only add hunk ranges that have lines. - new_hunks.push(*hunk); - index_after_last_added += 1; - - if i == index_of_interest { - index_after_interest = new_hunks.len(); - } - } - - if end < hunk_ranges.len() { - new_hunks.extend_from_slice(&hunk_ranges[end..]); - } - - *hunk_ranges = new_hunks; - - (index_after_interest, index_after_last_added) } diff --git a/crates/but-hunk-dependency/src/ranges/tests/mod.rs b/crates/but-hunk-dependency/src/ranges/tests/mod.rs index 9c792a2e16..5fb6afeaa5 100644 --- a/crates/but-hunk-dependency/src/ranges/tests/mod.rs +++ b/crates/but-hunk-dependency/src/ranges/tests/mod.rs @@ -3,7 +3,6 @@ use anyhow::{Context as _, anyhow}; use crate::InputDiffHunk; mod path; -mod path_utilities; mod workspace; /// Create a new object id by repeating the given `hex_char`. @@ -23,12 +22,24 @@ fn input_hunk_from_unified_diff(diff: &str) -> Result 0 && old_lines == 0 { + 1 + } else { + 0 + }, + old_lines: old_lines, + new_start: new_start + head_context_lines + - if context_lines > 0 && new_lines == 0 { + 1 + } else { + 0 + }, + new_lines: new_lines, }) } diff --git a/crates/but-hunk-dependency/src/ranges/tests/path.rs b/crates/but-hunk-dependency/src/ranges/tests/path.rs index 381c3a0faf..8d5143e5e2 100644 --- a/crates/but-hunk-dependency/src/ranges/tests/path.rs +++ b/crates/but-hunk-dependency/src/ranges/tests/path.rs @@ -1110,14 +1110,6 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { lines: 0, line_shift: 0 }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id, - commit_id: commit1_id, - start: 7, - lines: 0, - line_shift: 0 - } ] ); @@ -1161,14 +1153,6 @@ fn create_file_update_and_trim() -> anyhow::Result<()> { lines: 0, line_shift: 0 }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id, - commit_id: commit1_id, - start: 7, - lines: 0, - line_shift: 0 - } ] ); @@ -1451,7 +1435,7 @@ fn removing_line_updates_range() -> anyhow::Result<()> { stack_id, commit_id: commit1_id, start: 2, - lines: 0, + lines: 1, line_shift: 0 }, HunkRange { @@ -1466,8 +1450,8 @@ fn removing_line_updates_range() -> anyhow::Result<()> { change_type: TreeStatusKind::Modification, stack_id, commit_id: commit1_id, - start: 2, - lines: 2, + start: 3, + lines: 1, line_shift: 0 } ] @@ -1718,8 +1702,8 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { stack_id, commit_id: commit1_id, start: 7, - lines: 0, - line_shift: 0 + lines: 1, + line_shift: 1 }, HunkRange { change_type: TreeStatusKind::Modification, @@ -1733,9 +1717,9 @@ fn shift_is_correct_after_multiple_changes() -> anyhow::Result<()> { change_type: TreeStatusKind::Addition, stack_id, commit_id: commit1_id, - start: 7, - lines: 2, - line_shift: 2, + start: 8, + lines: 1, + line_shift: 1, }, HunkRange { change_type: TreeStatusKind::Modification, diff --git a/crates/but-hunk-dependency/src/ranges/tests/path_utilities.rs b/crates/but-hunk-dependency/src/ranges/tests/path_utilities.rs deleted file mode 100644 index 45d31c1f81..0000000000 --- a/crates/but-hunk-dependency/src/ranges/tests/path_utilities.rs +++ /dev/null @@ -1,957 +0,0 @@ -use but_core::{TreeStatusKind, ref_metadata::StackId}; - -use crate::{HunkRange, ranges::tests::id_from_hex_char}; - -#[test] -fn test_split_hunk_range() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 10, - line_shift: 9, - }]; - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 3, - line_shift: 9, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 4, - lines: 1, - line_shift: 0, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 5, - lines: 6, - line_shift: 9, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 0, 1, hunks, 1); - - assert_eq!(hunk_ranges.len(), 3); - assert_eq!(index_after_interest, 2); - assert_eq!(index_after_last_added, 3); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_b_id); - assert_eq!(hunk_ranges[2].commit_id, commit_a_id); - - Ok(()) -} - -#[test] -fn test_split_hunk_range_filter_before() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 10, - line_shift: 9, - }]; - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 0, - line_shift: 9, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 4, - lines: 1, - line_shift: 0, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 5, - lines: 6, - line_shift: 9, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 0, 1, hunks, 1); - - assert_eq!(hunk_ranges.len(), 3); - assert_eq!(index_after_interest, 2); - assert_eq!(index_after_last_added, 3); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_b_id); - assert_eq!(hunk_ranges[2].commit_id, commit_a_id); - - Ok(()) -} - -#[test] -fn test_split_hunk_range_filter_after() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 10, - line_shift: 9, - }]; - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 3, - line_shift: 9, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 4, - lines: 1, - line_shift: 0, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 5, - lines: 0, - line_shift: 9, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 6, - lines: 0, - line_shift: 9, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 0, 1, hunks, 1); - - assert_eq!(hunk_ranges.len(), 4); - assert_eq!(index_after_interest, 2); - assert_eq!(index_after_last_added, 4); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[0].start, 1); - assert_eq!(hunk_ranges[1].commit_id, commit_b_id); - assert_eq!(hunk_ranges[1].start, 4); - assert_eq!(hunk_ranges[2].commit_id, commit_a_id); - assert_eq!(hunk_ranges[2].start, 5); - assert_eq!(hunk_ranges[3].commit_id, commit_a_id); - assert_eq!(hunk_ranges[3].start, 6); - - Ok(()) -} - -#[test] -fn test_split_hunk_range_filter_incoming() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 10, - line_shift: 9, - }]; - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 4, - lines: 0, - line_shift: 0, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 3, - line_shift: 9, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 4, - lines: 0, - line_shift: 0, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 5, - lines: 6, - line_shift: 9, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 0, 1, hunks, 2); - - assert_eq!(hunk_ranges.len(), 4); - assert_eq!(index_after_interest, 3); - assert_eq!(index_after_last_added, 4); - assert_eq!(hunk_ranges[0].commit_id, commit_b_id); - assert_eq!(hunk_ranges[0].start, 4); - assert_eq!(hunk_ranges[1].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].start, 1); - assert_eq!(hunk_ranges[2].commit_id, commit_b_id); - assert_eq!(hunk_ranges[2].start, 4); - assert_eq!(hunk_ranges[3].commit_id, commit_a_id); - assert_eq!(hunk_ranges[3].start, 5); - - Ok(()) -} - -#[test] -fn test_split_hunk_range_filter_all() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 10, - line_shift: 9, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 11, - lines: 10, - line_shift: 9, - }, - ]; - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 4, - lines: 0, - line_shift: 0, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 0, - line_shift: 9, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 4, - lines: 0, - line_shift: 0, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 5, - lines: 0, - line_shift: 9, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 0, 1, hunks, 2); - - assert_eq!(hunk_ranges.len(), 5); - assert_eq!(index_after_interest, 3); - assert_eq!(index_after_last_added, 4); - assert_eq!(hunk_ranges[0].commit_id, commit_b_id); - assert_eq!(hunk_ranges[0].start, 4); - assert_eq!(hunk_ranges[1].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].start, 1); - assert_eq!(hunk_ranges[2].commit_id, commit_b_id); - assert_eq!(hunk_ranges[2].start, 4); - assert_eq!(hunk_ranges[3].commit_id, commit_a_id); - assert_eq!(hunk_ranges[3].start, 5); - assert_eq!(hunk_ranges[4].commit_id, commit_a_id); - assert_eq!(hunk_ranges[4].start, 11); - - Ok(()) -} - -#[test] -fn test_split_hunk_range_filter_only() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 10, - line_shift: 9, - }, - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 11, - lines: 10, - line_shift: 9, - }, - ]; - - let hunks = vec![HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 4, - lines: 0, - line_shift: 0, - }]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 2, 2, hunks, 0); - - assert_eq!(hunk_ranges.len(), 3); - assert_eq!(index_after_interest, 3); - assert_eq!(index_after_last_added, 3); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[0].start, 1); - assert_eq!(hunk_ranges[1].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].start, 11); - assert_eq!(hunk_ranges[2].commit_id, commit_b_id); - assert_eq!(hunk_ranges[2].start, 4); - - Ok(()) -} - -#[test] -fn test_replace_hunk_ranges_at_the_end() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: id_from_hex_char('1'), - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: id_from_hex_char('1'), - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let commit_c_id = id_from_hex_char('1'); - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 2, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 4, - lines: 1, - line_shift: 1, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 1, 5, hunks, 1); - - assert_eq!(hunk_ranges.len(), 3); - assert_eq!(index_after_interest, 3); - assert_eq!(index_after_last_added, 3); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_c_id); - assert_eq!(hunk_ranges[2].commit_id, commit_c_id); - - Ok(()) -} - -#[test] -fn test_replace_hunk_ranges_at_the_beginning() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let commit_c_id = id_from_hex_char('1'); - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 0, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 2, - lines: 1, - line_shift: 1, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 0, 2, hunks, 1); - - assert_eq!(hunk_ranges.len(), 3); - assert_eq!(index_after_interest, 2); - assert_eq!(index_after_last_added, 2); - assert_eq!(hunk_ranges[0].commit_id, commit_c_id); - assert_eq!(hunk_ranges[1].commit_id, commit_c_id); - assert_eq!(hunk_ranges[2].commit_id, commit_b_id); - - Ok(()) -} - -#[test] -fn test_insert_single_hunk_range_at_the_beginning() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - let commit_c_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let hunks = vec![HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 0, - lines: 1, - line_shift: 1, - }]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 0, 0, hunks, 0); - assert_eq!(hunk_ranges.len(), 4); - assert_eq!(index_after_interest, 1); - assert_eq!(index_after_last_added, 1); - assert_eq!(hunk_ranges[0].commit_id, commit_c_id); - assert_eq!(hunk_ranges[1].commit_id, commit_a_id); - assert_eq!(hunk_ranges[2].commit_id, commit_b_id); - assert_eq!(hunk_ranges[3].commit_id, commit_b_id); - - Ok(()) -} - -#[test] -fn test_insert_at_the_end() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - let commit_c_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let hunks = vec![HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 0, - lines: 1, - line_shift: 1, - }]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 3, 3, hunks, 0); - assert_eq!(hunk_ranges.len(), 4); - assert_eq!(index_after_interest, 4); - assert_eq!(index_after_last_added, 4); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_b_id); - assert_eq!(hunk_ranges[2].commit_id, commit_b_id); - assert_eq!(hunk_ranges[3].commit_id, commit_c_id); - - Ok(()) -} - -#[test] -fn test_replace_hunk_ranges_between_add() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - let commit_c_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 0, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 2, - lines: 1, - line_shift: 1, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 1, 2, hunks, 1); - assert_eq!(hunk_ranges.len(), 4); - assert_eq!(index_after_interest, 3); - assert_eq!(index_after_last_added, 3); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_c_id); - assert_eq!(hunk_ranges[2].commit_id, commit_c_id); - assert_eq!(hunk_ranges[3].commit_id, commit_b_id); - - Ok(()) -} - -#[test] -fn test_filter_out_single_range_replacement() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - let commit_c_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let hunks = vec![HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 4, - lines: 0, - line_shift: 1, - }]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 1, 2, hunks, 0); - assert_eq!(hunk_ranges.len(), 3); - assert_eq!(index_after_interest, 2); - assert_eq!(index_after_last_added, 2); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_c_id); - assert_eq!(hunk_ranges[2].commit_id, commit_b_id); - assert_eq!(hunk_ranges[2].start, 5); - - Ok(()) -} - -#[test] -fn test_filter_out_multiple_range_replacement() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - let commit_c_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 4, - lines: 0, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 5, - lines: 3, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 6, - lines: 0, - line_shift: 1, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 1, 2, hunks, 2); - assert_eq!(hunk_ranges.len(), 5); - assert_eq!(index_after_interest, 4); - assert_eq!(index_after_last_added, 4); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_c_id); - assert_eq!(hunk_ranges[1].start, 4); - assert_eq!(hunk_ranges[2].commit_id, commit_c_id); - assert_eq!(hunk_ranges[2].start, 5); - assert_eq!(hunk_ranges[3].commit_id, commit_c_id); - assert_eq!(hunk_ranges[3].start, 6); - assert_eq!(hunk_ranges[4].commit_id, commit_b_id); - assert_eq!(hunk_ranges[4].start, 5); - - Ok(()) -} - -#[test] -fn test_filter_out_single_range_insertion() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - let commit_c_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let hunks = vec![HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 4, - lines: 0, - line_shift: 1, - }]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 1, 1, hunks, 0); - assert_eq!(hunk_ranges.len(), 4); - assert_eq!(index_after_interest, 2); - assert_eq!(index_after_last_added, 2); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_c_id); - assert_eq!(hunk_ranges[2].commit_id, commit_b_id); - assert_eq!(hunk_ranges[2].start, 3); - assert_eq!(hunk_ranges[3].commit_id, commit_b_id); - assert_eq!(hunk_ranges[3].start, 5); - - Ok(()) -} - -#[test] -fn test_filter_out_multiple_range_insertion() -> anyhow::Result<()> { - let commit_a_id = id_from_hex_char('1'); - let commit_b_id = id_from_hex_char('1'); - let commit_c_id = id_from_hex_char('1'); - - let mut hunk_ranges = vec![ - HunkRange { - change_type: TreeStatusKind::Addition, - stack_id: StackId::generate(), - commit_id: commit_a_id, - start: 1, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 3, - lines: 1, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_b_id, - start: 5, - lines: 1, - line_shift: 1, - }, - ]; - - let hunks = vec![ - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 4, - lines: 0, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 5, - lines: 3, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 6, - lines: 0, - line_shift: 1, - }, - HunkRange { - change_type: TreeStatusKind::Modification, - stack_id: StackId::generate(), - commit_id: commit_c_id, - start: 8, - lines: 3, - line_shift: 1, - }, - ]; - - let (index_after_interest, index_after_last_added) = - crate::ranges::paths::insert_hunk_ranges(&mut hunk_ranges, 1, 1, hunks, 3); - assert_eq!(hunk_ranges.len(), 7); - assert_eq!(index_after_interest, 5); - assert_eq!(index_after_last_added, 5); - assert_eq!(hunk_ranges[0].commit_id, commit_a_id); - assert_eq!(hunk_ranges[1].commit_id, commit_c_id); - assert_eq!(hunk_ranges[1].start, 4); - assert_eq!(hunk_ranges[2].commit_id, commit_c_id); - assert_eq!(hunk_ranges[2].start, 5); - assert_eq!(hunk_ranges[3].commit_id, commit_c_id); - assert_eq!(hunk_ranges[3].start, 6); - assert_eq!(hunk_ranges[4].commit_id, commit_c_id); - assert_eq!(hunk_ranges[4].start, 8); - assert_eq!(hunk_ranges[5].commit_id, commit_b_id); - assert_eq!(hunk_ranges[5].start, 3); - assert_eq!(hunk_ranges[6].commit_id, commit_b_id); - assert_eq!(hunk_ranges[6].start, 5); - - Ok(()) -} diff --git a/crates/but-hunk-dependency/src/ranges/tests/workspace.rs b/crates/but-hunk-dependency/src/ranges/tests/workspace.rs index eb54b19839..5c1114229e 100644 --- a/crates/but-hunk-dependency/src/ranges/tests/workspace.rs +++ b/crates/but-hunk-dependency/src/ranges/tests/workspace.rs @@ -80,7 +80,7 @@ fn workspace_simple() -> anyhow::Result<()> { assert_eq!(dependencies_1[0].commit_id, commit1_id); assert_eq!(dependencies_1[0].stack_id, stack1_id); - let dependencies_2 = workspace_ranges.intersection(&path, 10, 1).unwrap(); + let dependencies_2 = workspace_ranges.intersection(&path, 9, 1).unwrap(); assert_eq!(dependencies_2.len(), 1); assert_eq!(dependencies_2[0].commit_id, commit2_id); assert_eq!(dependencies_2[0].stack_id, stack2_id); @@ -171,6 +171,7 @@ P5 }, ])?; + println!("{:?}", workspace_ranges); { // According to stack2, R1 is on line 6. Then, stack1 added 6 lines // before that, so R1 should now be on line 12. @@ -182,76 +183,3 @@ P5 Ok(()) } - -#[test] -fn gracefully_handle_invalid_input_commits() -> anyhow::Result<()> { - let path = BString::from("/test.txt"); - - let stack_id = StackId::generate(); - let commit_a_id = id_from_hex_char('a'); - let commit_b_id = id_from_hex_char('b'); - let commit_c_id = id_from_hex_char('c'); - - // Invalid input, two subsequent commits with the same changes. - let workspace_ranges = WorkspaceRanges::try_from_stacks(vec![InputStack { - stack_id, - commits_from_base_to_tip: vec![ - InputCommit { - commit_id: commit_a_id, // Delete file - files: vec![InputFile { - path: path.clone(), - change_type: TreeStatusKind::Deletion, - hunks: vec![InputDiffHunk { - old_start: 1, - old_lines: 2, - new_start: 0, - new_lines: 0, - }], - }], - }, - InputCommit { - commit_id: commit_b_id, // Delete file, again - files: vec![InputFile { - path: path.clone(), - change_type: TreeStatusKind::Deletion, - hunks: vec![InputDiffHunk { - old_start: 1, - old_lines: 2, - new_start: 0, - new_lines: 0, - }], - }], - }, - InputCommit { - commit_id: commit_c_id, // Re-add file - files: vec![InputFile { - path: path.clone(), - change_type: TreeStatusKind::Addition, - hunks: vec![InputDiffHunk { - old_start: 0, - old_lines: 0, - new_start: 1, - new_lines: 5, - }], - }], - }, - ], - }])?; - - let dependencies_1 = workspace_ranges.intersection(&path, 2, 1).unwrap(); - assert_eq!(dependencies_1.len(), 1); - assert_eq!(dependencies_1[0].commit_id, commit_c_id); - assert_eq!(dependencies_1[0].stack_id, stack_id); - - let errors = &workspace_ranges.errors; - assert_eq!(errors.len(), 1); - assert_eq!(errors[0].commit_id, commit_b_id); - assert_eq!(errors[0].stack_id, stack_id); - assert_eq!(errors[0].path, path); - assert_eq!( - errors[0].error_message, - "File recreation must be an addition" - ); - - Ok(()) -} diff --git a/crates/but-hunk-dependency/tests/hunk_dependency/main.rs b/crates/but-hunk-dependency/tests/hunk_dependency/main.rs index 529888a307..2105e714b8 100644 --- a/crates/but-hunk-dependency/tests/hunk_dependency/main.rs +++ b/crates/but-hunk-dependency/tests/hunk_dependency/main.rs @@ -23,7 +23,7 @@ fn intersect_workspace_ranges( ranges.intersection(&change.path, hunk.old_start, hunk.old_lines) { let hunk_ranges: Vec<_> = - hunk_ranges.into_iter().copied().map(Into::into).collect(); + hunk_ranges.into_iter().cloned().map(Into::into).collect(); intersections.push(HunkIntersection { hunk, commit_intersections: hunk_ranges, @@ -47,7 +47,7 @@ fn intersect_workspace_ranges( .map(|(path, ranges)| { ( path.to_owned(), - ranges.iter().map(|hr| (*hr).into()).collect(), + ranges.iter().map(|hr| hr.clone().into()).collect(), ) }) .collect(), diff --git a/crates/but-hunk-dependency/tests/hunk_dependency/ui.rs b/crates/but-hunk-dependency/tests/hunk_dependency/ui.rs index 73f56728b0..c518d9fa1c 100644 --- a/crates/but-hunk-dependency/tests/hunk_dependency/ui.rs +++ b/crates/but-hunk-dependency/tests/hunk_dependency/ui.rs @@ -34,10 +34,6 @@ fn hunk_dependencies_json_sample() -> anyhow::Result<()> { "diff": "@@ -7,2 +8,1 @@\n-update 7\n-update 8\n+aaaaa\n" }, [ - { - "stackId": "stack_1", - "commitId": "e954269ca7be71d09da50ec389b13f268a779c27" - }, { "stackId": "stack_1", "commitId": "fba21e9ecacde86f327537add23f96775064a486" @@ -58,6 +54,10 @@ fn hunk_dependencies_json_sample() -> anyhow::Result<()> { "diff": "@@ -10,1 +10,2 @@\n-added at the bottom\n+update bottom\n+add another line\n" }, [ + { + "stackId": "stack_1", + "commitId": "375e35becbf67fe2b246b120bc76bf070e3e41d8" + }, { "stackId": "stack_1", "commitId": "fba21e9ecacde86f327537add23f96775064a486" @@ -149,10 +149,6 @@ fn complex_file_manipulation_multiple_hunks_with_uncommitted_changes() -> anyhow +aaaaa "), [ - HunkLock { - stack_id: stack_1, - commit_id: Sha1(e954269ca7be71d09da50ec389b13f268a779c27), - }, HunkLock { stack_id: stack_1, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), @@ -171,6 +167,10 @@ fn complex_file_manipulation_multiple_hunks_with_uncommitted_changes() -> anyhow +add another line "), [ + HunkLock { + stack_id: stack_1, + commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), + }, HunkLock { stack_id: stack_1, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), diff --git a/crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs b/crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs index 8e696a9c4b..9932b64ec8 100644 --- a/crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs +++ b/crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs @@ -332,7 +332,7 @@ fn delete_and_recreate_file_multi_stack() -> anyhow::Result<()> { commit_id: Sha1(925d483a4608e3588d24d8c28c3b626c6d946a93), start: 1, lines: 1, - line_shift: 1, + line_shift: 0, }, ], ), @@ -370,7 +370,7 @@ fn complex_file_manipulation() -> anyhow::Result<()> { change_type: Addition, commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 1, - lines: 0, + lines: 1, line_shift: 0, }, StableHunkRange { @@ -383,9 +383,9 @@ fn complex_file_manipulation() -> anyhow::Result<()> { StableHunkRange { change_type: Addition, commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), - start: 3, - lines: 6, - line_shift: 6, + start: 4, + lines: 5, + line_shift: 5, }, StableHunkRange { change_type: Modification, @@ -394,13 +394,6 @@ fn complex_file_manipulation() -> anyhow::Result<()> { lines: 3, line_shift: 3, }, - StableHunkRange { - change_type: Addition, - commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), - start: 12, - lines: 0, - line_shift: 0, - }, ], ), ( @@ -459,7 +452,7 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { change_type: Addition, commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), start: 1, - lines: 0, + lines: 1, line_shift: 0, }, StableHunkRange { @@ -472,9 +465,9 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { StableHunkRange { change_type: Addition, commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), - start: 3, - lines: 6, - line_shift: 6, + start: 4, + lines: 5, + line_shift: 5, }, StableHunkRange { change_type: Modification, @@ -483,13 +476,6 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { lines: 3, line_shift: 3, }, - StableHunkRange { - change_type: Addition, - commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), - start: 12, - lines: 0, - line_shift: 0, - }, ], ), ( @@ -551,9 +537,9 @@ fn complex_file_manipulation_with_uncommitted_changes() -> anyhow::Result<()> { StableHunkRange { change_type: Addition, commit_id: Sha1(3113ca08b2b23d7646cdb9c9b88ffde8d1c7784a), - start: 3, - lines: 6, - line_shift: 6, + start: 4, + lines: 5, + line_shift: 5, }, ], }, @@ -600,7 +586,7 @@ fn complex_file_manipulation_multiple_hunks() -> anyhow::Result<()> { change_type: Modification, commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), start: 1, - lines: 0, + lines: 1, line_shift: 0, }, StableHunkRange { @@ -613,8 +599,8 @@ fn complex_file_manipulation_multiple_hunks() -> anyhow::Result<()> { StableHunkRange { change_type: Modification, commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), - start: 2, - lines: 2, + start: 3, + lines: 1, line_shift: 1, }, StableHunkRange { @@ -638,13 +624,6 @@ fn complex_file_manipulation_multiple_hunks() -> anyhow::Result<()> { lines: 1, line_shift: 1, }, - StableHunkRange { - change_type: Modification, - commit_id: Sha1(e954269ca7be71d09da50ec389b13f268a779c27), - start: 7, - lines: 0, - line_shift: 0, - }, StableHunkRange { change_type: Modification, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), @@ -663,15 +642,15 @@ fn complex_file_manipulation_multiple_hunks() -> anyhow::Result<()> { change_type: Modification, commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), start: 9, - lines: 1, - line_shift: 1, + lines: 2, + line_shift: 2, }, StableHunkRange { change_type: Modification, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), start: 10, lines: 1, - line_shift: 2, + line_shift: 1, }, ], ), @@ -705,8 +684,8 @@ fn complex_file_manipulation_multiple_hunks_with_uncommitted_changes() -> anyhow StableHunkRange { change_type: Modification, commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), - start: 2, - lines: 2, + start: 3, + lines: 1, line_shift: 1, }, ], @@ -718,13 +697,6 @@ fn complex_file_manipulation_multiple_hunks_with_uncommitted_changes() -> anyhow +aaaaa "), commit_intersections: [ - StableHunkRange { - change_type: Modification, - commit_id: Sha1(e954269ca7be71d09da50ec389b13f268a779c27), - start: 7, - lines: 0, - line_shift: 0, - }, StableHunkRange { change_type: Modification, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), @@ -748,12 +720,19 @@ fn complex_file_manipulation_multiple_hunks_with_uncommitted_changes() -> anyhow +add another line "), commit_intersections: [ + StableHunkRange { + change_type: Modification, + commit_id: Sha1(375e35becbf67fe2b246b120bc76bf070e3e41d8), + start: 9, + lines: 2, + line_shift: 2, + }, StableHunkRange { change_type: Modification, commit_id: Sha1(fba21e9ecacde86f327537add23f96775064a486), start: 10, lines: 1, - line_shift: 2, + line_shift: 1, }, ], },