From 23dc781908541cd9971785818dc5bde4f5fb71f7 Mon Sep 17 00:00:00 2001 From: claw Date: Tue, 12 May 2026 23:13:28 -0700 Subject: [PATCH] fix(gitea): map hunk-header positions in BuildPositionToLineMap BuildPositionToLineMap incremented position and updated maxPositions for @@ hunk-header lines but did not store a map entry, causing Translate() to return a hard error for any comment positioned at a hunk header. Store sentinel value 0 for hunk-header positions (analogous to -1 for deletions) and extend Translate() to fall through to the nearest context/addition line below, matching the existing deletion-line behavior. Fixes #97 --- gitea/position.go | 15 ++++++-- gitea/position_test.go | 85 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/gitea/position.go b/gitea/position.go index e1aa4be..e393c48 100644 --- a/gitea/position.go +++ b/gitea/position.go @@ -11,6 +11,7 @@ import ( type PositionMap struct { // files maps filename → (position → new-file line number). // Deletion lines are mapped to -1 (no new-file line). + // Hunk-header lines are mapped to 0 (no new-file line). files map[string]map[int]int // maxPositions caches the highest position number per file, // tracked during construction to avoid O(n) scans at translate time. @@ -19,8 +20,8 @@ type PositionMap struct { // Translate converts a GitHub diff-position to a new-file line number for a given file. // Returns an error if the file is not in the diff or the position is out of range. -// If the position targets a deletion line, it maps to the nearest non-deletion line below; -// if no such line exists, returns an error. +// If the position targets a deletion or hunk-header line, it maps to the nearest +// context/addition line below; if no such line exists, returns an error. func (pm *PositionMap) Translate(file string, position int) (int, error) { if pm == nil || pm.files == nil { return 0, fmt.Errorf("empty position map") @@ -41,14 +42,18 @@ func (pm *PositionMap) Translate(file string, position int) (int, error) { } // lineNum == -1 means this position is a deletion line. - // Map to the nearest non-deletion line below. - if lineNum == -1 { + // lineNum == 0 means this position is a hunk-header line. + // Both map to the nearest context/addition line below. + if lineNum <= 0 { maxPos := pm.maxPosition(file) for p := position + 1; p <= maxPos; p++ { if ln, exists := fileMap[p]; exists && ln > 0 { return ln, nil } } + if lineNum == 0 { + return 0, fmt.Errorf("position %d targets a hunk-header line with no subsequent new-file line in %q", position, file) + } return 0, fmt.Errorf("position %d targets a deletion line with no subsequent new-file line in %q", position, file) } @@ -70,6 +75,7 @@ func (pm *PositionMap) maxPosition(file string) int { // - A new @@ hunk within the same file continues incrementing (does not reset) // - Position maps to the new file line number for additions and context lines // - Deletion lines have a position but no new-file line number (stored as -1) +// - Hunk-header lines have a position but no new-file line number (stored as 0) func BuildPositionToLineMap(diff string) *PositionMap { pm := &PositionMap{ files: make(map[string]map[int]int), @@ -126,6 +132,7 @@ func BuildPositionToLineMap(diff string) *PositionMap { // Parse hunk headers if strings.HasPrefix(line, "@@") && currentFile != "" { position++ + pm.files[currentFile][position] = 0 // sentinel: hunk-header has no new-file line pm.maxPositions[currentFile] = position newLine = parseHunkStart(line) continue diff --git a/gitea/position_test.go b/gitea/position_test.go index 743a2b0..bb15042 100644 --- a/gitea/position_test.go +++ b/gitea/position_test.go @@ -272,3 +272,88 @@ diff --git a/b.go b/b.go t.Errorf("Translate(b.go, 3) = %d, want 2", got) } } + +func TestTranslate_HunkHeaderPosition_SingleHunk(t *testing.T) { + // Position 1 is the @@ hunk-header line. + // It should resolve to the first context/addition line below (new line 16). + diff := `diff --git a/file.go b/file.go +index abc..def 100644 +--- a/file.go ++++ b/file.go +@@ -16,4 +16,5 @@ func example() { + context line +-deleted line ++added line + context after +` + pm := BuildPositionToLineMap(diff) + + got, err := pm.Translate("file.go", 1) + if err != nil { + t.Fatalf("Translate(file.go, 1): unexpected error: %v", err) + } + if got != 16 { + t.Errorf("Translate(file.go, 1) = %d, want 16 (first context/addition line in hunk)", got) + } +} + +func TestTranslate_HunkHeaderPosition_MultiHunk(t *testing.T) { + // First hunk: @@ is pos 1, then " line1" (pos 2), "-old" (pos 3), "+new" (pos 4) + // Second hunk: @@ is pos 5, then " func foo() {" (pos 6), "+// added" (pos 7), etc. + // Translating position 5 (second @@) should resolve to new line 10. + diff := `diff --git a/file.go b/file.go +--- a/file.go ++++ b/file.go +@@ -1,3 +1,3 @@ package main + line1 +-old ++new +@@ -10,3 +10,4 @@ func foo() { + func foo() { ++ // added + return + } +` + pm := BuildPositionToLineMap(diff) + + // Position 5 is the second @@ hunk-header — should resolve to new line 10 + got, err := pm.Translate("file.go", 5) + if err != nil { + t.Fatalf("Translate(file.go, 5): unexpected error: %v", err) + } + if got != 10 { + t.Errorf("Translate(file.go, 5) = %d, want 10 (first context/addition line in second hunk)", got) + } + + // Also verify first hunk header at position 1 resolves to new line 1 + got, err = pm.Translate("file.go", 1) + if err != nil { + t.Fatalf("Translate(file.go, 1): unexpected error: %v", err) + } + if got != 1 { + t.Errorf("Translate(file.go, 1) = %d, want 1 (first context/addition line in first hunk)", got) + } +} + +func TestTranslate_HunkHeaderPosition_NewFile(t *testing.T) { + // New file: @@ -0,0 +1,3 @@ is position 1. + // Should resolve to new line 1 (the first addition). + diff := `diff --git a/new.go b/new.go +new file mode 100644 +--- /dev/null ++++ b/new.go +@@ -0,0 +1,3 @@ ++package main ++ ++func init() {} +` + pm := BuildPositionToLineMap(diff) + + got, err := pm.Translate("new.go", 1) + if err != nil { + t.Fatalf("Translate(new.go, 1): unexpected error: %v", err) + } + if got != 1 { + t.Errorf("Translate(new.go, 1) = %d, want 1 (first addition line)", got) + } +}