diff --git a/server/internal/github/api_source_prme.go b/server/internal/github/api_source_prme.go index ca613be..6041858 100644 --- a/server/internal/github/api_source_prme.go +++ b/server/internal/github/api_source_prme.go @@ -22,7 +22,15 @@ func (s *CommentsAPISource) GetReview(ctx context.Context) (api.Review, error) { return api.Review{}, errors.WithStack(err) } - model, err := FetchReviewModel(ctx, params, CommentMatchesTag(s.ReviewParamsMap.Tag)) + model, err := FetchReviewModel( + ctx, + params, + CommentMatchesTag(s.ReviewParamsMap.Tag), + func(body string) (int, bool) { + t, ok := parseReviewTag(body) + return t.Order, ok + }, + ) if err != nil { return api.Review{}, errors.WithStack(err) } diff --git a/server/internal/github/api_source_review.go b/server/internal/github/api_source_review.go index b38d3d8..02fe55b 100644 --- a/server/internal/github/api_source_review.go +++ b/server/internal/github/api_source_review.go @@ -25,7 +25,7 @@ func (s *ReviewAPISource) GetReview(ctx context.Context) (api.Review, error) { return api.Review{}, errors.New("review must have review id") } - model, err := FetchReviewModel(ctx, params, CommentMatchesReview(params.ReviewID)) + model, err := FetchReviewModel(ctx, params, CommentMatchesReview(params.ReviewID), orderOf) if err != nil { return api.Review{}, errors.WithStack(err) } diff --git a/server/internal/github/comment.go b/server/internal/github/comment.go index 261ecdc..f0f7b9e 100644 --- a/server/internal/github/comment.go +++ b/server/internal/github/comment.go @@ -1,11 +1,25 @@ package github -import "github.com/stanistan/present-me/internal/api" +import ( + "regexp" + "strconv" + "strings" + + "github.com/stanistan/present-me/internal/api" + "github.com/stanistan/present-me/internal/errors" + "github.com/stanistan/present-me/internal/github/diff" +) func transformComments(cs []*PullRequestComment) []api.Comment { out := make([]api.Comment, len(cs)) for idx, c := range cs { c := c + + diff, err := commentCodeDiff(c) + if err != nil { + diff = *c.DiffHunk + } + out[idx] = api.Comment{ Number: idx + 1, Title: api.MaybeLinked{ @@ -15,7 +29,7 @@ func transformComments(cs []*PullRequestComment) []api.Comment { Description: commentBody(*c.Body), CodeBlock: api.CodeBlock{ IsDiff: true, - Content: *c.DiffHunk, + Content: diff, Language: detectLanguage(*c.Path), }, } @@ -27,3 +41,46 @@ func commentBody(s string) string { out := startsWithNumberRegexp.ReplaceAllString(s, "") return prmeTagRegexp.ReplaceAllString(out, "") } + +func commentCodeDiff(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) + } + + // how are we counting lines? + hunkRange, err := meta.RangeForSide(*c.Side) + if err != nil { + return "", errors.WithStack(err) + } + + // - 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) + } + + // 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 +} + +var startsWithNumberRegexp = regexp.MustCompile(`^\s*(\d+)\.\s*`) + +func orderOf(c string) (int, bool) { + m := startsWithNumberRegexp.FindStringSubmatch(c) + if m == nil { + return 0, false + } + + n, _ := strconv.Atoi(m[1]) + return n, true +} diff --git a/server/internal/github/diff_test.go b/server/internal/github/diff_test.go index ad92546..3d74088 100644 --- a/server/internal/github/diff_test.go +++ b/server/internal/github/diff_test.go @@ -32,7 +32,7 @@ func TestDiffGenerator(t *testing.T) { assert.NoError(t, err, "test data file is an invalid json PullRequestComment") comment.DiffHunk = &input - diff, err := generateDiff(&comment) + diff, err := commentCodeDiff(&comment) assert.NoError(t, err, "failed to generate diff") assert.Equal(t, expected, diff+"\n", "generated diff doesn't match expected output") }) diff --git a/server/internal/github/github.go b/server/internal/github/github.go index 48d7aee..7ae2228 100644 --- a/server/internal/github/github.go +++ b/server/internal/github/github.go @@ -32,7 +32,7 @@ type PrivateKey struct { File string `name:"file" env:"GH_PK_FILE"` } -func (o *ClientOptions) HTTPClient(ctx context.Context) (*http.Client, error) { +func (o *ClientOptions) httpClient(ctx context.Context) (*http.Client, error) { var ( itr http.RoundTripper err error @@ -59,7 +59,7 @@ type Client struct { } func New(ctx context.Context, opts ClientOptions) (*Client, error) { - c, err := opts.HTTPClient(ctx) + c, err := opts.httpClient(ctx) if err != nil { return nil, err } @@ -167,13 +167,18 @@ func (g *Client) ListComments( ) } -func FetchReviewModel(ctx context.Context, r *ReviewParams, pred CommentPredicate) (*ReviewModel, error) { +func FetchReviewModel( + ctx context.Context, + r *ReviewParams, + pred CommentPredicate, + orderOf func(string) (int, bool), +) (*ReviewModel, error) { client, ok := Ctx(ctx) if !ok || client == nil { return nil, errors.New("context missing github client") } - model, err := client.FetchReviewModel(ctx, r, pred) + model, err := client.FetchReviewModel(ctx, r, pred, orderOf) if err != nil { return nil, errors.WithStack(err) } @@ -182,7 +187,10 @@ func FetchReviewModel(ctx context.Context, r *ReviewParams, pred CommentPredicat } func (g *Client) FetchReviewModel( - ctx context.Context, r *ReviewParams, pred CommentPredicate, + ctx context.Context, + r *ReviewParams, + pred CommentPredicate, + orderOf func(string) (int, bool), ) (*ReviewModel, error) { model := &ReviewModel{Params: r} group, ctx := errgroup.WithContext(ctx) @@ -195,6 +203,7 @@ func (g *Client) FetchReviewModel( return err }) + // N.B. this is legacy-ish if r.ReviewID != 0 { group.Go(func() error { review, err := g.GetReview(ctx, r) @@ -208,20 +217,6 @@ func (g *Client) FetchReviewModel( group.Go(func() error { comments, err := g.ListComments(ctx, r, pred) if err == nil { - for idx := range comments { - comment := comments[idx] - diff, err := generateDiff(comment) - if err != nil { - // TODO(stanistan): - // consider logging the warning here, and not mutating the diff as well, - // or _also_ returning the fact that there's an error warning in the API - // response. - return err - } - - comment.DiffHunk = &diff - comments[idx] = comment - } model.Comments = comments } return err diff --git a/server/internal/github/predicate.go b/server/internal/github/predicate.go index d6890b4..b7bbe04 100644 --- a/server/internal/github/predicate.go +++ b/server/internal/github/predicate.go @@ -15,21 +15,21 @@ func CommentMatchesReview(reviewID int64) CommentPredicate { } } -var prmeTagRegexp = regexp.MustCompile(`\(prme([^-\s]+)?(-(\d+))?\)\s*$`) - -type reviewTag struct { +type ReviewTag struct { Review string Order int } -func parseReviewTag(s string) (reviewTag, bool) { +var prmeTagRegexp = regexp.MustCompile(`\(prme([^-\s]+)?(-(\d+))?\)\s*$`) + +func parseReviewTag(s string) (ReviewTag, bool) { m := prmeTagRegexp.FindStringSubmatch(s) if m == nil { - return reviewTag{}, false + return ReviewTag{}, false } n, _ := strconv.Atoi(m[3]) - return reviewTag{Review: m[1], Order: n}, true + return ReviewTag{Review: m[1], Order: n}, true } func CommentMatchesTag(tag string) CommentPredicate { diff --git a/server/internal/github/predicate_test.go b/server/internal/github/predicate_test.go index 8a906c8..cb156f7 100644 --- a/server/internal/github/predicate_test.go +++ b/server/internal/github/predicate_test.go @@ -11,36 +11,36 @@ func TestCommentTagRegex(t *testing.T) { name string input string expectedOk bool - expected reviewTag + expected ReviewTag } for _, data := range []testData{ { - "standard", "foo (prme1)", true, reviewTag{"1", 0}, + "standard", "foo (prme1)", true, ReviewTag{"1", 0}, }, { - "no-prefix", "(prme2)", true, reviewTag{"2", 0}, + "no-prefix", "(prme2)", true, ReviewTag{"2", 0}, }, { - "trailing space", "(prme2) ", true, reviewTag{"2", 0}, + "trailing space", "(prme2) ", true, ReviewTag{"2", 0}, }, { - "with order", "end of text (prmesomething-1)\n", true, reviewTag{"something", 1}, + "with order", "end of text (prmesomething-1)\n", true, ReviewTag{"something", 1}, }, { - "with order 3", "(prmesomething-3)", true, reviewTag{"something", 3}, + "with order 3", "(prmesomething-3)", true, ReviewTag{"something", 3}, }, { - "doesnt match in the middle", "(prme1) banana", false, reviewTag{}, + "doesnt match in the middle", "(prme1) banana", false, ReviewTag{}, }, { - "can have naked prme", "foo (prme)", true, reviewTag{"", 0}, + "can have naked prme", "foo (prme)", true, ReviewTag{"", 0}, }, { - "won't parse one without parentheses", "foo prme", false, reviewTag{}, + "won't parse one without parentheses", "foo prme", false, ReviewTag{}, }, { - "no tag but order", "anything (prme-5)", true, reviewTag{"", 5}, + "no tag but order", "anything (prme-5)", true, ReviewTag{"", 5}, }, } { d := data diff --git a/server/internal/github/review_model.go b/server/internal/github/review_model.go index ef03851..98c8ccb 100644 --- a/server/internal/github/review_model.go +++ b/server/internal/github/review_model.go @@ -1,14 +1,5 @@ package github -import ( - "regexp" - "strconv" - "strings" - - "github.com/stanistan/present-me/internal/errors" - "github.com/stanistan/present-me/internal/github/diff" -) - type ReviewModel struct { Params *ReviewParams `json:"params"` @@ -22,46 +13,3 @@ type ReviewFile struct { IsAnnotated bool `json:"isAnnotated"` File *CommitFile `json:"file"` } - -var startsWithNumberRegexp = regexp.MustCompile(`^\s*(\d+)\.\s*`) - -func orderOf(c string) (int, bool) { - m := startsWithNumberRegexp.FindStringSubmatch(c) - if m == nil { - return 0, false - } - - n, _ := strconv.Atoi(m[1]) - return n, true -} - -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) - } - - // how are we counting lines? - hunkRange, err := meta.RangeForSide(*c.Side) - if err != nil { - return "", errors.WithStack(err) - } - - // - 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) - } - - // 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 -}