chore: diff parsing prefers different order of Line/OriginalLine (#90)

frontend/components/DiffBlock.vue

@@ -32,18 +32,18 @@ onMounted(() => {
   Prism.highlightElement(code.value);
 });
 
-const props = defineProps({ 
-  comment: { 
+const props = defineProps({
+  comment: {
     type: Object,
     required: true,
   },
 });
 
 // we grab the file extension and map it to the diff-language
-const languageMap = { 
-  rs: 'rust', 
-  vue: 'html', 
-  Dockerfile: 'docker' 
+const languageMap = {
+  rs: 'rust',
+  vue: 'html',
+  Dockerfile: 'docker'
 };
 
 function basename(str, sep) {
@@ -216,7 +216,7 @@ pre[class*="language-"] {
 	cursor: help;
 }
 
-.token.prefix.inserted, 
+.token.prefix.inserted,
 .token.prefix.deleted,
 .token.prefix.unchanged {
   padding-right: 2em;

server/internal/github/diff.go

@@ -2,14 +2,13 @@ package github
 
 import (
 	"fmt"
-	"regexp"
-	"strconv"
 	"strings"
+
+	"github.com/stanistan/present-me/internal/github/diff"
 )
 
 type diffScanner struct {
-	countFrom                 int
-	countLinesNotStartingWith string
+	hunkRange diff.HunkRange
 
 	start, end int
 }
@@ -41,8 +40,8 @@ func (p *diffScanner) filter(lines []string, auto bool) []string {
 		chunk  []string
 		chunks []diffChunk
 
-		lineNo     = p.countFrom
-		lastPrefix string
+		lineNo     = p.hunkRange.StartingAt
+		lastPrefix = "BANANA" // sentinel
 
 		pushChunk = func() {
 			if len(chunk) > 0 {
@@ -76,7 +75,7 @@ func (p *diffScanner) filter(lines []string, auto bool) []string {
 		lastPrefix = prefix
 
 		// track if we're moving forward to the desired place
-		if !strings.HasPrefix(line, p.countLinesNotStartingWith) {
+		if !strings.HasPrefix(line, p.hunkRange.IgnorePrefix) {
 			lineNo++
 		}
 	}
@@ -90,7 +89,7 @@ func (p *diffScanner) filter(lines []string, auto bool) []string {
 
 	for chunkIdx >= 0 {
 		chunk := chunks[chunkIdx]
-		if auto && len(out) >= 3 && !chunk.isUseful(p.countLinesNotStartingWith) {
+		if auto && len(out) >= 3 && !chunk.isUseful(p.hunkRange.IgnorePrefix) {
 			break
 		}
 
@@ -101,60 +100,27 @@ func (p *diffScanner) filter(lines []string, auto bool) []string {
 	return out
 }
 
-var diffHunkPrefixRegexp = regexp.MustCompile(`^@@ -(\d+),\d+ \+(\d+),\d+ @@`)
-
-func diffHunkPrefix(hunk string) (diffHunkMetadata, error) {
-	meta := diffHunkMetadata{}
-	m := diffHunkPrefixRegexp.FindStringSubmatch(hunk)
-	if m == nil {
-		return meta, fmt.Errorf("could not parse hunk prefix")
-	}
-
-	meta.Left, _ = strconv.Atoi(m[1])
-	meta.Right, _ = strconv.Atoi(m[2])
-	return meta, nil
-}
-
-type diffHunkMetadata struct {
-	Left, Right int
-}
-
-func (m *diffHunkMetadata) countConfig(side string) (int, string, error) {
-	// - using the meta, we can see what the first line of the hunk is.
-	// - depending on if we're doing LEFT or RIGHT, it means we will
-	//   count (or not) specific lines when deciding which ones to include
-	//   or not.
-	switch side {
-	case "LEFT":
-		return m.Left, "+", nil
-	case "RIGHT":
-		return m.Right, "-", nil
-	default:
-		return 0, "", fmt.Errorf("side should be one of LEFT/RIGHT got %s", side)
-	}
-}
-
 func diffRange(c *PullRequestComment) (int, int, bool, error) {
 
 	// - endLine is the line that the comment is on or after,
 	// - startLine is the beginning line that we'll include in our diff,
 	//   and it looks like github defaults to 4 lines included if there is
 	//   no `StartLine`.
 	var endLine int
-	if c.Line != nil {
-		endLine = *c.Line
-	} else if c.OriginalLine != nil {
+	if c.OriginalLine != nil {
 		endLine = *c.OriginalLine
+	} else if c.Line != nil {
+		endLine = *c.Line
 	} else {
 		return 0, 0, false, fmt.Errorf("invalid nil line")
 	}
 
 	var startLine int
 	var auto bool
-	if c.StartLine != nil {
-		startLine = *c.StartLine
-	} else if c.OriginalStartLine != nil {
+	if c.OriginalStartLine != nil {
 		startLine = *c.OriginalStartLine
+	} else if c.StartLine != nil {
+		startLine = *c.StartLine
 	} else {
 		startLine = endLine - 3
 		auto = true

server/internal/github/diff/hunk_meta.go

@@ -0,0 +1,53 @@
+package diff
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+)
+
+var (
+	hunkPrefixRegexp = regexp.MustCompile(`^@@ -(\d+),(\d+) \+(\d+),(\d+) @@`)
+)
+
+type HunkRange struct {
+	StartingAt, NumLines int
+	IgnorePrefix         string
+}
+
+type HunkMeta struct {
+	Original, New HunkRange
+}
+
+func ParseHunkMeta(hunk string) (HunkMeta, error) {
+
+	m := hunkPrefixRegexp.FindStringSubmatch(hunk)
+	if m == nil {
+		return HunkMeta{}, fmt.Errorf("could not parse hunk prefix")
+	}
+
+	l1, _ := strconv.Atoi(m[1])
+	l2, _ := strconv.Atoi(m[2])
+	r1, _ := strconv.Atoi(m[3])
+	r2, _ := strconv.Atoi(m[4])
+
+	return HunkMeta{
+		Original: HunkRange{StartingAt: l1, NumLines: l2, IgnorePrefix: "+"},
+		New:      HunkRange{StartingAt: r1, NumLines: r2, IgnorePrefix: "-"},
+	}, nil
+}
+
+func (m *HunkMeta) RangeForSide(side string) (HunkRange, error) {
+	// - using the meta, we can see what the first line of the hunk is.
+	// - depending on if we're doing LEFT or RIGHT, it means we will
+	//   count (or not) specific lines when deciding which ones to include
+	//   or not.
+	switch side {
+	case "LEFT":
+		return m.Original, nil
+	case "RIGHT":
+		return m.New, nil
+	default:
+		return HunkRange{}, fmt.Errorf("side should be one of LEFT/RIGHT got %s", side)
+	}
+}

server/internal/github/diff/range.go

@@ -0,0 +1 @@
+package diff

server/internal/github/diff_test.go

@@ -4,22 +4,29 @@ import (
 	_ "embed"
 	"testing"
 
+	"github.com/stanistan/present-me/internal/github/diff"
 	"github.com/stretchr/testify/require"
 )
 
 func TestParseDiffHunkPrefix(t *testing.T) {
 	t.Run("first case", func(t *testing.T) {
-		meta, err := diffHunkPrefix("@@ -230,6 +200,9 @@ if (!defined $initial_reply_to && $prompting) {")
+		meta, err := diff.ParseHunkMeta("@@ -230,6 +200,9 @@ if (!defined $initial_reply_to && $prompting) {")
 		require.NoError(t, err)
-		require.Equal(t, diffHunkMetadata{230, 200}, meta)
+		require.Equal(t, diff.HunkMeta{
+			Original: diff.HunkRange{StartingAt: 230, NumLines: 6, IgnorePrefix: "+"},
+			New:      diff.HunkRange{StartingAt: 200, NumLines: 9, IgnorePrefix: "-"},
+		}, meta)
 	})
 	t.Run("succeeds", func(t *testing.T) {
-		meta, err := diffHunkPrefix("@@ -0,6 +200,9 @@ if (!defined $initial_reply_to && $prompting) {")
+		meta, err := diff.ParseHunkMeta("@@ -0,6 +200,9 @@ if (!defined $initial_reply_to && $prompting) {")
 		require.NoError(t, err)
-		require.Equal(t, diffHunkMetadata{0, 200}, meta)
+		require.Equal(t, diff.HunkMeta{
+			Original: diff.HunkRange{StartingAt: 0, NumLines: 6, IgnorePrefix: "+"},
+			New:      diff.HunkRange{StartingAt: 200, NumLines: 9, IgnorePrefix: "-"},
+		}, meta)
 	})
 	t.Run("errs", func(t *testing.T) {
-		_, err := diffHunkPrefix("@@ -0,6 +a,9 @@ if (!defined $initial_reply_to && $prompting) {")
+		_, err := diff.ParseHunkMeta("@@ -0,6 +a,9 @@ if (!defined $initial_reply_to && $prompting) {")
 		require.Error(t, err)
 	})
 }

server/internal/github/github.go

@@ -172,6 +172,7 @@ func (g *Client) FetchReviewModel(ctx context.Context, r *ReviewParams) (*Review
 					// response.
 					return err
 				}
+
 				comment.DiffHunk = &diff
 				comments[idx] = comment
 			}

server/internal/github/review_model.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/stanistan/present-me/internal/errors"
+	"github.com/stanistan/present-me/internal/github/diff"
 )
 
 type ReviewModel struct {
@@ -36,13 +37,13 @@ func orderOf(c string) (int, bool) {
 
 func generateDiff(comment *PullRequestComment) (string, error) {
 	// 1. we extract the metadata, we know which side we are going to be starting on.
-	meta, err := diffHunkPrefix(*comment.DiffHunk)
+	meta, err := diff.ParseHunkMeta(*comment.DiffHunk)
 	if err != nil {
 		return "", errors.WithStack(err)
 	}
 
 	// 2. how are we counting lines?
-	countFrom, countLinesNotStartingWith, err := meta.countConfig(*comment.Side)
+	hunkRange, err := meta.RangeForSide(*comment.Side)
 	if err != nil {
 		return "", errors.WithStack(err)
 	}
@@ -55,17 +56,12 @@ func generateDiff(comment *PullRequestComment) (string, error) {
 
 	// 4. configure out scanner
 	scanner := &diffScanner{
-		countFrom:                 countFrom,
-		countLinesNotStartingWith: countLinesNotStartingWith,
-		start:                     startLine,
-		end:                       endLine,
+		hunkRange: hunkRange,
+		start:     startLine,
+		end:       endLine,
 	}
 
 	// 5. filter our diff lines to only what's relevant for this comment
-	out := scanner.filter(
-		strings.Split(*comment.DiffHunk, "\n"),
-		auto,
-	)
-
+	out := scanner.filter(strings.Split(*comment.DiffHunk, "\n"), auto)
 	return strings.Join(out, "\n"), nil
 }