Fix panic in matched_lines_indexes_for_hunk#944
Open
gthb wants to merge 6 commits intoWilfred:masterfrom
Open
Conversation
Add repro files from issue Wilfred#770 into `sample_files`. Add test case `test_display_hunks_out_of_order_issue_770` which reproduces the panic in a more targeted way. (Add `test_util.rs` with helpers for succinctly creating MatchedPos values)
Remove the slicing optimization in side_by_side.rs that assumed hunks appear in matched_lines order. It seems the rationale "We iterate through hunks in order, so we know the next hunk must appear after start_i" doesn't hold (maybe because something changed since this optimization landed?). When there are interleaved LHS-only and RHS-only novel sections, hunks from matched_novel_lines may not be strictly ordered by their position in matched_lines. Fixes Wilfred#770 where the slicing would remove lines that later hunks still needed to display, leading to a panic.
These Nix files trigger a panic in matched_lines_indexes_for_hunk Minimized from the files in Wilfred#770 (comment)
In matched_novel_lines, when a novel line on one side picks an opposite line using next_opposite, also update the opposite side's highest tracker. This prevents the same line from being added again when that side is processed, which was the source of duplicate entries that caused panics downstream. Also fix an apparent bug in the RHS arm: when looking up the opposite position for RHS novel lines, the filter should use highest_lhs, not highest_rhs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fix the panic in #770 where the assertion "Hunk lines should be present in matched lines" failed.
How
In matched_novel_lines, when a novel line on one side picks an opposite line using
next_opposite, also update the opposite side's highest tracker. This prevents the same line from being added again when that side is processed, which was the source of duplicate entries that caused panics downstream.Fix an apparent bug in the RHS arm: when looking up the opposite position for RHS novel lines, the filter should use highest_lhs, not highest_rhs.
Remove the slicing optimization in
side_by_side::printthat assumed hunks appear inmatched_linesorder. Looks like the rationale:does not (currently) hold (maybe because something changed since this optimization landed?): when there are interleaved LHS-only and RHS-only novel sections, hunks from
matched_novel_linesmay not be strictly ordered by their position inmatched_lines. So the slicing would remove lines that later hunks still needed to display, leading to that panic.