fix: hunk header counts and the deleted-line check - #580
Merged
jakubbortlik merged 2 commits intoAug 3, 2026
Conversation
Commenting on the unchanged line directly above a deletion of exactly one line sent it as an added line, anchored to the new side, and without the warning that comments on unmodified lines are placed in the old file. It now goes to the old file with both line numbers, the way it already did above a deletion of two or more lines.
The check implemented the general algorithm for arbitrary diffs, including context lines, and its range carried an off-by-one that no caller could reach. The only diff it ever sees comes from git.diff_files, which runs with --unified=0 and has no context lines, so a line is removed exactly when it falls in the hunk's old range. Simplify to that rather than fix the bound.
3 tasks
Collaborator
|
Hi Sebastian! Thanks for opening the PR. I'm aware of hunk header parsing bug and the same fix is part of my WIP. Regarding the modification type detection, I'm playing with the idea of actually checking the diff with a three line context ( Thank you especially for the tests! |
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
2 participants
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.
Git omits the
,Nin a hunk header when N is 1, so@@ -5 +4,0 @@is a one-line deletion.parse_possible_hunk_headersreads the empty capture as 0, which makes that header look like@@ -5,0 +4,0 @@:is_range_zeroshort-circuitsget_modification_type_from_new_sha, and the unmodified line above the deletion comes back asadded. The same deletion over two lines classifies correctly. The first commit reads the empty capture as 1.Found this while working on #577.
While I was in there I replaced the body walk in
line_was_removedwith a range check: with--unified=0a hunk has no context lines, so the old range already answers the question. No behaviour change. The old side had no tests, so that commit adds some.Putting
get_modification_typeunder test properly is a different matter; I will write that up in #386. Related to that issue, not a fix for it.