improvements

This commit is contained in:
Lunny Xiao 2025-07-14 09:58:28 -07:00
parent 345ae04837
commit f8cea67265
6 changed files with 68 additions and 28 deletions

View File

@ -258,11 +258,7 @@ func (c *Commit) CommitsBeforeLimit(num int) ([]*Commit, error) {
// CommitsBeforeUntil returns the commits between commitID to current revision // CommitsBeforeUntil returns the commits between commitID to current revision
func (c *Commit) CommitsBeforeUntil(commitID string) ([]*Commit, error) { func (c *Commit) CommitsBeforeUntil(commitID string) ([]*Commit, error) {
endCommit, err := c.repo.GetCommit(commitID) return c.repo.CommitsBetween(c.ID.String(), commitID)
if err != nil {
return nil, err
}
return c.repo.CommitsBetween(c, endCommit)
} }
// SearchCommitsOptions specify the parameters for SearchCommits // SearchCommitsOptions specify the parameters for SearchCommits

View File

@ -15,7 +15,6 @@ import (
"strings" "strings"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"
) )
// RawDiffType type of a raw diff. // RawDiffType type of a raw diff.
@ -109,7 +108,7 @@ func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightH
if len(leftRange) > 1 { if len(leftRange) > 1 {
leftHunk, _ = strconv.Atoi(leftRange[1]) leftHunk, _ = strconv.Atoi(leftRange[1])
} else { } else {
leftHunk = util.Iif(leftLine > 0, leftLine, -leftLine) leftHunk = 1
} }
if len(ranges) > 1 { if len(ranges) > 1 {
rightRange := strings.Split(ranges[1], ",") rightRange := strings.Split(ranges[1], ",")
@ -117,7 +116,7 @@ func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightH
if len(rightRange) > 1 { if len(rightRange) > 1 {
rightHunk, _ = strconv.Atoi(rightRange[1]) rightHunk, _ = strconv.Atoi(rightRange[1])
} else { } else {
rightHunk = rightLine rightHunk = 1
} }
} else { } else {
log.Debug("Parse line number failed: %v", diffHunk) log.Debug("Parse line number failed: %v", diffHunk)

View File

@ -185,6 +185,12 @@ func TestParseDiffHunkString(t *testing.T) {
leftLine, leftHunk, rightLine, rightHunk = ParseDiffHunkString("@@ -1 +0,0 @@") leftLine, leftHunk, rightLine, rightHunk = ParseDiffHunkString("@@ -1 +0,0 @@")
assert.Equal(t, 1, leftLine) assert.Equal(t, 1, leftLine)
assert.Equal(t, 1, leftHunk) assert.Equal(t, 1, leftHunk)
assert.Equal(t, 0, rightLine) assert.Equal(t, 1, rightLine)
assert.Equal(t, 0, rightHunk) assert.Equal(t, 0, rightHunk)
leftLine, leftHunk, rightLine, rightHunk = ParseDiffHunkString("@@ -2 +2 @@")
assert.Equal(t, 2, leftLine)
assert.Equal(t, 1, leftHunk)
assert.Equal(t, 2, rightLine)
assert.Equal(t, 1, rightHunk)
} }

View File

@ -6,6 +6,7 @@ package git
import ( import (
"bytes" "bytes"
"context"
"io" "io"
"os" "os"
"strconv" "strconv"
@ -304,23 +305,47 @@ func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (in
// CommitsBetween returns a list that contains commits between [before, last). // CommitsBetween returns a list that contains commits between [before, last).
// If before is detached (removed by reset + push) it is not included. // If before is detached (removed by reset + push) it is not included.
func (repo *Repository) CommitsBetween(last, before *Commit) ([]*Commit, error) { func (repo *Repository) CommitsBetween(lastCommitID, beforeCommitID string) ([]*Commit, error) {
commitIDs, err := CommitIDsBetween(repo.Ctx, repo.Path, beforeCommitID, lastCommitID)
if err != nil {
return nil, err
}
commits := make([]*Commit, 0, len(commitIDs))
for _, commitID := range commitIDs {
commit, err := repo.GetCommit(commitID)
if err != nil {
return nil, err
}
commits = append(commits, commit)
}
return commits, nil
}
func CommitIDsBetween(ctx context.Context, repoPath, beforeCommitID, afterCommitID string) ([]string, error) {
var stdout []byte var stdout []byte
var err error var err error
if before == nil { if beforeCommitID == "" {
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(last.ID.String()).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path}) stdout, _, err = NewCommand("rev-list").AddDynamicArguments(afterCommitID).RunStdBytes(ctx, &RunOpts{Dir: repoPath})
} else { } else {
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(before.ID.String()+".."+last.ID.String()).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path}) stdout, _, err = NewCommand("rev-list").AddDynamicArguments(beforeCommitID+".."+afterCommitID).RunStdBytes(ctx, &RunOpts{Dir: repoPath})
if err != nil && strings.Contains(err.Error(), "no merge base") { if err != nil && strings.Contains(err.Error(), "no merge base") {
// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated. // future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
// previously it would return the results of git rev-list before last so let's try that... // previously it would return the results of git rev-list before last so let's try that...
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(before.ID.String(), last.ID.String()).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path}) stdout, _, err = NewCommand("rev-list").AddDynamicArguments(beforeCommitID, afterCommitID).RunStdBytes(ctx, &RunOpts{Dir: repoPath})
} }
} }
if err != nil { if err != nil {
return nil, err return nil, err
} }
return repo.parsePrettyFormatLogToList(bytes.TrimSpace(stdout))
commitIDs := make([]string, 0, 10)
for commitID := range bytes.SplitSeq(stdout, []byte{'\n'}) {
if len(commitID) > 0 {
commitIDs = append(commitIDs, string(commitID))
}
}
return commitIDs, nil
} }
// CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [before, last) // CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [before, last)
@ -375,18 +400,17 @@ func (repo *Repository) CommitsBetweenNotBase(last, before *Commit, baseBranch s
// CommitsBetweenIDs return commits between twoe commits // CommitsBetweenIDs return commits between twoe commits
func (repo *Repository) CommitsBetweenIDs(last, before string) ([]*Commit, error) { func (repo *Repository) CommitsBetweenIDs(last, before string) ([]*Commit, error) {
lastCommit, err := repo.GetCommit(last) _, err := repo.GetCommit(last)
if err != nil { if err != nil {
return nil, err return nil, err
} }
if before == "" { if before != "" {
return repo.CommitsBetween(lastCommit, nil) _, err := repo.GetCommit(before)
if err != nil {
return nil, err
}
} }
beforeCommit, err := repo.GetCommit(before) return repo.CommitsBetween(last, before)
if err != nil {
return nil, err
}
return repo.CommitsBetween(lastCommit, beforeCommit)
} }
// CommitsCountBetween return numbers of commits between two commits // CommitsCountBetween return numbers of commits between two commits

View File

@ -134,6 +134,8 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion
func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error { func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error {
err := db.WithTx(ctx, func(ctx context.Context) error { err := db.WithTx(ctx, func(ctx context.Context) error {
return issues_model.DeleteComment(ctx, comment) return issues_model.DeleteComment(ctx, comment)
// TODO: delete review if the comment is the last comment of the review
// TODO: delete comment attachments
}) })
if err != nil { if err != nil {
return err return err

View File

@ -10,6 +10,7 @@ import (
"fmt" "fmt"
"io" "io"
"regexp" "regexp"
"slices"
"sort" "sort"
"strings" "strings"
@ -111,14 +112,23 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
defer closer.Close() defer closer.Close()
} }
if beforeCommitID == "" { prCommitIDs, err := git.CommitIDsBetween(ctx, gitRepo.Path, beforeCommitID, afterCommitID)
if err != nil {
return nil, fmt.Errorf("CommitIDsBetween[%s, %s]: %w", beforeCommitID, afterCommitID, err)
}
if beforeCommitID == "" || beforeCommitID == issue.PullRequest.MergeBase {
beforeCommitID = issue.PullRequest.MergeBase beforeCommitID = issue.PullRequest.MergeBase
} else { } else {
beforeCommit, err := gitRepo.GetCommit(beforeCommitID) // Ensure beforeCommitID is valid // beforeCommitID must be one of the pull request commits
if !slices.Contains(prCommitIDs, beforeCommitID) {
return nil, fmt.Errorf("beforeCommitID[%s] is not a valid pull request commit", beforeCommitID)
}
beforeCommit, err := gitRepo.GetCommit(beforeCommitID)
if err != nil { if err != nil {
return nil, fmt.Errorf("GetCommit[%s]: %w", beforeCommitID, err) return nil, fmt.Errorf("GetCommit[%s]: %w", beforeCommitID, err)
} }
// TODO: beforeCommitID must be one of the pull request commits
beforeCommit, err = beforeCommit.Parent(0) beforeCommit, err = beforeCommit.Parent(0)
if err != nil { if err != nil {
@ -131,8 +141,11 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
if err != nil { if err != nil {
return nil, fmt.Errorf("GetRefCommitID[%s]: %w", issue.PullRequest.GetGitHeadRefName(), err) return nil, fmt.Errorf("GetRefCommitID[%s]: %w", issue.PullRequest.GetGitHeadRefName(), err)
} }
} else { //nolint } else {
// TODO: afterCommitID must be one of the pull request commits // afterCommitID must be one of the pull request commits
if !slices.Contains(prCommitIDs, afterCommitID) {
return nil, fmt.Errorf("afterCommitID[%s] is not a valid pull request commit", afterCommitID)
}
} }
// CreateCodeComment() is used for: // CreateCodeComment() is used for: