fix(gitea): map hunk-header positions in BuildPositionToLineMap
CI / test (pull_request) Successful in 28s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 27s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 44s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m49s
CI / test (pull_request) Successful in 28s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 27s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 44s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m49s
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
This commit is contained in:
+11
-4
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user