chore: github diff code cleanup (#109)

Moving diff generation code into the `internal/github/diff` package

frontend/src/Review.ts

@@ -1,3 +1,8 @@
+//
+// Note!
+//
+// This should be kept *generally* in sync with server/internal/api/api.go structs.
+
 export interface MaybeLinked {
   text: string;
   href?: string;

server/internal/api/api.go

@@ -56,11 +56,13 @@ type CodeBlock struct {
 	IsDiff   bool   `json:"diff,omitempty"`
 }
 
+// MaybeLinked refers to text that can have a url associated with it.
 type MaybeLinked struct {
 	Text string `json:"text"`
 	HRef string `json:"href,omitempty"`
 }
 
+// LabelledLink is a text link with a label.
 type LabelledLink struct {
 	MaybeLinked
 	Label string `json:"label"`

server/internal/github/api_source_prme.go

@@ -3,7 +3,6 @@ package github
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/stanistan/present-me/internal/api"
 	"github.com/stanistan/present-me/internal/errors"
@@ -28,9 +27,9 @@ func (s *CommentsAPISource) GetReview(ctx context.Context) (api.Review, error) {
 		return api.Review{}, errors.WithStack(err)
 	}
 
-	var body []string
+	var body string
 	if model.PR.Body != nil && *model.PR.Body != "" {
-		body = append(body, *model.PR.Body)
+		body = *model.PR.Body
 	}
 
 	return api.Review{
@@ -62,7 +61,7 @@ func (s *CommentsAPISource) GetReview(ctx context.Context) (api.Review, error) {
 		MetaData: map[string]any{
 			"params": s.ReviewParamsMap,
 		},
-		Body:     strings.Join(body, "\n\n---\n\n"),
+		Body:     body,
 		Comments: transformComments(model.Comments),
 	}, nil
 }

server/internal/github/diff/range.go

@@ -1 +1,12 @@
 package diff
+
+type RangeFrom [2]*int
+
+func (r *RangeFrom) extract() (int, bool) {
+	if r[0] != nil {
+		return *r[0], true
+	} else if r[1] != nil {
+		return *r[1], true
+	}
+	return 0, false
+}

server/internal/github/diff/scanner.go

@@ -0,0 +1,130 @@
+package diff
+
+import (
+	"strings"
+
+	"github.com/stanistan/present-me/internal/errors"
+)
+
+const (
+	implicitNumberOfLines = 3
+)
+
+type Scanner struct {
+	r          HunkRange
+	start, end int
+	auto       bool
+}
+
+func NewScanner(r HunkRange, start, end RangeFrom) (*Scanner, error) {
+	endLine, ok := end.extract()
+	if !ok {
+		return nil, errors.New("cannot parse endLne from range")
+	}
+
+	auto := false
+	startLine, ok := start.extract()
+	if !ok {
+		startLine = endLine - implicitNumberOfLines
+		auto = true
+	}
+
+	return &Scanner{
+		r:     r,
+		start: startLine,
+		end:   endLine,
+		auto:  auto,
+	}, nil
+}
+
+func (s *Scanner) shouldCount(line string) bool {
+	return !strings.HasPrefix(line, s.r.IgnorePrefix)
+}
+
+func (s *Scanner) isChunkUseful(c *chunk) bool {
+	return c.useful && c.prefix != s.r.IgnorePrefix
+}
+
+type chunk struct {
+	prefix string
+	lines  []string
+	useful bool
+}
+
+func (c *chunk) pushLine(l string) {
+	lineIsUseful := strings.TrimSpace(l[1:]) != ""
+	c.useful = c.useful || lineIsUseful
+	c.lines = append(c.lines, l)
+}
+
+func (s *Scanner) Filter(ls []string) []string {
+	var (
+		c         = chunk{}
+		cs        []chunk
+		pushChunk = func() {
+			if len(c.lines) > 0 {
+				cs = append(cs, c)
+				c = chunk{}
+			}
+		}
+	)
+
+	lineNo := s.r.StartingAt
+	for idx, line := range ls {
+		if idx == 0 {
+			// we skip the first line
+			// it is hunk metadata
+			continue
+		}
+
+		if len(line) == 0 {
+			// ensure line is normalized for diffing,
+			// this only happens in testing because of trimmed whitespace
+			// and we need to absolutely have a prefix for
+			// empty context lines
+			line = " "
+		}
+
+		prefix := line[0:1]
+
+		// we go through the entire diff hunk, making sure
+		// to track that we're in the desired range.
+		//
+		// and while we're in here we parse out chunks
+		// based on the kind of context that they are, `+`, `-`, ` `
+		// based on the prefix changing
+		if lineNo >= s.start && lineNo <= s.end {
+			if prefix != c.prefix {
+				pushChunk()
+			}
+			c.pushLine(line)
+		}
+
+		// always track the last prefix for this chunk
+		c.prefix = prefix
+
+		if s.shouldCount(line) {
+			lineNo++
+		}
+	}
+
+	pushChunk()
+
+	var (
+		out       []string
+		numChunks = len(cs)
+		chunkIdx  = numChunks - 1
+	)
+
+	for chunkIdx >= 0 {
+		c := cs[chunkIdx]
+		if s.auto && len(out) >= implicitNumberOfLines && !s.isChunkUseful(&c) {
+			break
+		}
+
+		out = append(c.lines, out...)
+		chunkIdx--
+	}
+
+	return out
+}

server/internal/github/review_model.go

@@ -35,33 +35,33 @@ func orderOf(c string) (int, bool) {
 	return n, true
 }
 
-func generateDiff(comment *PullRequestComment) (string, error) {
-	// 1. we extract the metadata, we know which side we are going to be starting on.
-	meta, err := diff.ParseHunkMeta(*comment.DiffHunk)
+func generateDiff(c *PullRequestComment) (string, error) {
+	// we extract the metadata, we know which side we are going to be starting on.
+	meta, err := diff.ParseHunkMeta(*c.DiffHunk)
 	if err != nil {
 		return "", errors.WithStack(err)
 	}
 
-	// 2. how are we counting lines?
-	hunkRange, err := meta.RangeForSide(*comment.Side)
+	// how are we counting lines?
+	hunkRange, err := meta.RangeForSide(*c.Side)
 	if err != nil {
 		return "", errors.WithStack(err)
 	}
 
-	// 3. what is our range?
-	endLine, startLine, auto, err := diffRange(comment)
+	// - 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`.
+	scanner, err := diff.NewScanner(
+		hunkRange,
+		diff.RangeFrom{c.OriginalStartLine, c.StartLine},
+		diff.RangeFrom{c.OriginalLine, c.Line},
+	)
 	if err != nil {
 		return "", errors.WithStack(err)
 	}
 
-	// 4. configure out scanner
-	scanner := &diffScanner{
-		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)
+	// filter our diff lines to only what's relevant for this comment
+	out := scanner.Filter(strings.Split(*c.DiffHunk, "\n"))
 	return strings.Join(out, "\n"), nil
 }