fix: handle single-line hunks and no-newline markers in diff parser
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 55s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m42s
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 55s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m42s
- Hunk headers without comma ("@@ -1 +1 @@") now parse correctly by
splitting on comma OR space instead of comma only
- Explicit skip for "\ No newline at end of file" lines (was already
safe but now documents intent)
- Tests added for both edge cases (TDD: tests written first, confirmed
failure, then fixed)
Addresses sonnet findings #1 and #2 from PR #26 review.
This commit is contained in:
+12
-3
@@ -44,12 +44,16 @@ func ParseDiffNewLines(diff string) *DiffLineRanges {
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
// Parse hunk header: @@ -old,count +new,count @@
|
// Parse hunk header: @@ -old,count +new,count @@ or @@ -old +new @@
|
||||||
if strings.HasPrefix(line, "@@") && currentFile != "" {
|
if strings.HasPrefix(line, "@@") && currentFile != "" {
|
||||||
// Extract the +N part
|
// Extract the +N part — handle both "+10,8" and "+1" forms
|
||||||
parts := strings.Split(line, "+")
|
parts := strings.Split(line, "+")
|
||||||
if len(parts) >= 2 {
|
if len(parts) >= 2 {
|
||||||
numStr := strings.Split(parts[1], ",")[0]
|
// Take everything before comma or space
|
||||||
|
numStr := parts[1]
|
||||||
|
if idx := strings.IndexAny(numStr, ", "); idx != -1 {
|
||||||
|
numStr = numStr[:idx]
|
||||||
|
}
|
||||||
n, err := strconv.Atoi(numStr)
|
n, err := strconv.Atoi(numStr)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
newLine = n
|
newLine = n
|
||||||
@@ -62,6 +66,11 @@ func ParseDiffNewLines(diff string) *DiffLineRanges {
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Skip diff metadata lines
|
||||||
|
if strings.HasPrefix(line, "\\") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
// Count lines in hunk
|
// Count lines in hunk
|
||||||
if strings.HasPrefix(line, "+") || strings.HasPrefix(line, " ") {
|
if strings.HasPrefix(line, "+") || strings.HasPrefix(line, " ") {
|
||||||
result.files[currentFile][newLine] = true
|
result.files[currentFile][newLine] = true
|
||||||
|
|||||||
@@ -73,3 +73,43 @@ func TestParseDiffNewLines_Empty(t *testing.T) {
|
|||||||
t.Error("empty diff should contain nothing")
|
t.Error("empty diff should contain nothing")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestParseDiffNewLines_NoCommaHunk(t *testing.T) {
|
||||||
|
// Single-line hunks omit the comma: @@ -1 +1 @@
|
||||||
|
diff := `diff --git a/single.go b/single.go
|
||||||
|
--- a/single.go
|
||||||
|
+++ b/single.go
|
||||||
|
@@ -1 +1 @@
|
||||||
|
-old line
|
||||||
|
+new line
|
||||||
|
`
|
||||||
|
ranges := ParseDiffNewLines(diff)
|
||||||
|
if !ranges.Contains("single.go", 1) {
|
||||||
|
t.Error("expected single.go:1 to be in diff (no-comma hunk)")
|
||||||
|
}
|
||||||
|
if ranges.Contains("single.go", 2) {
|
||||||
|
t.Error("single.go:2 should NOT be in diff")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseDiffNewLines_NoNewlineMarker(t *testing.T) {
|
||||||
|
// "\ No newline at end of file" should not advance line counter
|
||||||
|
diff := `diff --git a/noeof.go b/noeof.go
|
||||||
|
--- a/noeof.go
|
||||||
|
+++ b/noeof.go
|
||||||
|
@@ -1,2 +1,2 @@
|
||||||
|
+line one
|
||||||
|
+line two
|
||||||
|
\ No newline at end of file
|
||||||
|
`
|
||||||
|
ranges := ParseDiffNewLines(diff)
|
||||||
|
if !ranges.Contains("noeof.go", 1) {
|
||||||
|
t.Error("expected noeof.go:1")
|
||||||
|
}
|
||||||
|
if !ranges.Contains("noeof.go", 2) {
|
||||||
|
t.Error("expected noeof.go:2")
|
||||||
|
}
|
||||||
|
if ranges.Contains("noeof.go", 3) {
|
||||||
|
t.Error("noeof.go:3 should NOT be in diff (no-newline marker)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user