diff: simplify line-range filter by classifying removals immediately#2099
Open
mmontalbo wants to merge 1 commit intogitgitgadget:masterfrom
Open
diff: simplify line-range filter by classifying removals immediately#2099mmontalbo wants to merge 1 commit intogitgitgadget:masterfrom
mmontalbo wants to merge 1 commit intogitgitgadget:masterfrom
Conversation
The line-range filter buffered '-' (removal) lines in a pending_rm strbuf, deferring their classification until a '+' or ' ' line arrived to reveal the post-image position. This buffering is unnecessary. Removal lines are pre-image content that occupies no post-image space, so they do not advance lno_post. Within a hunk, xdiff emits removals before additions for each change, so a '-' line always arrives while lno_post is at the same position that the following '+' or ' ' line will occupy. Each line can therefore be classified against the tracked ranges as it arrives, without waiting for a non-removal line to confirm the position. The buffering also had a bug: flush_rhunk() unconditionally drained the pending buffer when the range hunk was active, even if lno_post had advanced past the tracked range. This caused deletions immediately after the tracked function to be incorrectly included in patch output. Remove the pending_rm buffer and classify '-' lines using the same in_range check applied to '+' and ' ' lines. This simplifies the filter, removes three struct fields and a helper function, and makes the flush_rhunk() bug impossible by construction. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
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.
The line-range filter buffered '-' (removal) lines in a
pending_rm strbuf, deferring their classification until a '+'
or ' ' line arrived to reveal the post-image position.
This buffering is unnecessary. Removal lines are pre-image
content that occupies no post-image space, so they do not
advance lno_post. Within a hunk, xdiff emits removals before
additions for each change, so a '-' line always arrives while
lno_post is at the same position that the following '+' or ' '
line will occupy. Each line can therefore be classified against
the tracked ranges as it arrives, without waiting for a
non-removal line to confirm the position.
The buffering also had a bug: flush_rhunk() unconditionally
drained the pending buffer when the range hunk was active, even
if lno_post had advanced past the tracked range. This caused
deletions immediately after the tracked function to be
incorrectly included in patch output.
Remove the pending_rm buffer and classify '-' lines using the
same in_range check applied to '+' and ' ' lines. This
simplifies the filter, removes three struct fields and a helper
function, and makes the flush_rhunk() bug impossible by
construction.