diff --git a/go.mod b/go.mod index c85fc93d..0961edf6 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( charm.land/bubbletea/v2 v2.0.6 charm.land/lipgloss/v2 v2.0.3 github.com/alecthomas/kong v1.15.0 + github.com/bluekeyes/go-gitdiff v0.9.0 github.com/buildkite/shellwords v1.0.1 github.com/charmbracelet/colorprofile v0.4.3 github.com/charmbracelet/x/ansi v0.11.7 diff --git a/go.sum b/go.sum index ec549a5c..4386b803 100644 --- a/go.sum +++ b/go.sum @@ -16,6 +16,8 @@ github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiE github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ12Gv5o= github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= +github.com/bluekeyes/go-gitdiff v0.9.0 h1:w+O6lkRBOqfGcwF0Lf6FFHQrhmxM0hCJW5+rbilGuSs= +github.com/bluekeyes/go-gitdiff v0.9.0/go.mod h1:WWAk1Mc6EgWarCrPFO+xeYlujPu98VuLW3Tu+B/85AE= github.com/buildkite/shellwords v1.0.1 h1:88OjMbEBf+EliVB0tizXJynpAM2CKOvYwepg5n8O70M= github.com/buildkite/shellwords v1.0.1/go.mod h1:so0eQnTxgbo58CTYX+4BCx5UuMzvRha9dcKdCKl6NV4= github.com/charmbracelet/colorprofile v0.4.3 h1:QPa1IWkYI+AOB+fE+mg/5/4HRMZcaXex9t5KX76i20Q= diff --git a/internal/reviewdiff/patch.go b/internal/reviewdiff/patch.go new file mode 100644 index 00000000..1c6b2bde --- /dev/null +++ b/internal/reviewdiff/patch.go @@ -0,0 +1,145 @@ +// Package reviewdiff answers review-comment questions about a Git patch. +package reviewdiff + +import ( + "bytes" + "fmt" + + "github.com/bluekeyes/go-gitdiff/gitdiff" +) + +// Patch describes the files and line ranges represented by a Git patch. +// +// Postimage queries use destination paths and line numbers. Deletion queries +// use source paths and line numbers because those lines no longer exist in the +// postimage. +type Patch struct { + files map[string][]lineRange + deletions map[string][]lineRange +} + +// Parse parses a Git patch for review-comment queries. +func Parse(src []byte) (*Patch, error) { + files, _, err := gitdiff.Parse(bytes.NewReader(src)) + if err != nil { + return nil, fmt.Errorf("parse Git patch: %w", err) + } + + patch := &Patch{ + files: make(map[string][]lineRange), + deletions: make(map[string][]lineRange), + } + for _, file := range files { + // A destination path is enough to make a file commentable, including + // binary, rename-only, and mode-only changes without text fragments. + if file.NewName != "" { + if _, ok := patch.files[file.NewName]; !ok { + patch.files[file.NewName] = nil + } + } + + for _, fragment := range file.TextFragments { + if file.NewName != "" && fragment.NewLines > 0 { + patch.files[file.NewName] = append( + patch.files[file.NewName], + lineRange{ + start: fragment.NewPosition, + end: fragment.NewPosition + fragment.NewLines - 1, + }, + ) + } + + // Fragment ranges include context. Walk the old-side cursor so + // deletion queries distinguish removed lines from nearby context. + oldLine := fragment.OldPosition + for _, line := range fragment.Lines { + switch line.Op { + case gitdiff.OpContext: + oldLine++ + case gitdiff.OpDelete: + if file.OldName != "" { + patch.deletions[file.OldName] = appendLine( + patch.deletions[file.OldName], + oldLine, + ) + } + oldLine++ + case gitdiff.OpAdd: + } + } + } + } + + return patch, nil +} + +// ContainsFile reports whether path exists in the patch postimage. +func (p *Patch) ContainsFile(path string) bool { + _, ok := p.files[path] + return ok +} + +// ContainsLine reports whether the postimage line is in a patch fragment. +func (p *Patch) ContainsLine(path string, line int) bool { + return p.ContainsLineRange(path, line, line) +} + +// ContainsLineRange reports whether every line in the inclusive postimage +// range is in one patch fragment. +func (p *Patch) ContainsLineRange(path string, start, end int) bool { + if start <= 0 || end < start { + return false + } + + for _, fragment := range p.files[path] { + if fragment.contains(int64(start), int64(end)) { + return true + } + } + return false +} + +// DeletesLine reports whether the patch deletes the source line. +func (p *Patch) DeletesLine(path string, line int) bool { + return p.DeletesLineRange(path, line, line) +} + +// DeletesLineRange reports whether the patch deletes any source line in the +// inclusive range. +func (p *Patch) DeletesLineRange(path string, start, end int) bool { + if start <= 0 || end < start { + return false + } + + for _, deletion := range p.deletions[path] { + if deletion.overlaps(int64(start), int64(end)) { + return true + } + } + return false +} + +// lineRange is an inclusive interval in one side of a file diff. +type lineRange struct { + start int64 + end int64 +} + +func (r lineRange) contains(start, end int64) bool { + return r.start <= start && end <= r.end +} + +func (r lineRange) overlaps(start, end int64) bool { + return r.start <= end && start <= r.end +} + +// appendLine adds a deleted line while coalescing consecutive lines into a +// compact range. +func appendLine(ranges []lineRange, line int64) []lineRange { + if len(ranges) == 0 || ranges[len(ranges)-1].end+1 != line { + return append(ranges, lineRange{start: line, end: line}) + } + + ranges[len(ranges)-1].end = line + return ranges +} diff --git a/internal/reviewdiff/patch_test.go b/internal/reviewdiff/patch_test.go new file mode 100644 index 00000000..6c237e47 --- /dev/null +++ b/internal/reviewdiff/patch_test.go @@ -0,0 +1,103 @@ +package reviewdiff_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.abhg.dev/gs/internal/reviewdiff" +) + +func TestPatchContains(t *testing.T) { + patch, err := reviewdiff.Parse([]byte(`diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -1,4 +1,5 @@ + package main + ++import "fmt" + func main() {} + +@@ -10,3 +11,4 @@ func helper() { + work() ++ fmt.Println("done") + } + +diff --git "a/with space.go" "b/with space.go" +--- "a/with space.go" ++++ "b/with space.go" +@@ -1 +1,2 @@ + package spaced ++var Added = true + +diff --git a/old.go b/new.go +similarity index 100% +rename from old.go +rename to new.go + +diff --git a/script.sh b/script.sh +old mode 100644 +new mode 100755 +`)) + require.NoError(t, err) + + assert.True(t, patch.ContainsFile("main.go")) + assert.True(t, patch.ContainsFile("with space.go")) + assert.True(t, patch.ContainsFile("new.go")) + assert.True(t, patch.ContainsFile("script.sh")) + assert.False(t, patch.ContainsFile("old.go")) + assert.False(t, patch.ContainsFile("missing.go")) + + assert.True(t, patch.ContainsLine("main.go", 3)) + assert.True(t, patch.ContainsLine("main.go", 13)) + assert.False(t, patch.ContainsLine("main.go", 8)) + assert.False(t, patch.ContainsLine("main.go", 0)) + assert.True(t, patch.ContainsLine("with space.go", 2)) + + assert.True(t, patch.ContainsLineRange("main.go", 1, 5)) + assert.True(t, patch.ContainsLineRange("main.go", 11, 14)) + assert.False(t, patch.ContainsLineRange("main.go", 5, 11)) + assert.False(t, patch.ContainsLineRange("main.go", 4, 2)) +} + +func TestPatchDeletes(t *testing.T) { + patch, err := reviewdiff.Parse([]byte(`diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -2,5 +2,4 @@ package main + context +-delete one +-delete two ++replacement + more context + last context +diff --git a/old.go b/new.go +similarity index 80% +rename from old.go +rename to new.go +@@ -10,3 +10,2 @@ + keep +-removed + keep too +`)) + require.NoError(t, err) + + assert.True(t, patch.DeletesLine("main.go", 3)) + assert.True(t, patch.DeletesLine("main.go", 4)) + assert.False(t, patch.DeletesLine("main.go", 2)) + assert.False(t, patch.DeletesLine("main.go", 5)) + assert.True(t, patch.DeletesLineRange("main.go", 1, 3)) + assert.False(t, patch.DeletesLineRange("main.go", 5, 7)) + assert.False(t, patch.DeletesLineRange("main.go", 0, 4)) + + assert.True(t, patch.DeletesLine("old.go", 11)) + assert.False(t, patch.DeletesLine("new.go", 11)) +} + +func TestParseError(t *testing.T) { + _, err := reviewdiff.Parse([]byte(`detached fragment +@@ -1 +1 @@ +`)) + require.Error(t, err) + assert.ErrorContains(t, err, "parse Git patch") +}