Skip to content

Commit

Permalink
comment ordering is parameterized
Browse files Browse the repository at this point in the history
  • Loading branch information
stanistan committed Aug 21, 2023
1 parent a639b02 commit f50e980
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 39 deletions.
10 changes: 9 additions & 1 deletion server/internal/github/api_source_prme.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion server/internal/github/api_source_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
14 changes: 14 additions & 0 deletions server/internal/github/comment.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package github

import (
"regexp"
"strconv"
"strings"

"github.com/stanistan/present-me/internal/api"
Expand Down Expand Up @@ -70,3 +72,15 @@ func commentCodeDiff(c *PullRequestComment) (string, error) {
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
}
2 changes: 1 addition & 1 deletion server/internal/github/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand Down
17 changes: 14 additions & 3 deletions server/internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,21 +167,31 @@ 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)
}

return model, nil
}

func (g *Client) FetchReviewModel(ctx context.Context, r *ReviewParams, pred CommentPredicate) (*ReviewModel, error) {
func (g *Client) FetchReviewModel(
ctx context.Context,
r *ReviewParams,
pred CommentPredicate,
orderOf func(string) (int, bool),
) (*ReviewModel, error) {
model := &ReviewModel{Params: r}
group, ctx := errgroup.WithContext(ctx)

Expand All @@ -193,6 +203,7 @@ func (g *Client) FetchReviewModel(ctx context.Context, r *ReviewParams, pred Com
return err
})

// N.B. this is legacy-ish
if r.ReviewID != 0 {
group.Go(func() error {
review, err := g.GetReview(ctx, r)
Expand Down
12 changes: 6 additions & 6 deletions server/internal/github/predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 10 additions & 10 deletions server/internal/github/predicate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 0 additions & 17 deletions server/internal/github/review_model.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
package github

import (
"regexp"
"strconv"
)

type ReviewModel struct {
Params *ReviewParams `json:"params"`

Expand All @@ -18,15 +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
}

0 comments on commit f50e980

Please sign in to comment.