-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move git diff codes from models to services/gitdiff #7889
Changes from 4 commits
20931e3
bc75226
a16cf04
a805dde
09601e4
0c3acf6
d36ec0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
// Copyright 2019 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package comments | ||
|
||
import ( | ||
"bytes" | ||
"fmt" | ||
"strings" | ||
|
||
"code.gitea.io/gitea/models" | ||
"code.gitea.io/gitea/modules/git" | ||
"code.gitea.io/gitea/modules/setting" | ||
"code.gitea.io/gitea/services/gitdiff" | ||
) | ||
|
||
// CreateCodeComment creates a plain code comment at the specified line / path | ||
func CreateCodeComment(doer *models.User, repo *models.Repository, issue *models.Issue, content, treePath string, line, reviewID int64) (*models.Comment, error) { | ||
var commitID, patch string | ||
pr, err := models.GetPullRequestByIssueID(issue.ID) | ||
if err != nil { | ||
return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err) | ||
} | ||
if err := pr.GetBaseRepo(); err != nil { | ||
return nil, fmt.Errorf("GetHeadRepo: %v", err) | ||
} | ||
gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) | ||
if err != nil { | ||
return nil, fmt.Errorf("OpenRepository: %v", err) | ||
} | ||
|
||
// FIXME validate treePath | ||
// Get latest commit referencing the commented line | ||
// No need for get commit for base branch changes | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'No need to get ... or 'No need of getting...'? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just move them and haven't see it's correct. |
||
if line > 0 { | ||
commit, err := gitRepo.LineBlame(pr.GetGitRefName(), 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") { | ||
return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err) | ||
} | ||
} | ||
|
||
// Only fetch diff if comment is review comment | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Under what circumstances There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just move them and haven't see it's correct. |
||
if reviewID != 0 { | ||
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) | ||
if err != nil { | ||
return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err) | ||
} | ||
patchBuf := new(bytes.Buffer) | ||
if err := gitdiff.GetRawDiffForFile(gitRepo.Path, pr.MergeBase, headCommitID, gitdiff.RawDiffNormal, treePath, patchBuf); err != nil { | ||
return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath) | ||
} | ||
patch = gitdiff.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) | ||
} | ||
return models.CreateComment(&models.CreateCommentOptions{ | ||
Type: models.CommentTypeCode, | ||
Doer: doer, | ||
Repo: repo, | ||
Issue: issue, | ||
Content: content, | ||
LineNum: line, | ||
TreePath: treePath, | ||
CommitSHA: commitID, | ||
ReviewID: reviewID, | ||
Patch: patch, | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeComment
is a bit too general. It could mean comments in the source code at first glance.Maybe
ReviewComment
if there are no other types of comments?If renaming it would involve extensive changes beyond this PR, no need to bother now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to leave it for another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lunny By the way, after realizing it's already there and this is just a refactor. I think it is not worth the effort to change the name at this point.