Skip to content

Fix hunk position counting to match GitHub's convention#71

Open
C-Hipple wants to merge 1 commit intomainfrom
claude/fix-local-comment-display-SYZph
Open

Fix hunk position counting to match GitHub's convention#71
C-Hipple wants to merge 1 commit intomainfrom
claude/fix-local-comment-display-SYZph

Conversation

@C-Hipple
Copy link
Owner

@C-Hipple C-Hipple commented Mar 5, 2026

Summary

Fix the hunk position counting logic in two locations to properly handle subsequent hunk headers according to GitHub's position convention. Previously, only the first hunk was being counted, which caused incorrect position tracking for diffs with multiple hunks.

Changes

  • Modified hunk header handling in the first location (line ~962) to count subsequent hunk headers by incrementing current-position instead of only initializing it on the first hunk
  • Added similar logic in the second location (line ~1592) to track whether the first hunk has been counted and increment the count variable for subsequent hunks
  • Added first-hunk-counted variable to properly distinguish between the first and subsequent hunks in the second code path

Test Plan

  • Verify diff parsing with multiple hunks produces correct position values
  • Test with single-hunk diffs to ensure no regression
  • Verify GitHub position convention is correctly matched for multi-hunk diffs

https://claude.ai/code/session_01EmShngn7vgu9UCag1xDKd4

crs--get-comment-context and crs--find-position-in-diff were not
counting subsequent @@ hunk headers as positions, while
crs--insert-comments-into-buffer and crs--render-diff (and GitHub's
own position convention) do count them. This caused local comments
placed in the 2nd+ hunk of a file to have a position offset, so
they were never matched during re-rendering and appeared invisible.

https://claude.ai/code/session_01EmShngn7vgu9UCag1xDKd4
Copy link
Owner Author

@C-Hipple C-Hipple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants