From 345ae04837eb6bf01c33348ec5a80139484e9ff9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 13 Jul 2025 16:07:46 -0700 Subject: [PATCH] Allow code review comments display cross commits even if the head branch has a force-push --- models/issues/comment.go | 4 + models/issues/comment_code.go | 53 ++-- models/issues/comment_test.go | 6 +- models/issues/review.go | 2 +- models/issues/review_test.go | 2 +- modules/git/diff.go | 57 ++++ modules/git/diff_test.go | 6 + routers/api/v1/repo/pull_review.go | 13 +- routers/web/repo/issue_view.go | 35 +-- routers/web/repo/pull.go | 142 ++++----- routers/web/repo/pull_review.go | 56 ++-- routers/web/repo/pull_review_test.go | 14 +- services/convert/pull_review.go | 48 ++- services/feed/notifier.go | 28 +- services/forms/repo_form.go | 3 +- services/gitdiff/gitdiff.go | 28 -- services/gitdiff/gitdiff_test.go | 42 --- services/mailer/incoming/incoming_handler.go | 3 +- services/mailer/mail_issue_common.go | 7 +- services/pull/review.go | 297 +++++++++++++----- services/pull/review_test.go | 89 ++++++ templates/repo/diff/box.tmpl | 2 +- templates/repo/diff/comment_form.tmpl | 3 +- .../repo/issue/view_content/comments.tmpl | 6 +- 24 files changed, 562 insertions(+), 384 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 4fdb0c1808f..082ede1e199 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -1007,6 +1007,7 @@ type FindCommentsOptions struct { RepoID int64 IssueID int64 ReviewID int64 + CommitSHA string Since int64 Before int64 Line int64 @@ -1052,6 +1053,9 @@ func (opts FindCommentsOptions) ToConds() builder.Cond { if opts.IsPull.Has() { cond = cond.And(builder.Eq{"issue.is_pull": opts.IsPull.Value()}) } + if opts.CommitSHA != "" { + cond = cond.And(builder.Eq{"comment.commit_sha": opts.CommitSHA}) + } return cond } diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index 55e67a1243b..2cd2614ff02 100644 --- a/models/issues/comment_code.go +++ b/models/issues/comment_code.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/renderhelper" + repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/markup/markdown" @@ -16,39 +17,44 @@ import ( ) // CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS -type CodeComments map[string]map[int64][]*Comment +type CodeComments map[string][]*Comment -// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line -func FetchCodeComments(ctx context.Context, issue *Issue, currentUser *user_model.User, showOutdatedComments bool) (CodeComments, error) { - return fetchCodeCommentsByReview(ctx, issue, currentUser, nil, showOutdatedComments) +func (cc CodeComments) AllComments() []*Comment { + var allComments []*Comment + for _, comments := range cc { + allComments = append(allComments, comments...) + } + return allComments } -func fetchCodeCommentsByReview(ctx context.Context, issue *Issue, currentUser *user_model.User, review *Review, showOutdatedComments bool) (CodeComments, error) { - pathToLineToComment := make(CodeComments) +// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line +func FetchCodeComments(ctx context.Context, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, showOutdatedComments bool) (CodeComments, error) { + return fetchCodeCommentsByReview(ctx, repo, issueID, currentUser, nil, showOutdatedComments) +} + +func fetchCodeCommentsByReview(ctx context.Context, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, review *Review, showOutdatedComments bool) (CodeComments, error) { + codeCommentsPathMap := make(CodeComments) if review == nil { review = &Review{ID: 0} } opts := FindCommentsOptions{ Type: CommentTypeCode, - IssueID: issue.ID, + IssueID: issueID, ReviewID: review.ID, } - comments, err := findCodeComments(ctx, opts, issue, currentUser, review, showOutdatedComments) + comments, err := FindCodeComments(ctx, opts, repo, currentUser, review, showOutdatedComments) if err != nil { return nil, err } for _, comment := range comments { - if pathToLineToComment[comment.TreePath] == nil { - pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment) - } - pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment) + codeCommentsPathMap[comment.TreePath] = append(codeCommentsPathMap[comment.TreePath], comment) } - return pathToLineToComment, nil + return codeCommentsPathMap, nil } -func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issue, currentUser *user_model.User, review *Review, showOutdatedComments bool) ([]*Comment, error) { +func FindCodeComments(ctx context.Context, opts FindCommentsOptions, repo *repo_model.Repository, currentUser *user_model.User, review *Review, showOutdatedComments bool) ([]*Comment, error) { var comments CommentList if review == nil { review = &Review{ID: 0} @@ -67,10 +73,6 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu return nil, err } - if err := issue.LoadRepo(ctx); err != nil { - return nil, err - } - if err := comments.LoadPosters(ctx); err != nil { return nil, err } @@ -110,12 +112,12 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu return nil, err } - if err := comment.LoadReactions(ctx, issue.Repo); err != nil { + if err := comment.LoadReactions(ctx, repo); err != nil { return nil, err } var err error - rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{ + rctx := renderhelper.NewRenderContextRepoComment(ctx, repo, renderhelper.RepoCommentOptions{ FootnoteContextID: strconv.FormatInt(comment.ID, 10), }) if comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content); err != nil { @@ -124,14 +126,3 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu } return comments[:n], nil } - -// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number -func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, showOutdatedComments bool) (CommentList, error) { - opts := FindCommentsOptions{ - Type: CommentTypeCode, - IssueID: issue.ID, - TreePath: treePath, - Line: line, - } - return findCodeComments(ctx, opts, issue, currentUser, nil, showOutdatedComments) -} diff --git a/models/issues/comment_test.go b/models/issues/comment_test.go index c08e3b970d3..627cc188a66 100644 --- a/models/issues/comment_test.go +++ b/models/issues/comment_test.go @@ -68,15 +68,15 @@ func TestFetchCodeComments(t *testing.T) { issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - res, err := issues_model.FetchCodeComments(db.DefaultContext, issue, user, false) + res, err := issues_model.FetchCodeComments(db.DefaultContext, issue.Repo, issue.ID, user, false) assert.NoError(t, err) assert.Contains(t, res, "README.md") assert.Contains(t, res["README.md"], int64(4)) assert.Len(t, res["README.md"][4], 1) - assert.Equal(t, int64(4), res["README.md"][4][0].ID) + assert.Equal(t, int64(4), res["README.md"][0].ID) user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - res, err = issues_model.FetchCodeComments(db.DefaultContext, issue, user2, false) + res, err = issues_model.FetchCodeComments(db.DefaultContext, issue.Repo, issue.ID, user2, false) assert.NoError(t, err) assert.Len(t, res, 1) } diff --git a/models/issues/review.go b/models/issues/review.go index 71fdb7456f1..2102da28453 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -159,7 +159,7 @@ func (r *Review) LoadCodeComments(ctx context.Context) (err error) { if err = r.LoadIssue(ctx); err != nil { return err } - r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue, nil, r, false) + r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue.Repo, r.Issue.ID, nil, r, false) return err } diff --git a/models/issues/review_test.go b/models/issues/review_test.go index 2588b8ba41b..182efb76dce 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -48,7 +48,7 @@ func TestReview_LoadCodeComments(t *testing.T) { assert.NoError(t, review.LoadAttributes(db.DefaultContext)) assert.NoError(t, review.LoadCodeComments(db.DefaultContext)) assert.Len(t, review.CodeComments, 1) - assert.Equal(t, int64(4), review.CodeComments["README.md"][int64(4)][0].Line) + assert.Equal(t, int64(4), review.CodeComments["README.md"][0].Line) } func TestReviewType_Icon(t *testing.T) { diff --git a/modules/git/diff.go b/modules/git/diff.go index 35d115be0e5..7f3d57ddbc8 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -15,6 +15,7 @@ import ( "strings" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) // RawDiffType type of a raw diff. @@ -107,12 +108,16 @@ func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightH leftLine, _ = strconv.Atoi(leftRange[0][1:]) if len(leftRange) > 1 { leftHunk, _ = strconv.Atoi(leftRange[1]) + } else { + leftHunk = util.Iif(leftLine > 0, leftLine, -leftLine) } if len(ranges) > 1 { rightRange := strings.Split(ranges[1], ",") rightLine, _ = strconv.Atoi(rightRange[0]) if len(rightRange) > 1 { rightHunk, _ = strconv.Atoi(rightRange[1]) + } else { + rightHunk = rightLine } } else { log.Debug("Parse line number failed: %v", diffHunk) @@ -342,3 +347,55 @@ func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID str return affectedFiles, err } + +type HunkInfo struct { + LeftLine int64 // Line number in the old file + LeftHunk int64 // Number of lines in the old file + RightLine int64 // Line number in the new file + RightHunk int64 // Number of lines in the new file +} + +// GetAffectedHunksForTwoCommitsSpecialFile returns the affected hunks between two commits for a special file +// git diff --unified=0 abc123 def456 -- src/main.go +func GetAffectedHunksForTwoCommitsSpecialFile(ctx context.Context, repoPath, oldCommitID, newCommitID, filePath string) ([]*HunkInfo, error) { + reader, writer := io.Pipe() + defer func() { + _ = reader.Close() + _ = writer.Close() + }() + go func() { + if err := NewCommand("diff", "--unified=0", "--no-color"). + AddDynamicArguments(oldCommitID, newCommitID). + AddDashesAndList(filePath). + Run(ctx, &RunOpts{ + Dir: repoPath, + Stdout: writer, + }); err != nil { + _ = writer.CloseWithError(fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s, %s]: %w", repoPath, oldCommitID, newCommitID, filePath, err)) + return + } + _ = writer.Close() + }() + + scanner := bufio.NewScanner(reader) + hunks := make([]*HunkInfo, 0, 32) + for scanner.Scan() { + lof := scanner.Text() + if !strings.HasPrefix(lof, "@@") { + continue + } + // Parse the hunk header + leftLine, leftHunk, rightLine, rightHunk := ParseDiffHunkString(lof) + hunks = append([]*HunkInfo{}, &HunkInfo{ + LeftLine: int64(leftLine), + LeftHunk: int64(leftHunk), + RightLine: int64(rightLine), + RightHunk: int64(rightHunk), + }) + } + if scanner.Err() != nil { + return nil, fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s, %s]: %w", repoPath, oldCommitID, newCommitID, filePath, scanner.Err()) + } + + return hunks, nil +} diff --git a/modules/git/diff_test.go b/modules/git/diff_test.go index 7671fffcc16..11bdcd35fb1 100644 --- a/modules/git/diff_test.go +++ b/modules/git/diff_test.go @@ -181,4 +181,10 @@ func TestParseDiffHunkString(t *testing.T) { assert.Equal(t, 3, leftHunk) assert.Equal(t, 19, rightLine) assert.Equal(t, 5, rightHunk) + + leftLine, leftHunk, rightLine, rightHunk = ParseDiffHunkString("@@ -1 +0,0 @@") + assert.Equal(t, 1, leftLine) + assert.Equal(t, 1, leftHunk) + assert.Equal(t, 0, rightLine) + assert.Equal(t, 0, rightHunk) } diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 3c00193fac1..082326333b7 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -13,7 +13,6 @@ import ( "code.gitea.io/gitea/models/organization" access_model "code.gitea.io/gitea/models/perm/access" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/gitrepo" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" @@ -329,14 +328,7 @@ func CreatePullReview(ctx *context.APIContext) { // if CommitID is empty, set it as lastCommitID if opts.CommitID == "" { - gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.Issue.Repo) - if err != nil { - ctx.APIErrorInternal(err) - return - } - defer closer.Close() - - headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName()) + headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pr.GetGitHeadRefName()) if err != nil { ctx.APIErrorInternal(err) return @@ -357,11 +349,12 @@ func CreatePullReview(ctx *context.APIContext) { ctx.Repo.GitRepo, pr.Issue, line, + pr.MergeBase, + opts.CommitID, c.Body, c.Path, true, // pending review 0, // no reply - opts.CommitID, nil, ); err != nil { ctx.APIErrorInternal(err) diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index d0064e763ef..30e0043ee1c 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -730,28 +730,23 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue } comment.Review.Reviewer = user_model.NewGhostUser() } - if err = comment.Review.LoadCodeComments(ctx); err != nil { - ctx.ServerError("Review.LoadCodeComments", err) - return - } - for _, codeComments := range comment.Review.CodeComments { - for _, lineComments := range codeComments { - for _, c := range lineComments { - // Check tag. - role, ok = marked[c.PosterID] - if ok { - c.ShowRole = role - continue - } - c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, permCache, issue, c.HasOriginalAuthor()) - if err != nil { - ctx.ServerError("roleDescriptor", err) - return - } - marked[c.PosterID] = c.ShowRole - participants = addParticipant(c.Poster, participants) + for _, codeComments := range comment.Review.CodeComments { + for _, c := range codeComments { + // Check tag. + role, ok = marked[c.PosterID] + if ok { + c.ShowRole = role + continue } + + c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, permCache, issue, c.HasOriginalAuthor()) + if err != nil { + ctx.ServerError("roleDescriptor", err) + return + } + marked[c.PosterID] = c.ShowRole + participants = addParticipant(c.Poster, participants) } } if err = comment.LoadResolveDoer(ctx); err != nil { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index c5302dd50f5..64d2ad330c9 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -643,8 +643,17 @@ func ViewPullCommits(ctx *context.Context) { ctx.HTML(http.StatusOK, tplPullCommits) } +func indexCommit(commits []*git.Commit, commitID string) *git.Commit { + for i := range commits { + if commits[i].ID.String() == commitID { + return commits[i] + } + } + return nil +} + // ViewPullFiles render pull request changed files list page -func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommit string, willShowSpecifiedCommitRange, willShowSpecifiedCommit bool) { +func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) { ctx.Data["PageIsPullList"] = true ctx.Data["PageIsPullFiles"] = true @@ -653,12 +662,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi return } pull := issue.PullRequest - - var ( - startCommitID string - endCommitID string - gitRepo = ctx.Repo.GitRepo - ) + gitRepo := ctx.Repo.GitRepo prInfo := preparePullViewPullInfo(ctx, issue) if ctx.Written() { @@ -668,77 +672,65 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi return } - // Validate the given commit sha to show (if any passed) - if willShowSpecifiedCommit || willShowSpecifiedCommitRange { - foundStartCommit := len(specifiedStartCommit) == 0 - foundEndCommit := len(specifiedEndCommit) == 0 - - if !(foundStartCommit && foundEndCommit) { - for _, commit := range prInfo.Commits { - if commit.ID.String() == specifiedStartCommit { - foundStartCommit = true - } - if commit.ID.String() == specifiedEndCommit { - foundEndCommit = true - } - - if foundStartCommit && foundEndCommit { - break - } - } - } - - if !(foundStartCommit && foundEndCommit) { - ctx.NotFound(nil) - return - } - } - - if ctx.Written() { - return - } - headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitHeadRefName()) if err != nil { ctx.ServerError("GetRefCommitID", err) return } - ctx.Data["IsShowingOnlySingleCommit"] = willShowSpecifiedCommit + ctx.Data["IsShowingOnlySingleCommit"] = beforeCommitID != "" && beforeCommitID == afterCommitID + isShowAllCommits := (beforeCommitID == "" || beforeCommitID == prInfo.MergeBase) && (afterCommitID == "" || afterCommitID == headCommitID) + ctx.Data["IsShowingAllCommits"] = isShowAllCommits - if willShowSpecifiedCommit || willShowSpecifiedCommitRange { - if len(specifiedEndCommit) > 0 { - endCommitID = specifiedEndCommit - } else { - endCommitID = headCommitID + if beforeCommitID == "" { + beforeCommitID = prInfo.MergeBase + } + if afterCommitID == "" { + afterCommitID = headCommitID + } + + var beforeCommit, afterCommit *git.Commit + if beforeCommitID != prInfo.MergeBase { + beforeCommit = indexCommit(prInfo.Commits, beforeCommitID) + if beforeCommit == nil { + ctx.NotFound(errors.New("before commit not found in PR commits")) + return } - if len(specifiedStartCommit) > 0 { - startCommitID = specifiedStartCommit - } else { - startCommitID = prInfo.MergeBase + beforeCommit, err = beforeCommit.Parent(0) + if err != nil { + ctx.ServerError("GetParentCommit", err) + return } - ctx.Data["IsShowingAllCommits"] = false - } else { - endCommitID = headCommitID - startCommitID = prInfo.MergeBase - ctx.Data["IsShowingAllCommits"] = true + beforeCommitID = beforeCommit.ID.String() + } else { // mergebase commit is not in the list of the pull request commits + beforeCommit, err = gitRepo.GetCommit(beforeCommitID) + if err != nil { + ctx.ServerError("GetCommit", err) + return + } + } + + afterCommit = indexCommit(prInfo.Commits, afterCommitID) + if afterCommit == nil { + ctx.NotFound(errors.New("after commit not found in PR commits")) + return } ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name - ctx.Data["AfterCommitID"] = endCommitID - ctx.Data["BeforeCommitID"] = startCommitID - - fileOnly := ctx.FormBool("file-only") + ctx.Data["AfterCommitID"] = afterCommitID + ctx.Data["BeforeCommitID"] = beforeCommitID maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles files := ctx.FormStrings("files") + fileOnly := ctx.FormBool("file-only") if fileOnly && (len(files) == 2 || len(files) == 1) { maxLines, maxFiles = -1, -1 } diffOptions := &gitdiff.DiffOptions{ - AfterCommitID: endCommitID, + BeforeCommitID: beforeCommitID, + AfterCommitID: afterCommitID, SkipTo: ctx.FormString("skip-to"), MaxLines: maxLines, MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, @@ -746,10 +738,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), } - if !willShowSpecifiedCommit { - diffOptions.BeforeCommitID = startCommitID - } - diff, err := gitdiff.GetDiffForRender(ctx, ctx.Repo.RepoLink, gitRepo, diffOptions, files...) if err != nil { ctx.ServerError("GetDiff", err) @@ -761,7 +749,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi // as the viewed information is designed to be loaded only on latest PR // diff and if you're signed in. var reviewState *pull_model.ReviewState - if ctx.IsSigned && !willShowSpecifiedCommit && !willShowSpecifiedCommitRange { + if ctx.IsSigned && isShowAllCommits { reviewState, err = gitdiff.SyncUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diff, diffOptions) if err != nil { ctx.ServerError("SyncUserSpecificDiff", err) @@ -769,7 +757,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } } - diffShortStat, err := gitdiff.GetDiffShortStat(ctx.Repo.GitRepo, startCommitID, endCommitID) + diffShortStat, err := gitdiff.GetDiffShortStat(ctx.Repo.GitRepo, beforeCommitID, afterCommitID) if err != nil { ctx.ServerError("GetDiffShortStat", err) return @@ -781,7 +769,8 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi "numberOfViewedFiles": diff.NumViewedFiles, } - if err = diff.LoadComments(ctx, issue, ctx.Doer, ctx.Data["ShowOutdatedComments"].(bool)); err != nil { + if err = pull_service.LoadCodeComments(ctx, ctx.Repo.GitRepo, ctx.Repo.Repository, + diff, issue.ID, ctx.Doer, beforeCommit, afterCommit, ctx.Data["ShowOutdatedComments"].(bool)); err != nil { ctx.ServerError("LoadComments", err) return } @@ -816,7 +805,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi if !fileOnly { // note: use mergeBase is set to false because we already have the merge base from the pull request info - diffTree, err := gitdiff.GetDiffTree(ctx, gitRepo, false, startCommitID, endCommitID) + diffTree, err := gitdiff.GetDiffTree(ctx, gitRepo, false, beforeCommitID, afterCommitID) if err != nil { ctx.ServerError("GetDiffTree", err) return @@ -836,17 +825,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi ctx.Data["Diff"] = diff ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0 - baseCommit, err := ctx.Repo.GitRepo.GetCommit(startCommitID) - if err != nil { - ctx.ServerError("GetCommit", err) - return - } - commit, err := gitRepo.GetCommit(endCommitID) - if err != nil { - ctx.ServerError("GetCommit", err) - return - } - if ctx.IsSigned && ctx.Doer != nil { if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil { ctx.ServerError("CanMarkConversation", err) @@ -854,7 +832,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } } - setCompareContext(ctx, baseCommit, commit, ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) + setCompareContext(ctx, beforeCommit, afterCommit, ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) assigneeUsers, err := repo_model.GetRepoAssignees(ctx, ctx.Repo.Repository) if err != nil { @@ -901,7 +879,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool { return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee) } - if !willShowSpecifiedCommit && !willShowSpecifiedCommitRange && pull.Flow == issues_model.PullRequestFlowGithub { + if !isShowAllCommits && pull.Flow == issues_model.PullRequestFlowGithub { if err := pull.LoadHeadRepo(ctx); err != nil { ctx.ServerError("LoadHeadRepo", err) return @@ -930,19 +908,19 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } func ViewPullFilesForSingleCommit(ctx *context.Context) { - viewPullFiles(ctx, "", ctx.PathParam("sha"), true, true) + viewPullFiles(ctx, ctx.PathParam("sha"), ctx.PathParam("sha")) } func ViewPullFilesForRange(ctx *context.Context) { - viewPullFiles(ctx, ctx.PathParam("shaFrom"), ctx.PathParam("shaTo"), true, false) + viewPullFiles(ctx, ctx.PathParam("shaFrom"), ctx.PathParam("shaTo")) } func ViewPullFilesStartingFromCommit(ctx *context.Context) { - viewPullFiles(ctx, "", ctx.PathParam("sha"), true, false) + viewPullFiles(ctx, ctx.PathParam("sha"), "") } func ViewPullFilesForAllCommitsOfPr(ctx *context.Context) { - viewPullFiles(ctx, "", "", false, false) + viewPullFiles(ctx, "", "") } // UpdatePullRequest merge PR's baseBranch into headBranch diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 18e14e9b224..a8fdd613a7b 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" @@ -49,12 +50,8 @@ func RenderNewCodeCommentForm(ctx *context.Context) { ctx.Data["PageIsPullFiles"] = true ctx.Data["Issue"] = issue ctx.Data["CurrentReview"] = currentReview - pullHeadCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(issue.PullRequest.GetGitHeadRefName()) - if err != nil { - ctx.ServerError("GetRefCommitID", err) - return - } - ctx.Data["AfterCommitID"] = pullHeadCommitID + ctx.Data["BeforeCommitID"] = ctx.FormString("before_commit_id") + ctx.Data["AfterCommitID"] = ctx.FormString("after_commit_id") ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled upload.AddUploadContext(ctx, "comment") ctx.HTML(http.StatusOK, tplNewComment) @@ -77,10 +74,7 @@ func CreateCodeComment(ctx *context.Context) { return } - signedLine := form.Line - if form.Side == "previous" { - signedLine *= -1 - } + signedLine := util.Iif(form.Side == "previous", -form.Line, form.Line) var attachments []string if setting.Attachment.Enabled { @@ -92,11 +86,12 @@ func CreateCodeComment(ctx *context.Context) { ctx.Repo.GitRepo, issue, signedLine, + form.BeforeCommitID, + form.AfterCommitID, form.Content, form.TreePath, !form.SingleReview, form.Reply, - form.LatestCommitID, attachments, ) if err != nil { @@ -112,7 +107,7 @@ func CreateCodeComment(ctx *context.Context) { log.Trace("Comment created: %-v #%d[%d] Comment[%d]", ctx.Repo.Repository, issue.Index, issue.ID, comment.ID) - renderConversation(ctx, comment, form.Origin) + renderConversation(ctx, comment, form.Origin, form.BeforeCommitID, form.AfterCommitID) } // UpdateResolveConversation add or remove an Conversation resolved mark @@ -163,14 +158,33 @@ func UpdateResolveConversation(ctx *context.Context) { return } - renderConversation(ctx, comment, origin) + beforeCommitID, afterCommitID := ctx.FormString("before_commit_id"), ctx.FormString("after_commit_id") + + renderConversation(ctx, comment, origin, beforeCommitID, afterCommitID) } -func renderConversation(ctx *context.Context, comment *issues_model.Comment, origin string) { +func renderConversation(ctx *context.Context, comment *issues_model.Comment, origin, beforeCommitID, afterCommitID string) { ctx.Data["PageIsPullFiles"] = origin == "diff" + if err := comment.Issue.LoadPullRequest(ctx); err != nil { + ctx.ServerError("comment.Issue.LoadPullRequest", err) + return + } + if beforeCommitID == "" { + beforeCommitID = comment.Issue.PullRequest.MergeBase + } + if afterCommitID == "" { + var err error + afterCommitID, err = ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitHeadRefName()) + if err != nil { + ctx.ServerError("GetRefCommitID", err) + return + } + } + showOutdatedComments := origin == "timeline" || ctx.Data["ShowOutdatedComments"].(bool) - comments, err := issues_model.FetchCodeCommentsByLine(ctx, comment.Issue, ctx.Doer, comment.TreePath, comment.Line, showOutdatedComments) + comments, err := pull_service.FetchCodeCommentsByLine(ctx, ctx.Repo.Repository, comment.IssueID, + ctx.Doer, beforeCommitID, afterCommitID, comment.TreePath, comment.Line, showOutdatedComments) if err != nil { ctx.ServerError("FetchCodeCommentsByLine", err) return @@ -195,16 +209,8 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori return } ctx.Data["Issue"] = comment.Issue - if err = comment.Issue.LoadPullRequest(ctx); err != nil { - ctx.ServerError("comment.Issue.LoadPullRequest", err) - return - } - pullHeadCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitHeadRefName()) - if err != nil { - ctx.ServerError("GetRefCommitID", err) - return - } - ctx.Data["AfterCommitID"] = pullHeadCommitID + ctx.Data["BeforeCommitID"] = beforeCommitID + ctx.Data["AfterCommitID"] = afterCommitID ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool { return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee) } diff --git a/routers/web/repo/pull_review_test.go b/routers/web/repo/pull_review_test.go index 3d0997ab4d8..7ebf93c7807 100644 --- a/routers/web/repo/pull_review_test.go +++ b/routers/web/repo/pull_review_test.go @@ -41,7 +41,7 @@ func TestRenderConversation(t *testing.T) { var preparedComment *issues_model.Comment run("prepare", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) { - comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "content", "", false, 0, pr.HeadCommitID, nil) + comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "", "", "content", "", false, 0, nil) require.NoError(t, err) comment.Invalidated = true @@ -54,29 +54,29 @@ func TestRenderConversation(t *testing.T) { run("diff with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) { ctx.Data["ShowOutdatedComments"] = true - renderConversation(ctx, preparedComment, "diff") + renderConversation(ctx, preparedComment, "diff", "", "") assert.Contains(t, resp.Body.String(), `
0 { - if reviewID != 0 { - first, err := issues_model.FindComments(ctx, &issues_model.FindCommentsOptions{ - ReviewID: reviewID, - Line: line, - TreePath: treePath, - Type: issues_model.CommentTypeCode, - ListOptions: db.ListOptions{ - PageSize: 1, - Page: 1, - }, - }) - if err == nil && len(first) > 0 { - commitID = first[0].CommitSHA - invalidated = first[0].Invalidated - patch = first[0].Patch - } else if err != nil && !issues_model.IsErrCommentNotExist(err) { - return nil, fmt.Errorf("Find first comment for %d line %d path %s. Error: %w", reviewID, line, treePath, err) - } else { - review, err := issues_model.GetReviewByID(ctx, reviewID) - if err == nil && len(review.CommitID) > 0 { - head = review.CommitID - } else if err != nil && !issues_model.IsErrReviewNotExist(err) { - return nil, fmt.Errorf("GetReviewByID %d. Error: %w", reviewID, err) - } - } - } - - if len(commitID) == 0 { - // FIXME validate treePath - // Get latest commit referencing the commented line - // No need for get commit for base branch changes - commit, err := gitRepo.LineBlame(head, gitRepo.Path, treePath, uint(line)) - if err == nil { - commitID = commit.ID.String() - } else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) { - return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %w", pr.GetGitHeadRefName(), gitRepo.Path, treePath, line, err) - } - } - } - - // Only fetch diff if comment is review comment - if len(patch) == 0 && reviewID != 0 { - headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName()) - if err != nil { - return nil, fmt.Errorf("GetRefCommitID[%s]: %w", pr.GetGitHeadRefName(), err) - } - if len(commitID) == 0 { - commitID = headCommitID - } + patch, err := cache.GetString(patchCacheKey(issue.ID, beforeCommitID, afterCommitID, treePath, line), func() (string, error) { reader, writer := io.Pipe() defer func() { _ = reader.Close() _ = writer.Close() }() go func() { - if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, writer); err != nil { - _ = writer.CloseWithError(fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %w", gitRepo.Path, pr.MergeBase, headCommitID, treePath, err)) + if err := git.GetRepoRawDiffForFile(gitRepo, beforeCommitID, afterCommitID, git.RawDiffNormal, treePath, writer); err != nil { + _ = writer.CloseWithError(fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %w", gitRepo.Path, beforeCommitID, afterCommitID, treePath, err)) return } _ = writer.Close() }() - patch, err = git.CutDiffAroundLine(reader, int64((&issues_model.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) - if err != nil { - log.Error("Error whilst generating patch: %v", err) - return nil, err - } + return git.CutDiffAroundLine(reader, int64((&issues_model.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) + }) + if err != nil { + return nil, fmt.Errorf("GetPatch failed: %w", err) } + + lineCommitID := util.Iif(line < 0, beforeCommitID, afterCommitID) + // TODO: the commit ID Must be referenced in the git repository, because the branch maybe rebased or force-pushed. + + // If the commit ID is not referenced, it cannot be calculated the position dynamically. return issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{ Type: issues_model.CommentTypeCode, Doer: doer, @@ -278,10 +277,10 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo Content: content, LineNum: line, TreePath: treePath, - CommitSHA: commitID, + CommitSHA: lineCommitID, ReviewID: reviewID, Patch: patch, - Invalidated: invalidated, + Invalidated: false, Attachments: attachments, }) } @@ -328,15 +327,13 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos notify_service.PullRequestReview(ctx, pr, review, comm, mentions) - for _, lines := range review.CodeComments { - for _, comments := range lines { - for _, codeComment := range comments { - mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, codeComment.Content) - if err != nil { - return nil, nil, err - } - notify_service.PullRequestCodeComment(ctx, pr, codeComment, mentions) + for _, fileComments := range review.CodeComments { + for _, codeComment := range fileComments { + mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, codeComment.Content) + if err != nil { + return nil, nil, err } + notify_service.PullRequestCodeComment(ctx, pr, codeComment, mentions) } } @@ -471,3 +468,143 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, return comment, nil } + +// ReCalculateLineNumber recalculates the line number based on the hunks of the diff. +// If the returned line number is zero, it should not be displayed. +func ReCalculateLineNumber(hunks []*git.HunkInfo, leftLine int64) int64 { + if len(hunks) == 0 || leftLine == 0 { + return leftLine + } + + isLeft := leftLine < 0 + absLine := leftLine + if isLeft { + absLine = -leftLine + } + newLine := absLine + + for _, hunk := range hunks { + if hunk.LeftLine+hunk.LeftHunk <= absLine { + newLine += hunk.RightHunk - hunk.LeftHunk + } else if hunk.LeftLine <= absLine && absLine < hunk.LeftLine+hunk.LeftHunk { + // The line has been removed, so it should not be displayed + return 0 + } else if absLine < hunk.LeftLine { + // The line is before the hunk, so we can ignore it + continue + } + } + return util.Iif(isLeft, -newLine, newLine) +} + +// FetchCodeCommentsByLine fetches the code comments for a given commit, treePath and line number of a pull request. +func FetchCodeCommentsByLine(ctx context.Context, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, startCommitID, endCommitID, treePath string, line int64, showOutdatedComments bool) (issues_model.CommentList, error) { + opts := issues_model.FindCommentsOptions{ + Type: issues_model.CommentTypeCode, + IssueID: issueID, + TreePath: treePath, + } + // load all the comments on this file and then filter them by line number + // we cannot use the line number in the options because some comments's line number may have changed + comments, err := issues_model.FindCodeComments(ctx, opts, repo, currentUser, nil, showOutdatedComments) + if err != nil { + return nil, fmt.Errorf("FindCodeComments: %w", err) + } + if len(comments) == 0 { + return nil, nil + } + n := 0 + hunksCache := make(map[string][]*git.HunkInfo) + for _, comment := range comments { + if comment.CommitSHA == endCommitID { + if comment.Line == line { + comments[n] = comment + n++ + } + continue + } + + // If the comment is not for the current commit, we need to recalculate the line number + hunks, ok := hunksCache[comment.CommitSHA] + if !ok { + hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), comment.CommitSHA, endCommitID, treePath) + if err != nil { + return nil, fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), comment.CommitSHA, endCommitID, err) + } + hunksCache[comment.CommitSHA] = hunks + } + + comment.Line = ReCalculateLineNumber(hunks, comment.Line) + comments[n] = comment + n++ + } + return comments[:n], nil +} + +// LoadCodeComments loads comments into each line, so that the comments can be displayed in the diff view. +// the comments' line number is recalculated based on the hunks of the diff. +func LoadCodeComments(ctx context.Context, gitRepo *git.Repository, repo *repo_model.Repository, diff *gitdiff.Diff, issueID int64, currentUser *user_model.User, startCommit, endCommit *git.Commit, showOutdatedComments bool) error { + if startCommit == nil || endCommit == nil { + return errors.New("startCommit and endCommit cannot be nil") + } + + allComments, err := issues_model.FetchCodeComments(ctx, repo, issueID, currentUser, showOutdatedComments) + if err != nil { + return err + } + + for _, file := range diff.Files { + if fileComments, ok := allComments[file.Name]; ok { + lineComments := make(map[int64][]*issues_model.Comment) + hunksCache := make(map[string][]*git.HunkInfo) + // filecomments should be sorted by created time, so that the latest comments are at the end + for _, comment := range fileComments { + dstCommitID := startCommit.ID.String() + if comment.Line > 0 { + dstCommitID = endCommit.ID.String() + } + + if comment.CommitSHA == dstCommitID { + lineComments[comment.Line] = append(lineComments[comment.Line], comment) + continue + } + + // If the comment is not for the current commit, we need to recalculate the line number + hunks, ok := hunksCache[comment.CommitSHA+".."+dstCommitID] + if !ok { + hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), comment.CommitSHA, dstCommitID, file.Name) + if err != nil { + return fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), dstCommitID, comment.CommitSHA, err) + } + hunksCache[comment.CommitSHA+".."+dstCommitID] = hunks + } + comment.Line = ReCalculateLineNumber(hunks, comment.Line) + if comment.Line != 0 { + dstCommit, err := gitRepo.GetCommit(dstCommitID) + if err != nil { + return fmt.Errorf("GetCommit[%s]: %w", dstCommitID, err) + } + // If the comment is not the first one or the comment created before the current commit + if len(lineComments[comment.Line]) > 0 || comment.CreatedUnix.AsTime().Before(dstCommit.Committer.When) { + lineComments[comment.Line] = append(lineComments[comment.Line], comment) + } + } + } + + for _, section := range file.Sections { + for _, line := range section.Lines { + if comments, ok := lineComments[int64(line.LeftIdx*-1)]; ok { + line.Comments = append(line.Comments, comments...) + } + if comments, ok := lineComments[int64(line.RightIdx)]; ok { + line.Comments = append(line.Comments, comments...) + } + sort.SliceStable(line.Comments, func(i, j int) bool { + return line.Comments[i].CreatedUnix < line.Comments[j].CreatedUnix + }) + } + } + } + } + return nil +} diff --git a/services/pull/review_test.go b/services/pull/review_test.go index 3bce1e523d7..80cc78bcbe6 100644 --- a/services/pull/review_test.go +++ b/services/pull/review_test.go @@ -10,6 +10,9 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/services/gitdiff" pull_service "code.gitea.io/gitea/services/pull" "github.com/stretchr/testify/assert" @@ -46,3 +49,89 @@ func TestDismissReview(t *testing.T) { assert.Error(t, err) assert.True(t, pull_service.IsErrDismissRequestOnClosedPR(err)) } + +func setupDefaultDiff() *gitdiff.Diff { + return &gitdiff.Diff{ + Files: []*gitdiff.DiffFile{ + { + Name: "README.md", + Sections: []*gitdiff.DiffSection{ + { + Lines: []*gitdiff.DiffLine{ + { + LeftIdx: 4, + RightIdx: 4, + }, + }, + }, + }, + }, + }, + } +} + +func TestDiff_LoadCommentsNoOutdated(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + diff := setupDefaultDiff() + assert.NoError(t, issue.LoadRepo(t.Context())) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + + gitRepo, err := gitrepo.OpenRepository(t.Context(), issue.Repo) + assert.NoError(t, err) + defer gitRepo.Close() + startCommit, err := gitRepo.GetCommit(issue.PullRequest.MergeBase) + assert.NoError(t, err) + endCommit, err := gitRepo.GetCommit(issue.PullRequest.GetGitHeadRefName()) + assert.NoError(t, err) + + assert.NoError(t, pull_service.LoadCodeComments(db.DefaultContext, gitRepo, issue.Repo, diff, issue.ID, user, startCommit, endCommit, false)) + assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 2) +} + +func TestDiff_LoadCommentsWithOutdated(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.NoError(t, issue.LoadRepo(t.Context())) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + + diff := setupDefaultDiff() + gitRepo, err := gitrepo.OpenRepository(t.Context(), issue.Repo) + assert.NoError(t, err) + defer gitRepo.Close() + startCommit, err := gitRepo.GetCommit(issue.PullRequest.MergeBase) + assert.NoError(t, err) + endCommit, err := gitRepo.GetCommit(issue.PullRequest.GetGitHeadRefName()) + assert.NoError(t, err) + + assert.NoError(t, pull_service.LoadCodeComments(db.DefaultContext, gitRepo, issue.Repo, diff, issue.ID, user, startCommit, endCommit, true)) + assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 3) +} + +func Test_reCalculateLineNumber(t *testing.T) { + hunks := []*git.HunkInfo{ + { + LeftLine: 0, + LeftHunk: 0, + RightLine: 1, + RightHunk: 3, + }, + } + assert.EqualValues(t, 6, pull_service.ReCalculateLineNumber(hunks, 3)) + + hunks = []*git.HunkInfo{ + { + LeftLine: 1, + LeftHunk: 4, + RightLine: 1, + RightHunk: 4, + }, + } + assert.EqualValues(t, 0, pull_service.ReCalculateLineNumber(hunks, 4)) + assert.EqualValues(t, 5, pull_service.ReCalculateLineNumber(hunks, 5)) + assert.EqualValues(t, 0, pull_service.ReCalculateLineNumber(hunks, -1)) +} diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 22abf9a2193..ade1da655a0 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -184,7 +184,7 @@ {{end}}
{{else}} - +
{{if $.IsSplitStyle}} {{template "repo/diff/section_split" dict "file" . "root" $}} {{else}} diff --git a/templates/repo/diff/comment_form.tmpl b/templates/repo/diff/comment_form.tmpl index 58b675467c0..f701c700fc9 100644 --- a/templates/repo/diff/comment_form.tmpl +++ b/templates/repo/diff/comment_form.tmpl @@ -2,7 +2,8 @@ {{$.root.CsrfTokenHtml}} - + + diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 089cdf2ccdd..ea616b355b0 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -448,10 +448,8 @@ {{if and .Review .Review.CodeComments}}
- {{range $filename, $lines := .Review.CodeComments}} - {{range $line, $comms := $lines}} - {{template "repo/issue/view_content/conversation" dict "." $ "comments" $comms}} - {{end}} + {{range $filename, $comms := .Review.CodeComments}} + {{template "repo/issue/view_content/conversation" dict "." $ "comments" $comms}} {{end}}
{{end}}