Merge ff7a6616fb1311e0e8d5a623d65c57283bb76349 into 54fe47fbca4023061a657bd54425c17b8667c5d2

models/issues/comment.go

@@ -293,7 +293,8 @@ type Comment struct {
 	UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
 
 	// Reference issue in commit message
-	CommitSHA string `xorm:"VARCHAR(64)"`
+	BeforeCommitID string `xorm:"VARCHAR(64)"`
+	CommitSHA      string `xorm:"VARCHAR(64)"`
 
 	Attachments []*repo_model.Attachment `xorm:"-"`
 	Reactions   ReactionList             `xorm:"-"`
@@ -764,6 +765,10 @@ func (c *Comment) CodeCommentLink(ctx context.Context) string {
 	return fmt.Sprintf("%s/files#%s", c.Issue.Link(), c.HashTag())
 }
 
+func GetCodeCommentRefName(prIndex, commentID int64, suffix string) string {
+	return fmt.Sprintf("refs/pull/%d/code-comment-%d", prIndex, commentID)
+}
+
 // CreateComment creates comment with context
 func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, err error) {
 	return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) {
@@ -796,6 +801,7 @@ func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment,
 			AssigneeID:       opts.AssigneeID,
 			AssigneeTeamID:   opts.AssigneeTeamID,
 			CommitID:         opts.CommitID,
+			BeforeCommitID:   opts.BeforeCommitID,
 			CommitSHA:        opts.CommitSHA,
 			Line:             opts.LineNum,
 			Content:          opts.Content,
@@ -965,7 +971,8 @@ type CreateCommentOptions struct {
 	OldRef             string
 	NewRef             string
 	CommitID           int64
-	CommitSHA          string
+	BeforeCommitID     string // before commit id when creating this code comment
+	CommitSHA          string // after commit id when creating this code comment, ref commit id for other comment
 	Patch              string
 	LineNum            int64
 	TreePath           string

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 (cc CodeComments) AllComments() []*Comment {
-func FetchCodeComments(ctx context.Context, issue *Issue, currentUser *user_model.User, showOutdatedComments bool) (CodeComments, error) {
+	var allComments []*Comment
-	return fetchCodeCommentsByReview(ctx, issue, currentUser, nil, showOutdatedComments)
+	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) {
+// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line
-	pathToLineToComment := make(CodeComments)
+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 {
+		codeCommentsPathMap[comment.TreePath] = append(codeCommentsPathMap[comment.TreePath], comment)
-			pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment)
-		}
-		pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], 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)
-}

models/issues/comment_test.go

@@ -68,15 +68,20 @@ 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))
+	fourthLineComments := []*issues_model.Comment{}
-	assert.Len(t, res["README.md"][4], 1)
+	for _, comment := range res["README.md"] {
-	assert.Equal(t, int64(4), res["README.md"][4][0].ID)
+		if comment.Line == 4 {
+			fourthLineComments = append(fourthLineComments, comment)
+		}
+	}
+	assert.Len(t, fourthLineComments, 1)
+	assert.Equal(t, int64(4), fourthLineComments[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)
 }

models/issues/issue.go

@@ -223,7 +223,7 @@ func (issue *Issue) LoadPullRequest(ctx context.Context) (err error) {
 	return nil
 }
 
-func (issue *Issue) loadComments(ctx context.Context) (err error) {
+func (issue *Issue) LoadComments(ctx context.Context) (err error) {
 	return issue.loadCommentsByType(ctx, CommentTypeUndefined)
 }
 
@@ -344,7 +344,7 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
 		return err
 	}
 
-	if err = issue.loadComments(ctx); err != nil {
+	if err = issue.LoadComments(ctx); err != nil {
 		return err
 	}
 

models/issues/pull.go

@@ -15,7 +15,6 @@ import (
 	"code.gitea.io/gitea/models/db"
 	git_model "code.gitea.io/gitea/models/git"
 	org_model "code.gitea.io/gitea/models/organization"
-	pull_model "code.gitea.io/gitea/models/pull"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
@@ -156,26 +155,6 @@ func init() {
 	db.RegisterModel(new(PullRequest))
 }
 
-// DeletePullsByBaseRepoID deletes all pull requests by the base repository ID
-func DeletePullsByBaseRepoID(ctx context.Context, repoID int64) error {
-	deleteCond := builder.Select("id").From("pull_request").Where(builder.Eq{"pull_request.base_repo_id": repoID})
-
-	// Delete scheduled auto merges
-	if _, err := db.GetEngine(ctx).In("pull_id", deleteCond).
-		Delete(&pull_model.AutoMerge{}); err != nil {
-		return err
-	}
-
-	// Delete review states
-	if _, err := db.GetEngine(ctx).In("pull_id", deleteCond).
-		Delete(&pull_model.ReviewState{}); err != nil {
-		return err
-	}
-
-	_, err := db.DeleteByBean(ctx, &PullRequest{BaseRepoID: repoID})
-	return err
-}
-
 func (pr *PullRequest) String() string {
 	if pr == nil {
 		return "<PullRequest nil>"
@@ -406,11 +385,16 @@ func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer)
 	return nil
 }
 
-// GetGitHeadRefName returns git ref for hidden pull request branch
+// GetGitHeadRefName returns git head commit id ref for the pull request's branch
 func (pr *PullRequest) GetGitHeadRefName() string {
 	return fmt.Sprintf("%s%d/head", git.PullPrefix, pr.Index)
 }
 
+// GetGitMergeRefName returns git merged commit id ref for the pull request
+func (pr *PullRequest) GetGitMergeRefName() string {
+	return fmt.Sprintf("%s%d/merge", git.PullPrefix, pr.Index)
+}
+
 func (pr *PullRequest) GetGitHeadBranchRefName() string {
 	return fmt.Sprintf("%s%s", git.BranchPrefix, pr.HeadBranch)
 }

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
 }
 
@@ -429,6 +429,10 @@ func SubmitReview(ctx context.Context, doer *user_model.User, issue *Issue, revi
 	defer committer.Close()
 	sess := db.GetEngine(ctx)
 
+	if err := issue.LoadRepo(ctx); err != nil {
+		return nil, nil, fmt.Errorf("LoadRepo: %w", err)
+	}
+
 	official := false
 
 	review, err := GetCurrentReview(ctx, doer, issue)

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) {

models/migrations/migrations.go

@@ -386,6 +386,7 @@ func prepareMigrationTasks() []*migration {
 
 		// Gitea 1.24.0 ends at database version 321
 		newMigration(321, "Use LONGTEXT for some columns and fix review_state.updated_files column", v1_25.UseLongTextInSomeColumnsAndFixBugs),
+		newMigration(322, "Add BeforeCommitID to Comment table", v1_25.AddBeforeCommitIDForComment),
 	}
 	return preparedMigrations
 }

models/migrations/v1_25/v322.go

@@ -0,0 +1,28 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_25
+
+import (
+	"xorm.io/xorm"
+)
+
+type comment struct {
+	BeforeCommitID string `xorm:"VARCHAR(64)"`
+}
+
+// TableName return database table name for xorm
+func (comment) TableName() string {
+	return "comment"
+}
+
+func AddBeforeCommitIDForComment(x *xorm.Engine) error {
+	if _, err := x.SyncWithOptions(xorm.SyncOptions{
+		IgnoreConstrains: true,
+		IgnoreIndices:    true,
+	}, new(comment)); err != nil {
+		return err
+	}
+	_, err := x.Exec("UPDATE comment SET before_commit_id = (SELECT merge_base FROM pull_request WHERE pull_request.issue_id = comment.issue_id) WHERE `type`=21 AND before_commit_id IS NULL")
+	return err
+}

models/migrations/v1_25/v322_test.go

@@ -0,0 +1,103 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_25
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/migrations/base"
+	"code.gitea.io/gitea/modules/references"
+	"code.gitea.io/gitea/modules/timeutil"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_AddBeforeCommitIDForComment(t *testing.T) {
+	type Comment struct { // old struct
+		ID               int64 `xorm:"pk autoincr"`
+		Type             int   `xorm:"INDEX"`
+		PosterID         int64 `xorm:"INDEX"`
+		OriginalAuthor   string
+		OriginalAuthorID int64
+		IssueID          int64 `xorm:"INDEX"`
+		LabelID          int64
+		OldProjectID     int64
+		ProjectID        int64
+		OldMilestoneID   int64
+		MilestoneID      int64
+		TimeID           int64
+		AssigneeID       int64
+		RemovedAssignee  bool
+		AssigneeTeamID   int64 `xorm:"NOT NULL DEFAULT 0"`
+		ResolveDoerID    int64
+		OldTitle         string
+		NewTitle         string
+		OldRef           string
+		NewRef           string
+		DependentIssueID int64 `xorm:"index"` // This is used by issue_service.deleteIssue
+
+		CommitID       int64
+		Line           int64 // - previous line / + proposed line
+		TreePath       string
+		Content        string `xorm:"LONGTEXT"`
+		ContentVersion int    `xorm:"NOT NULL DEFAULT 0"`
+
+		// Path represents the 4 lines of code cemented by this comment
+		Patch       string `xorm:"-"`
+		PatchQuoted string `xorm:"LONGTEXT patch"`
+
+		CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
+		UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
+
+		// Reference issue in commit message
+		CommitSHA string `xorm:"VARCHAR(64)"`
+
+		ReviewID    int64 `xorm:"index"`
+		Invalidated bool
+
+		// Reference an issue or pull from another comment, issue or PR
+		// All information is about the origin of the reference
+		RefRepoID    int64                 `xorm:"index"` // Repo where the referencing
+		RefIssueID   int64                 `xorm:"index"`
+		RefCommentID int64                 `xorm:"index"`    // 0 if origin is Issue title or content (or PR's)
+		RefAction    references.XRefAction `xorm:"SMALLINT"` // What happens if RefIssueID resolves
+		RefIsPull    bool
+
+		CommentMetaData string `xorm:"JSON TEXT"` // put all non-index metadata in a single field
+	}
+
+	type PullRequest struct {
+		ID              int64 `xorm:"pk autoincr"`
+		Type            int
+		Status          int
+		ConflictedFiles []string `xorm:"TEXT JSON"`
+		CommitsAhead    int
+		CommitsBehind   int
+
+		ChangedProtectedFiles []string `xorm:"TEXT JSON"`
+
+		IssueID int64 `xorm:"INDEX"`
+		Index   int64
+
+		HeadRepoID          int64 `xorm:"INDEX"`
+		BaseRepoID          int64 `xorm:"INDEX"`
+		HeadBranch          string
+		BaseBranch          string
+		MergeBase           string `xorm:"VARCHAR(64)"`
+		AllowMaintainerEdit bool   `xorm:"NOT NULL DEFAULT false"`
+
+		HasMerged      bool               `xorm:"INDEX"`
+		MergedCommitID string             `xorm:"VARCHAR(64)"`
+		MergerID       int64              `xorm:"INDEX"`
+		MergedUnix     timeutil.TimeStamp `xorm:"updated INDEX"`
+
+		Flow int `xorm:"NOT NULL DEFAULT 0"`
+	}
+
+	// Prepare and load the testing database
+	x, deferable := base.PrepareTestEnv(t, 0, new(Comment), new(PullRequest))
+	defer deferable()
+
+	assert.NoError(t, AddBeforeCommitIDForComment(x))
+}

modules/git/commit.go

@@ -258,11 +258,7 @@ func (c *Commit) CommitsBeforeLimit(num int) ([]*Commit, error) {
 
 // CommitsBeforeUntil returns the commits between commitID to current revision
 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

modules/git/diff.go

@@ -107,12 +107,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 = 1
 	}
 	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 = 1
 		}
 	} else {
 		log.Debug("Parse line number failed: %v", diffHunk)
@@ -342,3 +346,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(hunks, &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
+}

modules/git/diff_test.go

@@ -4,6 +4,7 @@
 package git
 
 import (
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -181,4 +182,28 @@ 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, 1, rightLine)
+	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)
+}
+
+func Test_GetAffectedHunksForTwoCommitsSpecialFile(t *testing.T) {
+	repoPath := filepath.Join(testReposDir, "repo4_commitsbetween")
+	hunks, err := GetAffectedHunksForTwoCommitsSpecialFile(t.Context(), repoPath, "fdc1b615bdcff0f0658b216df0c9209e5ecb7c78", "a78e5638b66ccfe7e1b4689d3d5684e42c97d7ca", "test.txt")
+	assert.NoError(t, err)
+	assert.Len(t, hunks, 1)
+	// @@ -1 +1 @@
+	assert.Equal(t, int64(1), hunks[0].LeftLine)
+	assert.Equal(t, int64(1), hunks[0].LeftHunk)
+	assert.Equal(t, int64(1), hunks[0].RightLine)
+	assert.Equal(t, int64(1), hunks[0].RightHunk)
 }

modules/git/ref.go

@@ -4,6 +4,7 @@
 package git
 
 import (
+	"context"
 	"regexp"
 	"strings"
 
@@ -220,3 +221,14 @@ func (ref RefName) RefWebLinkPath() string {
 	}
 	return string(refType) + "/" + util.PathEscapeSegments(ref.ShortName())
 }
+
+func UpdateRef(ctx context.Context, repoPath, refName, newCommitID string) error {
+	_, _, err := NewCommand("update-ref").AddDynamicArguments(refName, newCommitID).RunStdString(ctx, &RunOpts{Dir: repoPath})
+	return err
+}
+
+func RemoveRef(ctx context.Context, repoPath, refName string) error {
+	_, _, err := NewCommand("update-ref", "--no-deref", "-d").
+		AddDynamicArguments(refName).RunStdString(ctx, &RunOpts{Dir: repoPath})
+	return err
+}

modules/git/repo_commit.go

@@ -6,6 +6,7 @@ package git
 
 import (
 	"bytes"
+	"context"
 	"io"
 	"os"
 	"strconv"
@@ -304,23 +305,50 @@ func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (in
 
 // CommitsBetween returns a list that contains commits between [before, last).
 // 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
+}
+
+// CommitIDsBetween returns a list that contains commit IDs between (beforeCommitID, afterCommitID].
+// If beforeCommitID is empty, it will return all commits before afterCommitID.
+// If beforeCommitID is given, it will not be included in the result.
+func CommitIDsBetween(ctx context.Context, repoPath, beforeCommitID, afterCommitID string) ([]string, error) {
 	var stdout []byte
 	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 {
-		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") {
 			// 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...
-			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 {
 		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)
@@ -375,18 +403,17 @@ func (repo *Repository) CommitsBetweenNotBase(last, before *Commit, baseBranch s
 
 // CommitsBetweenIDs return commits between twoe commits
 func (repo *Repository) CommitsBetweenIDs(last, before string) ([]*Commit, error) {
-	lastCommit, err := repo.GetCommit(last)
+	_, err := repo.GetCommit(last)
 	if err != nil {
 		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

modules/git/repo_commit_nogogit.go

@@ -50,18 +50,6 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
 	return string(shaBs), nil
 }
 
-// SetReference sets the commit ID string of given reference (e.g. branch or tag).
-func (repo *Repository) SetReference(name, commitID string) error {
-	_, _, err := NewCommand("update-ref").AddDynamicArguments(name, commitID).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path})
-	return err
-}
-
-// RemoveReference removes the given reference (e.g. branch or tag).
-func (repo *Repository) RemoveReference(name string) error {
-	_, _, err := NewCommand("update-ref", "--no-deref", "-d").AddDynamicArguments(name).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path})
-	return err
-}
-
 // IsCommitExist returns true if given commit exists in current repository.
 func (repo *Repository) IsCommitExist(name string) bool {
 	if err := ensureValidGitRepository(repo.Ctx, repo.Path); err != nil {

modules/git/repo_commit_test.go

@@ -148,3 +148,26 @@ func TestCommitsByFileAndRange(t *testing.T) {
 	require.NoError(t, err)
 	assert.Len(t, commits, 1)
 }
+
+func Test_CommitIDsBetween(t *testing.T) {
+	defer test.MockVariableValue(&setting.Git.CommitsRangeSize, 2)()
+
+	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
+	bareRepo1, err := openRepositoryWithDefaultContext(bareRepo1Path)
+	require.NoError(t, err)
+	defer bareRepo1.Close()
+
+	// Test with empty beforeCommitID
+	commitIDs, err := CommitIDsBetween(bareRepo1.Ctx, bareRepo1.Path, "", "master")
+	require.NoError(t, err)
+	assert.Len(t, commitIDs, 7)
+	assert.Equal(t, "ce064814f4a0d337b333e646ece456cd39fab612", commitIDs[0])
+	assert.Equal(t, "95bb4d39648ee7e325106df01a621c530863a653", commitIDs[6])
+
+	// Test with a specific beforeCommitID
+	commitIDs, err = CommitIDsBetween(bareRepo1.Ctx, bareRepo1.Path, "37991dec2c8e592043f47155ce4808d4580f9123", "master")
+	require.NoError(t, err)
+	assert.Len(t, commitIDs, 2)
+	assert.Equal(t, "ce064814f4a0d337b333e646ece456cd39fab612", commitIDs[0])
+	assert.Equal(t, "feaf4ba6bc635fec442f46ddd4512416ec43c2c2", commitIDs[1])
+}

modules/git/repo_compare.go

@@ -5,14 +5,10 @@
 package git
 
 import (
-	"bufio"
 	"bytes"
 	"context"
-	"errors"
 	"fmt"
 	"io"
-	"os"
-	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -193,8 +189,6 @@ func GetDiffShortStatByCmdArgs(ctx context.Context, repoPath string, trustedArgs
 var shortStatFormat = regexp.MustCompile(
 	`\s*(\d+) files? changed(?:, (\d+) insertions?\(\+\))?(?:, (\d+) deletions?\(-\))?`)
 
-var patchCommits = regexp.MustCompile(`^From\s(\w+)\s`)
-
 func parseDiffStat(stdout string) (numFiles, totalAdditions, totalDeletions int, err error) {
 	if len(stdout) == 0 || stdout == "\n" {
 		return 0, 0, 0, nil
@@ -282,25 +276,3 @@ func (repo *Repository) GetFilesChangedBetween(base, head string) ([]string, err
 
 	return split, err
 }
-
-// ReadPatchCommit will check if a diff patch exists and return stats
-func (repo *Repository) ReadPatchCommit(prID int64) (commitSHA string, err error) {
-	// Migrated repositories download patches to "pulls" location
-	patchFile := fmt.Sprintf("pulls/%d.patch", prID)
-	loadPatch, err := os.Open(filepath.Join(repo.Path, patchFile))
-	if err != nil {
-		return "", err
-	}
-	defer loadPatch.Close()
-	// Read only the first line of the patch - usually it contains the first commit made in patch
-	scanner := bufio.NewScanner(loadPatch)
-	scanner.Scan()
-	// Parse the Patch stats, sometimes Migration returns a 404 for the patch file
-	commitSHAGroups := patchCommits.FindStringSubmatch(scanner.Text())
-	if len(commitSHAGroups) != 0 {
-		commitSHA = commitSHAGroups[1]
-	} else {
-		return "", errors.New("patch file doesn't contain valid commit ID")
-	}
-	return commitSHA, nil
-}

modules/git/repo_compare_test.go

@@ -45,36 +45,6 @@ func TestGetFormatPatch(t *testing.T) {
 	assert.Contains(t, patch, "Subject: [PATCH] Add file2.txt")
 }
 
-func TestReadPatch(t *testing.T) {
-	// Ensure we can read the patch files
-	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
-	repo, err := openRepositoryWithDefaultContext(bareRepo1Path)
-	if err != nil {
-		assert.NoError(t, err)
-		return
-	}
-	defer repo.Close()
-	// This patch doesn't exist
-	noFile, err := repo.ReadPatchCommit(0)
-	assert.Error(t, err)
-
-	// This patch is an empty one (sometimes it's a 404)
-	noCommit, err := repo.ReadPatchCommit(1)
-	assert.Error(t, err)
-
-	// This patch is legit and should return a commit
-	oldCommit, err := repo.ReadPatchCommit(2)
-	if err != nil {
-		assert.NoError(t, err)
-		return
-	}
-
-	assert.Empty(t, noFile)
-	assert.Empty(t, noCommit)
-	assert.Len(t, oldCommit, 40)
-	assert.Equal(t, "6e8e2a6f9efd71dbe6917816343ed8415ad696c3", oldCommit)
-}
-
 func TestReadWritePullHead(t *testing.T) {
 	// Ensure we can write SHA1 head corresponding to PR and open them
 	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
@@ -99,7 +69,7 @@ func TestReadWritePullHead(t *testing.T) {
 
 	// Write a fake sha1 with only 40 zeros
 	newCommit := "feaf4ba6bc635fec442f46ddd4512416ec43c2c2"
-	err = repo.SetReference(PullPrefix+"1/head", newCommit)
+	err = UpdateRef(t.Context(), repo.Path, PullPrefix+"1/head", newCommit)
 	if err != nil {
 		assert.NoError(t, err)
 		return
@@ -116,7 +86,7 @@ func TestReadWritePullHead(t *testing.T) {
 	assert.Equal(t, headContents, newCommit)
 
 	// Remove file after the test
-	err = repo.RemoveReference(PullPrefix + "1/head")
+	err = RemoveRef(t.Context(), repo.Path, PullPrefix+"1/head")
 	assert.NoError(t, err)
 }
 

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)
+		headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pr.GetGitHeadRefName())
-		if err != nil {
-			ctx.APIErrorInternal(err)
-			return
-		}
-		defer closer.Close()
-
-		headCommitID, err := 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)

routers/private/hook_post_receive_test.go

@@ -37,13 +37,14 @@ func TestHandlePullRequestMerging(t *testing.T) {
 		PullRequestID: pr.ID,
 		UserID:        2,
 	}, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, []*repo_module.PushUpdateOptions{
-		{NewCommitID: "01234567"},
+		// assume the first commit is merged from this pull request but it's not a real world scenario
+		{NewCommitID: "65f1bf27bc3bf70f64657658635e66094edbcb4d"},
 	})
 	assert.Empty(t, resp.Body.String())
 	pr, err = issues_model.GetPullRequestByID(db.DefaultContext, pr.ID)
 	assert.NoError(t, err)
 	assert.True(t, pr.HasMerged)
-	assert.Equal(t, "01234567", pr.MergedCommitID)
+	assert.Equal(t, "65f1bf27bc3bf70f64657658635e66094edbcb4d", pr.MergedCommitID)
 
 	unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{ID: autoMerge.ID})
 }

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())
+			for _, codeComments := range comment.Review.CodeComments {
-						if err != nil {
+				for _, c := range codeComments {
-							ctx.ServerError("roleDescriptor", err)
+					// Check tag.
-							return
+					role, ok = marked[c.PosterID]
-						}
+					if ok {
-						marked[c.PosterID] = c.ShowRole
+						c.ShowRole = role
-						participants = addParticipant(c.Poster, participants)
+						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 {

routers/web/repo/pull.go

@@ -190,7 +190,7 @@ func GetPullDiffStats(ctx *context.Context) {
 	}
 	pull := issue.PullRequest
 
-	mergeBaseCommitID := GetMergedBaseCommitID(ctx, issue)
+	mergeBaseCommitID := GetMergedBaseCommitID(ctx, pull)
 	if mergeBaseCommitID == "" {
 		return // no merge base, do nothing, do not stop the route handler, see below
 	}
@@ -210,48 +210,17 @@ func GetPullDiffStats(ctx *context.Context) {
 	ctx.Data["DiffShortStat"] = diffShortStat
 }
 
-func GetMergedBaseCommitID(ctx *context.Context, issue *issues_model.Issue) string {
+func GetMergedBaseCommitID(ctx *context.Context, pull *issues_model.PullRequest) string {
-	pull := issue.PullRequest
+	if pull.MergeBase != "" {
-
+		return pull.MergeBase
-	var baseCommit string
-	// Some migrated PR won't have any Base SHA and lose history, try to get one
-	if pull.MergeBase == "" {
-		var commitSHA, parentCommit string
-		// If there is a head or a patch file, and it is readable, grab info
-		commitSHA, err := ctx.Repo.GitRepo.GetRefCommitID(pull.GetGitHeadRefName())
-		if err != nil {
-			// Head File does not exist, try the patch
-			commitSHA, err = ctx.Repo.GitRepo.ReadPatchCommit(pull.Index)
-			if err == nil {
-				// Recreate pull head in files for next time
-				if err := ctx.Repo.GitRepo.SetReference(pull.GetGitHeadRefName(), commitSHA); err != nil {
-					log.Error("Could not write head file", err)
-				}
-			} else {
-				// There is no history available
-				log.Trace("No history file available for PR %d", pull.Index)
-			}
-		}
-		if commitSHA != "" {
-			// Get immediate parent of the first commit in the patch, grab history back
-			parentCommit, _, err = git.NewCommand("rev-list", "-1", "--skip=1").AddDynamicArguments(commitSHA).RunStdString(ctx, &git.RunOpts{Dir: ctx.Repo.GitRepo.Path})
-			if err == nil {
-				parentCommit = strings.TrimSpace(parentCommit)
-			}
-			// Special case on Git < 2.25 that doesn't fail on immediate empty history
-			if err != nil || parentCommit == "" {
-				log.Info("No known parent commit for PR %d, error: %v", pull.Index, err)
-				// bring at least partial history if it can work
-				parentCommit = commitSHA
-			}
-		}
-		baseCommit = parentCommit
-	} else {
-		// Keep an empty history or original commit
-		baseCommit = pull.MergeBase
 	}
 
-	return baseCommit
+	var err error
+	pull.MergeBase, err = pull_service.CalcMergeBase(ctx, pull)
+	if err != nil {
+		log.Error("CalcMergeBase: %v", err)
+	}
+	return pull.MergeBase
 }
 
 func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo {
@@ -271,7 +240,13 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue)
 	setMergeTarget(ctx, pull)
 	ctx.Data["HasMerged"] = true
 
-	baseCommit := GetMergedBaseCommitID(ctx, issue)
+	baseCommit := GetMergedBaseCommitID(ctx, pull)
+	if baseCommit == "" {
+		ctx.Data["BaseTarget"] = pull.BaseBranch
+		ctx.Data["NumCommits"] = 0
+		ctx.Data["NumFiles"] = 0
+		return nil // no merge base, do nothing
+	}
 
 	compareInfo, err := ctx.Repo.GitRepo.GetCompareInfo(ctx.Repo.Repository.RepoPath(),
 		baseCommit, pull.GetGitHeadRefName(), false, false)
@@ -643,8 +618,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 +637,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
 		return
 	}
 	pull := issue.PullRequest
-
+	gitRepo := ctx.Repo.GitRepo
-	var (
-		startCommitID string
-		endCommitID   string
-		gitRepo       = ctx.Repo.GitRepo
-	)
 
 	prInfo := preparePullViewPullInfo(ctx, issue)
 	if ctx.Written() {
@@ -668,77 +647,70 @@ 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
+	isSingleCommit := beforeCommitID == "" && afterCommitID != ""
+	ctx.Data["IsShowingOnlySingleCommit"] = isSingleCommit
+	isShowAllCommits := (beforeCommitID == "" || beforeCommitID == prInfo.MergeBase) && (afterCommitID == "" || afterCommitID == headCommitID)
+	ctx.Data["IsShowingAllCommits"] = isShowAllCommits
 
-	if willShowSpecifiedCommit || willShowSpecifiedCommitRange {
+	var beforeCommit, afterCommit *git.Commit
-		if len(specifiedEndCommit) > 0 {
+	if !isSingleCommit {
-			endCommitID = specifiedEndCommit
+		if beforeCommitID == "" || beforeCommitID == prInfo.MergeBase {
+			beforeCommitID = prInfo.MergeBase
+			// 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
+			}
 		} else {
-			endCommitID = headCommitID
+			beforeCommit = indexCommit(prInfo.Commits, beforeCommitID)
+			if beforeCommit == nil {
+				ctx.HTTPError(http.StatusBadRequest, "before commit not found in PR commits")
+				return
+			}
 		}
-		if len(specifiedStartCommit) > 0 {
+	}
-			startCommitID = specifiedStartCommit
+
-		} else {
+	if afterCommitID == "" || afterCommitID == headCommitID {
-			startCommitID = prInfo.MergeBase
+		afterCommitID = headCommitID
+	}
+	afterCommit = indexCommit(prInfo.Commits, afterCommitID)
+	if afterCommit == nil {
+		ctx.HTTPError(http.StatusBadRequest, "after commit not found in PR commits")
+		return
+	}
+
+	if isSingleCommit {
+		beforeCommit, err = afterCommit.Parent(0)
+		if err != nil {
+			ctx.ServerError("GetParentCommit", err)
+			return
 		}
-		ctx.Data["IsShowingAllCommits"] = false
+		beforeCommitID = beforeCommit.ID.String()
-	} else {
-		endCommitID = headCommitID
-		startCommitID = prInfo.MergeBase
-		ctx.Data["IsShowingAllCommits"] = true
 	}
 
 	ctx.Data["Username"] = ctx.Repo.Owner.Name
 	ctx.Data["Reponame"] = ctx.Repo.Repository.Name
-	ctx.Data["AfterCommitID"] = endCommitID
+	ctx.Data["MergeBase"] = prInfo.MergeBase
-	ctx.Data["BeforeCommitID"] = startCommitID
+	ctx.Data["AfterCommitID"] = afterCommitID
-
+	ctx.Data["BeforeCommitID"] = beforeCommitID
-	fileOnly := ctx.FormBool("file-only")
 
 	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 +718,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 +729,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 +737,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 +749,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 +785,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 +805,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 +812,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 +859,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 +888,17 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
 }
 
 func ViewPullFilesForSingleCommit(ctx *context.Context) {
-	viewPullFiles(ctx, "", ctx.PathParam("sha"), true, true)
+	// it doesn't support showing files from mergebase to the special commit
+	// otherwise it will be ambiguous
+	viewPullFiles(ctx, "", 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)
 }
 
 func ViewPullFilesForAllCommitsOfPr(ctx *context.Context) {
-	viewPullFiles(ctx, "", "", false, false)
+	viewPullFiles(ctx, "", "")
 }
 
 // UpdatePullRequest merge PR's baseBranch into headBranch

routers/web/repo/pull_review.go

@@ -7,15 +7,18 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"slices"
 
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/organization"
 	pull_model "code.gitea.io/gitea/models/pull"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/json"
 	"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 +52,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())
+	ctx.Data["BeforeCommitID"] = ctx.FormString("before_commit_id")
-	if err != nil {
+	ctx.Data["AfterCommitID"] = ctx.FormString("after_commit_id")
-		ctx.ServerError("GetRefCommitID", err)
-		return
-	}
-	ctx.Data["AfterCommitID"] = pullHeadCommitID
 	ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
 	upload.AddUploadContext(ctx, "comment")
 	ctx.HTML(http.StatusOK, tplNewComment)
@@ -77,10 +76,7 @@ func CreateCodeComment(ctx *context.Context) {
 		return
 	}
 
-	signedLine := form.Line
+	signedLine := util.Iif(form.Side == "previous", -form.Line, form.Line)
-	if form.Side == "previous" {
-		signedLine *= -1
-	}
 
 	var attachments []string
 	if setting.Attachment.Enabled {
@@ -92,11 +88,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 +109,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 +160,47 @@ 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
+	}
+
+	headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitHeadRefName())
+	if err != nil {
+		ctx.ServerError("GetRefCommitID", err)
+		return
+	}
+	prCommitIDs, err := git.CommitIDsBetween(ctx, ctx.Repo.GitRepo.Path, comment.Issue.PullRequest.MergeBase, headCommitID)
+	if err != nil {
+		ctx.ServerError("CommitIDsBetween", err)
+		return
+	}
+
+	if beforeCommitID == "" || beforeCommitID == comment.Issue.PullRequest.MergeBase {
+		beforeCommitID = comment.Issue.PullRequest.MergeBase
+	} else if !slices.Contains(prCommitIDs, beforeCommitID) { // beforeCommitID must be one of the pull request commits
+		ctx.HTTPError(http.StatusBadRequest, fmt.Sprintf("beforeCommitID[%s] is not a valid pull request commit", beforeCommitID))
+		return
+	}
+
+	if afterCommitID == "" || afterCommitID == headCommitID {
+		afterCommitID = headCommitID
+	} else if !slices.Contains(prCommitIDs, afterCommitID) { // afterCommitID must be one of the pull request commits
+		ctx.HTTPError(http.StatusBadRequest, fmt.Sprintf("afterCommitID[%s] is not a valid pull request commit", afterCommitID))
+		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.GitRepo, ctx.Repo.Repository, comment.IssueID,
+		ctx.Doer, beforeCommitID, afterCommitID, comment.TreePath, comment.Line, showOutdatedComments)
 	if err != nil {
 		ctx.ServerError("FetchCodeCommentsByLine", err)
 		return
@@ -195,16 +225,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.Data["BeforeCommitID"] = beforeCommitID
-		ctx.ServerError("comment.Issue.LoadPullRequest", err)
+	ctx.Data["AfterCommitID"] = afterCommitID
-		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["CanBlockUser"] = func(blocker, blockee *user_model.User) bool {
 		return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee)
 	}

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(), `<div class="content comment-container"`)
 	})
 	run("diff without outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
 		ctx.Data["ShowOutdatedComments"] = false
-		renderConversation(ctx, preparedComment, "diff")
+		renderConversation(ctx, preparedComment, "diff", "", "")
 		assert.Contains(t, resp.Body.String(), `conversation-not-existing`)
 	})
 	run("timeline with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
 		ctx.Data["ShowOutdatedComments"] = true
-		renderConversation(ctx, preparedComment, "timeline")
+		renderConversation(ctx, preparedComment, "timeline", "", "")
 		assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
 	})
 	run("timeline is not affected by ShowOutdatedComments=false", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
 		ctx.Data["ShowOutdatedComments"] = false
-		renderConversation(ctx, preparedComment, "timeline")
+		renderConversation(ctx, preparedComment, "timeline", "", "")
 		assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
 	})
 	run("diff non-existing review", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
 		err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
 		assert.NoError(t, err)
 		ctx.Data["ShowOutdatedComments"] = true
-		renderConversation(ctx, preparedComment, "diff")
+		renderConversation(ctx, preparedComment, "diff", "", "")
 		assert.Equal(t, http.StatusOK, resp.Code)
 		assert.NotContains(t, resp.Body.String(), `status-page-500`)
 	})
@@ -84,7 +84,7 @@ func TestRenderConversation(t *testing.T) {
 		err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
 		assert.NoError(t, err)
 		ctx.Data["ShowOutdatedComments"] = true
-		renderConversation(ctx, preparedComment, "timeline")
+		renderConversation(ctx, preparedComment, "timeline", "", "")
 		assert.Equal(t, http.StatusOK, resp.Code)
 		assert.NotContains(t, resp.Body.String(), `status-page-500`)
 	})

routers/web/web.go

@@ -1540,7 +1540,6 @@ func registerWebRoutes(m *web.Router) {
 			m.Post("/cleanup", context.RepoMustNotBeArchived(), repo.CleanUpPullRequest)
 			m.Group("/files", func() {
 				m.Get("", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForAllCommitsOfPr)
-				m.Get("/{sha:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesStartingFromCommit)
 				m.Get("/{shaFrom:[a-f0-9]{7,40}}..{shaTo:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForRange)
 				m.Group("/reviews", func() {
 					m.Get("/new_comment", repo.RenderNewCodeCommentForm)

services/convert/pull_review.go

@@ -21,6 +21,10 @@ func ToPullReview(ctx context.Context, r *issues_model.Review, doer *user_model.
 		r.Reviewer = user_model.NewGhostUser()
 	}
 
+	if err := r.Issue.LoadRepo(ctx); err != nil {
+		return nil, err
+	}
+
 	result := &api.PullReview{
 		ID:                r.ID,
 		Reviewer:          ToUser(ctx, r.Reviewer, doer),
@@ -87,34 +91,36 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d
 		review.Reviewer = user_model.NewGhostUser()
 	}
 
+	if err := review.Issue.LoadRepo(ctx); err != nil {
+		return nil, err
+	}
+
 	apiComments := make([]*api.PullReviewComment, 0, len(review.CodeComments))
 
-	for _, lines := range review.CodeComments {
+	for _, comments := range review.CodeComments {
-		for _, comments := range lines {
+		for _, comment := range comments {
-			for _, comment := range comments {
+			apiComment := &api.PullReviewComment{
-				apiComment := &api.PullReviewComment{
+				ID:           comment.ID,
-					ID:           comment.ID,
+				Body:         comment.Content,
-					Body:         comment.Content,
+				Poster:       ToUser(ctx, comment.Poster, doer),
-					Poster:       ToUser(ctx, comment.Poster, doer),
+				Resolver:     ToUser(ctx, comment.ResolveDoer, doer),
-					Resolver:     ToUser(ctx, comment.ResolveDoer, doer),
+				ReviewID:     review.ID,
-					ReviewID:     review.ID,
+				Created:      comment.CreatedUnix.AsTime(),
-					Created:      comment.CreatedUnix.AsTime(),
+				Updated:      comment.UpdatedUnix.AsTime(),
-					Updated:      comment.UpdatedUnix.AsTime(),
+				Path:         comment.TreePath,
-					Path:         comment.TreePath,
+				CommitID:     comment.CommitSHA,
-					CommitID:     comment.CommitSHA,
+				OrigCommitID: comment.OldRef,
-					OrigCommitID: comment.OldRef,
+				DiffHunk:     patch2diff(comment.Patch),
-					DiffHunk:     patch2diff(comment.Patch),
+				HTMLURL:      comment.HTMLURL(ctx),
-					HTMLURL:      comment.HTMLURL(ctx),
+				HTMLPullURL:  review.Issue.HTMLURL(),
-					HTMLPullURL:  review.Issue.HTMLURL(),
-				}
-
-				if comment.Line < 0 {
-					apiComment.OldLineNum = comment.UnsignedLine()
-				} else {
-					apiComment.LineNum = comment.UnsignedLine()
-				}
-				apiComments = append(apiComments, apiComment)
 			}
+
+			if comment.Line < 0 {
+				apiComment.OldLineNum = comment.UnsignedLine()
+			} else {
+				apiComment.LineNum = comment.UnsignedLine()
+			}
+			apiComments = append(apiComments, apiComment)
 		}
 	}
 	return apiComments, nil

services/feed/notifier.go

@@ -223,21 +223,19 @@ func (a *actionNotifier) PullRequestReview(ctx context.Context, pr *issues_model
 	}
 
 	actions := make([]*activities_model.Action, 0, 10)
-	for _, lines := range review.CodeComments {
+	for _, comments := range review.CodeComments {
-		for _, comments := range lines {
+		for _, comm := range comments {
-			for _, comm := range comments {
+			actions = append(actions, &activities_model.Action{
-				actions = append(actions, &activities_model.Action{
+				ActUserID: review.Reviewer.ID,
-					ActUserID: review.Reviewer.ID,
+				ActUser:   review.Reviewer,
-					ActUser:   review.Reviewer,
+				Content:   fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comm.Content, "\n")[0]),
-					Content:   fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comm.Content, "\n")[0]),
+				OpType:    activities_model.ActionCommentPull,
-					OpType:    activities_model.ActionCommentPull,
+				RepoID:    review.Issue.RepoID,
-					RepoID:    review.Issue.RepoID,
+				Repo:      review.Issue.Repo,
-					Repo:      review.Issue.Repo,
+				IsPrivate: review.Issue.Repo.IsPrivate,
-					IsPrivate: review.Issue.Repo.IsPrivate,
+				Comment:   comm,
-					Comment:   comm,
+				CommentID: comm.ID,
-					CommentID: comm.ID,
+			})
-				})
-			}
 		}
 	}
 

services/forms/repo_form.go

@@ -558,7 +558,8 @@ type CodeCommentForm struct {
 	TreePath       string `form:"path" binding:"Required"`
 	SingleReview   bool   `form:"single_review"`
 	Reply          int64  `form:"reply"`
-	LatestCommitID string
+	BeforeCommitID string `form:"before_commit_id"`
+	AfterCommitID  string `form:"after_commit_id"`
 	Files          []string
 }
 

services/gitdiff/gitdiff.go

@@ -13,7 +13,6 @@ import (
 	"html/template"
 	"io"
 	"net/url"
-	"sort"
 	"strings"
 	"time"
 
@@ -21,7 +20,6 @@ import (
 	git_model "code.gitea.io/gitea/models/git"
 	issues_model "code.gitea.io/gitea/models/issues"
 	pull_model "code.gitea.io/gitea/models/pull"
-	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/analyze"
 	"code.gitea.io/gitea/modules/charset"
 	"code.gitea.io/gitea/modules/git"
@@ -454,32 +452,6 @@ type Diff struct {
 	NumViewedFiles int // user-specific
 }
 
-// LoadComments loads comments into each line
-func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, currentUser *user_model.User, showOutdatedComments bool) error {
-	allComments, err := issues_model.FetchCodeComments(ctx, issue, currentUser, showOutdatedComments)
-	if err != nil {
-		return err
-	}
-	for _, file := range diff.Files {
-		if lineCommits, ok := allComments[file.Name]; ok {
-			for _, section := range file.Sections {
-				for _, line := range section.Lines {
-					if comments, ok := lineCommits[int64(line.LeftIdx*-1)]; ok {
-						line.Comments = append(line.Comments, comments...)
-					}
-					if comments, ok := lineCommits[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
-}
-
 const cmdDiffHead = "diff --git "
 
 // ParsePatch builds a Diff object from a io.Reader and some parameters.

services/gitdiff/gitdiff_test.go

@@ -11,8 +11,6 @@ import (
 
 	"code.gitea.io/gitea/models/db"
 	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/json"
 	"code.gitea.io/gitea/modules/setting"
@@ -545,46 +543,6 @@ index 0000000..6bb8f39
 	}
 }
 
-func setupDefaultDiff() *Diff {
-	return &Diff{
-		Files: []*DiffFile{
-			{
-				Name: "README.md",
-				Sections: []*DiffSection{
-					{
-						Lines: []*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, diff.LoadComments(db.DefaultContext, issue, user, 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})
-	diff := setupDefaultDiff()
-	assert.NoError(t, diff.LoadComments(db.DefaultContext, issue, user, true))
-	assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 3)
-}
-
 func TestDiffLine_CanComment(t *testing.T) {
 	assert.False(t, (&DiffLine{Type: DiffLineSection}).CanComment())
 	assert.False(t, (&DiffLine{Type: DiffLineAdd, Comments: []*issues_model.Comment{{Content: "bla"}}}).CanComment())

services/issue/comments.go

@@ -13,8 +13,10 @@ import (
 	access_model "code.gitea.io/gitea/models/perm/access"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/json"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/timeutil"
 	git_service "code.gitea.io/gitea/services/git"
 	notify_service "code.gitea.io/gitea/services/notify"
@@ -132,13 +134,36 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion
 
 // DeleteComment deletes the comment
 func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error {
-	err := db.WithTx(ctx, func(ctx context.Context) error {
+	if comment.Type == issues_model.CommentTypeCode {
+		if err := comment.LoadIssue(ctx); err != nil {
+			return err
+		}
+		if err := comment.Issue.LoadRepo(ctx); err != nil {
+			return err
+		}
+		if err := comment.Issue.LoadPullRequest(ctx); err != nil {
+			return err
+		}
+	}
+
+	if err := db.WithTx(ctx, func(ctx context.Context) error {
 		return issues_model.DeleteComment(ctx, comment)
-	})
+	}); err != nil {
-	if err != nil {
 		return err
 	}
 
+	if comment.Type == issues_model.CommentTypeCode {
+		// We should not return error here, because the comment has been removed from database.
+		// users have to delete this ref manually or we should have a synchronize between
+		// database comment table and git refs.
+		if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID, "before")); err != nil {
+			log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
+		}
+		if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID, "after")); err != nil {
+			log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
+		}
+	}
+
 	notify_service.DeleteComment(ctx, doer, comment)
 
 	return nil

services/issue/issue.go

@@ -12,6 +12,7 @@ import (
 	issues_model "code.gitea.io/gitea/models/issues"
 	access_model "code.gitea.io/gitea/models/perm/access"
 	project_model "code.gitea.io/gitea/models/project"
+	pull_model "code.gitea.io/gitea/models/pull"
 	repo_model "code.gitea.io/gitea/models/repo"
 	system_model "code.gitea.io/gitea/models/system"
 	user_model "code.gitea.io/gitea/models/user"
@@ -200,8 +201,24 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
 
 	// delete pull request related git data
 	if issue.IsPull && gitRepo != nil {
-		if err := gitRepo.RemoveReference(issue.PullRequest.GetGitHeadRefName()); err != nil {
+		if err := git.RemoveRef(ctx, gitRepo.Path, issue.PullRequest.GetGitHeadRefName()); err != nil {
-			return err
+			log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issue.PullRequest.GetGitHeadRefName(), issue.PullRequest.ID, err)
+		}
+		if issue.PullRequest.HasMerged {
+			if err := git.RemoveRef(ctx, gitRepo.Path, issue.PullRequest.GetGitMergeRefName()); err != nil {
+				log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issue.PullRequest.GetGitMergeRefName(), issue.PullRequest.ID, err)
+			}
+		}
+	}
+
+	for _, comment := range issue.Comments {
+		if comment.Type == issues_model.CommentTypeCode {
+			if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before")); err != nil {
+				log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before"), issue.PullRequest.ID, err)
+			}
+			if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after")); err != nil {
+				log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after"), issue.PullRequest.ID, err)
+			}
 		}
 	}
 
@@ -261,74 +278,102 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
 
 // deleteIssue deletes the issue
 func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, error) {
-	ctx, committer, err := db.TxContext(ctx)
+	return db.WithTx2(ctx, func(ctx context.Context) ([]string, error) {
-	if err != nil {
+		// update the total issue numbers
-		return nil, err
+		if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
-	}
-	defer committer.Close()
-
-	if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
-		return nil, err
-	}
-
-	// update the total issue numbers
-	if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
-		return nil, err
-	}
-	// if the issue is closed, update the closed issue numbers
-	if issue.IsClosed {
-		if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
 			return nil, err
 		}
-	}
+		// if the issue is closed, update the closed issue numbers
+		if issue.IsClosed {
+			if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
+				return nil, err
+			}
+		}
 
-	if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
+		if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
-		return nil, fmt.Errorf("error updating counters for milestone id %d: %w",
+			return nil, fmt.Errorf("error updating counters for milestone id %d: %w",
-			issue.MilestoneID, err)
+				issue.MilestoneID, err)
-	}
+		}
 
-	if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
+		if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
-		return nil, err
+			return nil, err
-	}
+		}
 
-	// find attachments related to this issue and remove them
+		// find attachments related to this issue and remove them
-	if err := issue.LoadAttachments(ctx); err != nil {
+		if err := issue.LoadAttachments(ctx); err != nil {
-		return nil, err
+			return nil, err
-	}
+		}
 
-	var attachmentPaths []string
+		var attachmentPaths []string
-	for i := range issue.Attachments {
+		for i := range issue.Attachments {
-		attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath())
+			attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath())
-	}
+		}
 
-	// delete all database data still assigned to this issue
+		// deference all review comments
-	if err := db.DeleteBeans(ctx,
+		if err := issue.LoadRepo(ctx); err != nil {
-		&issues_model.ContentHistory{IssueID: issue.ID},
+			return nil, err
-		&issues_model.Comment{IssueID: issue.ID},
+		}
-		&issues_model.IssueLabel{IssueID: issue.ID},
+		if err := issue.LoadPullRequest(ctx); err != nil {
-		&issues_model.IssueDependency{IssueID: issue.ID},
+			return nil, err
-		&issues_model.IssueAssignees{IssueID: issue.ID},
+		}
-		&issues_model.IssueUser{IssueID: issue.ID},
-		&activities_model.Notification{IssueID: issue.ID},
-		&issues_model.Reaction{IssueID: issue.ID},
-		&issues_model.IssueWatch{IssueID: issue.ID},
-		&issues_model.Stopwatch{IssueID: issue.ID},
-		&issues_model.TrackedTime{IssueID: issue.ID},
-		&project_model.ProjectIssue{IssueID: issue.ID},
-		&repo_model.Attachment{IssueID: issue.ID},
-		&issues_model.PullRequest{IssueID: issue.ID},
-		&issues_model.Comment{RefIssueID: issue.ID},
-		&issues_model.IssueDependency{DependencyID: issue.ID},
-		&issues_model.Comment{DependentIssueID: issue.ID},
-		&issues_model.IssuePin{IssueID: issue.ID},
-	); err != nil {
-		return nil, err
-	}
 
-	if err := committer.Commit(); err != nil {
+		if err := issue.LoadComments(ctx); err != nil {
-		return nil, err
+			return nil, err
-	}
+		}
-	return attachmentPaths, nil
+
+		// delete all database data still assigned to this issue
+		if err := db.DeleteBeans(ctx,
+			&issues_model.ContentHistory{IssueID: issue.ID},
+			&issues_model.IssueLabel{IssueID: issue.ID},
+			&issues_model.IssueDependency{IssueID: issue.ID},
+			&issues_model.IssueAssignees{IssueID: issue.ID},
+			&issues_model.IssueUser{IssueID: issue.ID},
+			&activities_model.Notification{IssueID: issue.ID},
+			&issues_model.Reaction{IssueID: issue.ID},
+			&issues_model.IssueWatch{IssueID: issue.ID},
+			&issues_model.Stopwatch{IssueID: issue.ID},
+			&issues_model.TrackedTime{IssueID: issue.ID},
+			&project_model.ProjectIssue{IssueID: issue.ID},
+			&repo_model.Attachment{IssueID: issue.ID},
+			&issues_model.PullRequest{IssueID: issue.ID},
+			&issues_model.Comment{RefIssueID: issue.ID},
+			&issues_model.IssueDependency{DependencyID: issue.ID},
+			&issues_model.Comment{DependentIssueID: issue.ID},
+			&issues_model.IssuePin{IssueID: issue.ID},
+		); err != nil {
+			return nil, err
+		}
+
+		for _, comment := range issue.Comments {
+			if err := issues_model.DeleteComment(ctx, comment); err != nil {
+				return nil, err
+			}
+		}
+
+		// delete all pull request records
+		if issue.IsPull {
+			// Delete scheduled auto merges
+			if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
+				Delete(&pull_model.AutoMerge{}); err != nil {
+				return nil, err
+			}
+
+			// Delete review states
+			if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
+				Delete(&pull_model.ReviewState{}); err != nil {
+				return nil, err
+			}
+
+			if _, err := db.GetEngine(ctx).ID(issue.PullRequest.ID).Delete(&issues_model.PullRequest{}); err != nil {
+				return nil, err
+			}
+		}
+
+		if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
+			return nil, err
+		}
+		return attachmentPaths, nil
+	})
 }
 
 // DeleteOrphanedIssues delete issues without a repo
@@ -381,6 +426,17 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []
 				return nil, fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err)
 			}
 
+			for _, comment := range issue.Comments {
+				if comment.Type == issues_model.CommentTypeCode {
+					if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before")); err != nil {
+						log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before"), issue.PullRequest.ID, err)
+					}
+					if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after")); err != nil {
+						log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after"), issue.PullRequest.ID, err)
+					}
+				}
+			}
+
 			attachmentPaths = append(attachmentPaths, issueAttachPaths...)
 		}
 	}

services/issue/issue_test.go

@@ -39,10 +39,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Len(t, issueIDs, 5)
 
-	issue := &issues_model.Issue{
+	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: issueIDs[2]})
-		RepoID: 1,
-		ID:     issueIDs[2],
-	}
 
 	_, err = deleteIssue(db.DefaultContext, issue)
 	assert.NoError(t, err)

services/issue/pull.go

@@ -7,33 +7,15 @@ import (
 	"context"
 	"fmt"
 	"slices"
-	"time"
 
 	issues_model "code.gitea.io/gitea/models/issues"
 	org_model "code.gitea.io/gitea/models/organization"
 	user_model "code.gitea.io/gitea/models/user"
-	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 )
 
-func getMergeBase(repo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) {
-	// Add a temporary remote
-	tmpRemote := fmt.Sprintf("mergebase-%d-%d", pr.ID, time.Now().UnixNano())
-	if err := repo.AddRemote(tmpRemote, repo.Path, false); err != nil {
-		return "", fmt.Errorf("AddRemote: %w", err)
-	}
-	defer func() {
-		if err := repo.RemoveRemote(tmpRemote); err != nil {
-			log.Error("getMergeBase: RemoveRemote: %v", err)
-		}
-	}()
-
-	mergeBase, _, err := repo.GetMergeBase(tmpRemote, baseBranch, headBranch)
-	return mergeBase, err
-}
-
 type ReviewRequestNotifier struct {
 	Comment    *issues_model.Comment
 	IsAdd      bool
@@ -96,15 +78,9 @@ func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullReque
 		return nil, nil
 	}
 
-	// get the mergebase
-	mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName())
-	if err != nil {
-		return nil, err
-	}
-
 	// https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed
 	// between the merge base and the head commit but not the base branch and the head commit
-	changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitHeadRefName())
+	changedFiles, err := repo.GetFilesChangedBetween(pr.MergeBase, pr.GetGitHeadRefName())
 	if err != nil {
 		return nil, err
 	}

services/mailer/incoming/incoming_handler.go

@@ -122,11 +122,12 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u
 				nil,
 				issue,
 				comment.Line,
+				"", // no special commit ID, so we use the merge base of the pull request
+				"", // no special commit ID, so we use the current HEAD of the pull request
 				content.Content,
 				comment.TreePath,
 				false, // not pending review but a single review
 				comment.ReviewID,
-				"",
 				attachmentIDs,
 			)
 			if err != nil {

services/mailer/mail_issue_common.go

@@ -90,12 +90,7 @@ func composeIssueCommentMessages(ctx context.Context, comment *mailComment, lang
 	fallback = prefix + fallbackIssueMailSubject(comment.Issue)
 
 	if comment.Comment != nil && comment.Comment.Review != nil {
-		reviewComments = make([]*issues_model.Comment, 0, 10)
+		reviewComments = comment.Comment.Review.CodeComments.AllComments()
-		for _, lines := range comment.Comment.Review.CodeComments {
-			for _, comments := range lines {
-				reviewComments = append(reviewComments, comments...)
-			}
-		}
 	}
 	locale := translation.NewLocale(lang)
 

services/migrations/gitea_uploader.go

@@ -927,15 +927,16 @@ func (g *GiteaLocalUploader) CreateReviews(ctx context.Context, reviews ...*base
 			}
 
 			c := issues_model.Comment{
-				Type:        issues_model.CommentTypeCode,
+				Type:           issues_model.CommentTypeCode,
-				IssueID:     issue.ID,
+				IssueID:        issue.ID,
-				Content:     comment.Content,
+				Content:        comment.Content,
-				Line:        int64(line + comment.Position - 1),
+				Line:           int64(line + comment.Position - 1),
-				TreePath:    comment.TreePath,
+				TreePath:       comment.TreePath,
-				CommitSHA:   comment.CommitID,
+				BeforeCommitID: pr.MergeBase,
-				Patch:       patch,
+				CommitSHA:      comment.CommitID,
-				CreatedUnix: timeutil.TimeStamp(comment.CreatedAt.Unix()),
+				Patch:          patch,
-				UpdatedUnix: timeutil.TimeStamp(comment.UpdatedAt.Unix()),
+				CreatedUnix:    timeutil.TimeStamp(comment.CreatedAt.Unix()),
+				UpdatedUnix:    timeutil.TimeStamp(comment.UpdatedAt.Unix()),
 			}
 
 			if err := g.remapUser(ctx, review, &c); err != nil {

services/pull/merge.go

@@ -721,6 +721,12 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID
 			return false, issues_model.ErrIssueAlreadyChanged
 		}
 
+		// update merge ref, this is necessary to ensure pr.MergedCommitID can be used to do diff operations even
+		// if the repository rebased/force-pushed and the pull request's merge commit is no longer in the history
+		if err := git.UpdateRef(ctx, pr.Issue.Repo.RepoPath(), pr.GetGitMergeRefName(), pr.MergedCommitID); err != nil {
+			return false, fmt.Errorf("UpdateRef: %w", err)
+		}
+
 		return true, nil
 	})
 }

services/pull/merge_merge.go

@@ -4,6 +4,10 @@
 package pull
 
 import (
+	"context"
+	"strings"
+
+	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
@@ -23,3 +27,26 @@ func doMergeStyleMerge(ctx *mergeContext, message string) error {
 	}
 	return nil
 }
+
+// CalcMergeBase calculates the merge base for a pull request.
+func CalcMergeBase(ctx context.Context, pr *issues_model.PullRequest) (string, error) {
+	repoPath := pr.BaseRepo.RepoPath()
+	if pr.HasMerged {
+		mergeBase, _, err := git.NewCommand("merge-base").AddDashesAndList(pr.MergedCommitID+"^", pr.GetGitHeadRefName()).
+			RunStdString(ctx, &git.RunOpts{Dir: repoPath})
+		return strings.TrimSpace(mergeBase), err
+	}
+
+	mergeBase, _, err := git.NewCommand("merge-base").AddDashesAndList(pr.BaseBranch, pr.GetGitHeadRefName()).
+		RunStdString(ctx, &git.RunOpts{Dir: repoPath})
+	if err != nil {
+		var err2 error
+		mergeBase, _, err2 = git.NewCommand("rev-parse").AddDynamicArguments(git.BranchPrefix+pr.BaseBranch).
+			RunStdString(ctx, &git.RunOpts{Dir: repoPath})
+		if err2 != nil {
+			log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2)
+			return "", err2
+		}
+	}
+	return strings.TrimSpace(mergeBase), nil
+}

services/pull/pull.go

@@ -184,7 +184,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
 		return nil
 	}); err != nil {
 		// cleanup: this will only remove the reference, the real commit will be clean up when next GC
-		if err1 := baseGitRepo.RemoveReference(pr.GetGitHeadRefName()); err1 != nil {
+		if err1 := git.RemoveRef(ctx, baseGitRepo.Path, pr.GetGitHeadRefName()); err1 != nil {
 			log.Error("RemoveReference: %v", err1)
 		}
 		return err
@@ -648,7 +648,7 @@ func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) {
 		return err
 	}
 
-	_, _, err = git.NewCommand("update-ref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.HeadCommitID).RunStdString(ctx, &git.RunOpts{Dir: pr.BaseRepo.RepoPath()})
+	err = git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), pr.GetGitHeadRefName(), pr.HeadCommitID)
 	if err != nil {
 		log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err)
 	}

services/pull/review.go

@@ -10,18 +10,22 @@ import (
 	"fmt"
 	"io"
 	"regexp"
+	"slices"
+	"sort"
 	"strings"
 
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/cache"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/optional"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
+	"code.gitea.io/gitea/services/gitdiff"
 	notify_service "code.gitea.io/gitea/services/notify"
 )
 
@@ -91,12 +95,49 @@ func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestLis
 }
 
 // CreateCodeComment creates a comment on the code line
-func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, line int64, content, treePath string, pendingReview bool, replyReviewID int64, latestCommitID string, attachments []string) (*issues_model.Comment, error) {
+func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository,
+	issue *issues_model.Issue, line int64, beforeCommitID, afterCommitID, content, treePath string,
+	pendingReview bool, replyReviewID int64, attachments []string,
+) (*issues_model.Comment, error) {
 	var (
 		existsReview bool
 		err          error
 	)
 
+	if gitRepo == nil {
+		var closer io.Closer
+		gitRepo, closer, err = gitrepo.RepositoryFromContextOrOpen(ctx, issue.Repo)
+		if err != nil {
+			return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
+		}
+		defer closer.Close()
+	}
+
+	if err := issue.LoadPullRequest(ctx); err != nil {
+		return nil, fmt.Errorf("LoadPullRequest: %w", err)
+	}
+
+	headCommitID, err := gitRepo.GetRefCommitID(issue.PullRequest.GetGitHeadRefName())
+	if err != nil {
+		return nil, fmt.Errorf("GetRefCommitID[%s]: %w", issue.PullRequest.GetGitHeadRefName(), err)
+	}
+	prCommitIDs, err := git.CommitIDsBetween(ctx, gitRepo.Path, issue.PullRequest.MergeBase, headCommitID)
+	if err != nil {
+		return nil, fmt.Errorf("CommitIDsBetween[%s, %s]: %w", beforeCommitID, afterCommitID, err)
+	}
+
+	if beforeCommitID == "" || beforeCommitID == issue.PullRequest.MergeBase {
+		beforeCommitID = issue.PullRequest.MergeBase
+	} else if !slices.Contains(prCommitIDs, beforeCommitID) { // beforeCommitID must be one of the pull request commits
+		return nil, fmt.Errorf("beforeCommitID[%s] is not a valid pull request commit", beforeCommitID)
+	}
+
+	if afterCommitID == "" || afterCommitID == headCommitID {
+		afterCommitID = headCommitID
+	} else if !slices.Contains(prCommitIDs, afterCommitID) { // afterCommitID must be one of the pull request commits
+		return nil, fmt.Errorf("afterCommitID[%s] is not a valid pull request commit", afterCommitID)
+	}
+
 	// CreateCodeComment() is used for:
 	// - Single comments
 	// - Comments that are part of a review
@@ -119,7 +160,10 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
 		comment, err := createCodeComment(ctx,
 			doer,
 			issue.Repo,
+			gitRepo,
 			issue,
+			beforeCommitID,
+			afterCommitID,
 			content,
 			treePath,
 			line,
@@ -151,7 +195,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
 			Reviewer: doer,
 			Issue:    issue,
 			Official: false,
-			CommitID: latestCommitID,
+			CommitID: afterCommitID,
 		}); err != nil {
 			return nil, err
 		}
@@ -160,7 +204,10 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
 	comment, err := createCodeComment(ctx,
 		doer,
 		issue.Repo,
+		gitRepo,
 		issue,
+		beforeCommitID,
+		afterCommitID,
 		content,
 		treePath,
 		line,
@@ -173,7 +220,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
 
 	if !pendingReview && !existsReview {
 		// Submit the review we've just created so the comment shows up in the issue view
-		if _, _, err = SubmitReview(ctx, doer, gitRepo, issue, issues_model.ReviewTypeComment, "", latestCommitID, nil); err != nil {
+		if _, _, err = SubmitReview(ctx, doer, gitRepo, issue, issues_model.ReviewTypeComment, "", afterCommitID, nil); err != nil {
 			return nil, err
 		}
 	}
@@ -183,9 +230,16 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
 	return comment, nil
 }
 
+func patchCacheKey(issueID int64, beforeCommitID, afterCommitID, treePath string, line int64) string {
+	// The key is used to cache the patch for a specific line in a review comment.
+	// It is composed of the issue ID, commit IDs, tree path and line number.
+	return fmt.Sprintf("review-line-patch-%d-%s-%s-%s-%d", issueID, beforeCommitID, afterCommitID, treePath, line)
+}
+
 // createCodeComment creates a plain code comment at the specified line / path
-func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content, treePath string, line, reviewID int64, attachments []string) (*issues_model.Comment, error) {
+func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository,
-	var commitID, patch string
+	issue *issues_model.Issue, beforeCommitID, afterCommitID, content, treePath string, line, reviewID int64, attachments []string,
+) (*issues_model.Comment, error) {
 	if err := issue.LoadPullRequest(ctx); err != nil {
 		return nil, fmt.Errorf("LoadPullRequest: %w", err)
 	}
@@ -193,96 +247,59 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
 	if err := pr.LoadBaseRepo(ctx); err != nil {
 		return nil, fmt.Errorf("LoadBaseRepo: %w", err)
 	}
-	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
-	if err != nil {
-		return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
-	}
-	defer closer.Close()
 
-	invalidated := false
+	patch, err := cache.GetString(patchCacheKey(issue.ID, beforeCommitID, afterCommitID, treePath, line), func() (string, error) {
-	head := pr.GetGitHeadRefName()
-	if line > 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
-		}
 		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 {
+			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, pr.MergeBase, headCommitID, treePath, err))
+				_ = 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)
+		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)
+	}
+
+	return db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) {
+		comment, err := issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
+			Type:           issues_model.CommentTypeCode,
+			Doer:           doer,
+			Repo:           repo,
+			Issue:          issue,
+			Content:        content,
+			LineNum:        line,
+			TreePath:       treePath,
+			BeforeCommitID: beforeCommitID,
+			CommitSHA:      afterCommitID,
+			ReviewID:       reviewID,
+			Patch:          patch,
+			Invalidated:    false,
+			Attachments:    attachments,
+		})
 		if err != nil {
-			log.Error("Error whilst generating patch: %v", err)
 			return nil, err
 		}
-	}
+
-	return issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
+		// The line commit ID Must be referenced in the git repository, because the branch maybe rebased or force-pushed.
-		Type:        issues_model.CommentTypeCode,
+		// If the review commit is GC, the position can not be calculated dynamically.
-		Doer:        doer,
+		if err := git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), issues_model.GetCodeCommentRefName(pr.Index, comment.ID, "before"), beforeCommitID); err != nil {
-		Repo:        repo,
+			log.Error("Unable to update ref before_commitid in base repository for PR[%d] Error: %v", pr.ID, err)
-		Issue:       issue,
+			return nil, err
-		Content:     content,
+		}
-		LineNum:     line,
+		if err := git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), issues_model.GetCodeCommentRefName(pr.Index, comment.ID, "after"), afterCommitID); err != nil {
-		TreePath:    treePath,
+			log.Error("Unable to update ref after_commitid in base repository for PR[%d] Error: %v", pr.ID, err)
-		CommitSHA:   commitID,
+			return nil, err
-		ReviewID:    reviewID,
+		}
-		Patch:       patch,
+
-		Invalidated: invalidated,
+		return comment, nil
-		Attachments: attachments,
 	})
 }
 
@@ -328,15 +345,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 _, fileComments := range review.CodeComments {
-		for _, comments := range lines {
+		for _, codeComment := range fileComments {
-			for _, codeComment := range comments {
+			mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, codeComment.Content)
-				mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, codeComment.Content)
+			if err != nil {
-				if err != nil {
+				return nil, nil, err
-					return nil, nil, err
-				}
-				notify_service.PullRequestCodeComment(ctx, pr, codeComment, mentions)
 			}
+			notify_service.PullRequestCodeComment(ctx, pr, codeComment, mentions)
 		}
 	}
 
@@ -471,3 +486,180 @@ 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.
+// left side is the commit the comment was created on, right side is the commit the comment is displayed on.
+// 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 absLine < hunk.LeftLine {
+			// The line is before the hunk, so we can ignore it
+			continue
+		} else if hunk.LeftLine <= absLine && absLine < hunk.LeftLine+hunk.LeftHunk {
+			// The line is within the hunk, that means the line is deleted from the current commit
+			// So that we don't need to display this line
+			return 0
+		}
+		// The line is after the hunk, so we can add the right hunk size
+		newLine += hunk.RightHunk - hunk.LeftHunk
+	}
+	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, gitRepo *git.Repository, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, beforeCommitID, afterCommitID, 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
+	}
+	afterCommit, err := gitRepo.GetCommit(afterCommitID)
+	if err != nil {
+		return nil, fmt.Errorf("GetCommit[%s]: %w", afterCommitID, err)
+	}
+	n := 0
+	hunksCache := make(map[string][]*git.HunkInfo)
+	for _, comment := range comments {
+		// Code comment should always have a commit SHA, if not, we need to set it based on the line number
+		if comment.BeforeCommitID == "" {
+			comment.BeforeCommitID = beforeCommitID
+		}
+		if comment.CommitSHA == "" {
+			comment.CommitSHA = afterCommitID
+		}
+
+		dstCommitID := beforeCommitID
+		commentCommitID := comment.BeforeCommitID
+		if comment.Line > 0 {
+			dstCommitID = afterCommitID
+			commentCommitID = comment.CommitSHA
+		}
+
+		if commentCommitID != dstCommitID {
+			// If the comment is not for the current commit, we need to recalculate the line number
+			hunks, ok := hunksCache[commentCommitID+".."+dstCommitID]
+			if !ok {
+				hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), commentCommitID, dstCommitID, treePath)
+				if err != nil {
+					return nil, fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), commentCommitID, dstCommitID, err)
+				}
+				hunksCache[commentCommitID+".."+dstCommitID] = hunks
+			}
+			comment.Line = ReCalculateLineNumber(hunks, comment.Line)
+		}
+
+		if comment.Line == line {
+			commentAfterCommit, err := gitRepo.GetCommit(comment.CommitSHA)
+			if err != nil {
+				return nil, fmt.Errorf("GetCommit[%s]: %w", comment.CommitSHA, err)
+			}
+
+			// If the comment is not the first one or the comment created before the current commit
+			if n > 0 || comment.CommitSHA == afterCommitID ||
+				commentAfterCommit.Committer.When.Before(afterCommit.Committer.When) {
+				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,
+	beforeCommit, afterCommit *git.Commit, showOutdatedComments bool,
+) error {
+	if beforeCommit == nil || afterCommit == 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 {
+				if comment.BeforeCommitID == "" {
+					comment.BeforeCommitID = beforeCommit.ID.String()
+				}
+				if comment.CommitSHA == "" {
+					comment.CommitSHA = afterCommit.ID.String()
+				}
+
+				dstCommitID := beforeCommit.ID.String()
+				commentCommitID := comment.BeforeCommitID
+				if comment.Line > 0 {
+					dstCommitID = afterCommit.ID.String()
+					commentCommitID = comment.CommitSHA
+				}
+
+				if commentCommitID != dstCommitID {
+					// If the comment is not for the current commit, we need to recalculate the line number
+					hunks, ok := hunksCache[commentCommitID+".."+dstCommitID]
+					if !ok {
+						hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), commentCommitID, dstCommitID, file.Name)
+						if err != nil {
+							return fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), commentCommitID, dstCommitID, err)
+						}
+						hunksCache[commentCommitID+".."+dstCommitID] = hunks
+					}
+					comment.Line = ReCalculateLineNumber(hunks, comment.Line)
+				}
+
+				if comment.Line != 0 {
+					commentAfterCommit, err := gitRepo.GetCommit(comment.CommitSHA)
+					if err != nil {
+						return fmt.Errorf("GetCommit[%s]: %w", comment.CommitSHA, err)
+					}
+					// If the comment is not the first one or the comment created before the current commit
+					if lineComments[comment.Line] != nil || comment.CommitSHA == afterCommit.ID.String() ||
+						commentAfterCommit.Committer.When.Before(afterCommit.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
+}

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()
+	beforeCommit, err := gitRepo.GetCommit(issue.PullRequest.MergeBase)
+	assert.NoError(t, err)
+	afterCommit, 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, beforeCommit, afterCommit, 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))
+}

services/repository/delete.go

@@ -97,10 +97,6 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams
 	}
 	needRewriteKeysFile := deleted > 0
 
-	if err := deleteDBRepository(ctx, repoID); err != nil {
-		return err
-	}
-
 	if org != nil && org.IsOrganization() {
 		teams, err := organization.FindOrgTeams(ctx, org.ID)
 		if err != nil {
@@ -187,11 +183,6 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams
 		return err
 	}
 
-	// Delete Pulls and related objects
-	if err := issues_model.DeletePullsByBaseRepoID(ctx, repoID); err != nil {
-		return err
-	}
-
 	// Delete Issues and related objects
 	var attachmentPaths []string
 	if attachmentPaths, err = issue_service.DeleteIssuesByRepoID(ctx, repoID); err != nil {
@@ -291,6 +282,11 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams
 		return err
 	}
 
+	// delete all related database records first before deleting the repository record
+	if err := deleteDBRepository(ctx, repoID); err != nil {
+		return err
+	}
+
 	if err = committer.Commit(); err != nil {
 		return err
 	}

templates/repo/diff/box.tmpl

@@ -35,7 +35,7 @@
 			{{template "repo/diff/whitespace_dropdown" .}}
 			{{template "repo/diff/options_dropdown" .}}
 			{{if .PageIsPullFiles}}
-				<div id="diff-commit-select" data-issuelink="{{$.Issue.Link}}" data-queryparams="?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}" data-filter_changes_by_commit="{{ctx.Locale.Tr "repo.pulls.filter_changes_by_commit"}}">
+				<div id="diff-commit-select" data-merge-base="{{.MergeBase}}" data-issuelink="{{$.Issue.Link}}" data-queryparams="?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}" data-filter_changes_by_commit="{{ctx.Locale.Tr "repo.pulls.filter_changes_by_commit"}}">
 					{{/* the following will be replaced by vue component, but this avoids any loading artifacts till the vue component is initialized */}}
 					<div class="ui jump dropdown tiny basic button custom">
 						{{svg "octicon-git-commit"}}
@@ -184,7 +184,7 @@
 										{{end}}
 									</div>
 								{{else}}
-									<table class="chroma" data-new-comment-url="{{$.Issue.Link}}/files/reviews/new_comment" data-path="{{$file.Name}}">
+									<table class="chroma" data-new-comment-url="{{$.Issue.Link}}/files/reviews/new_comment?before_commit_id={{$.BeforeCommitID}}&after_commit_id={{$.AfterCommitID}}" data-path="{{$file.Name}}">
 										{{if $.IsSplitStyle}}
 											{{template "repo/diff/section_split" dict "file" . "root" $}}
 										{{else}}

templates/repo/diff/comment_form.tmpl

@@ -2,7 +2,8 @@
 	<form class="ui form {{if $.hidden}}tw-hidden comment-form{{end}}" action="{{$.root.Issue.Link}}/files/reviews/comments" method="post">
 	{{$.root.CsrfTokenHtml}}
 		<input type="hidden" name="origin" value="{{if $.root.PageIsPullFiles}}diff{{else}}timeline{{end}}">
-		<input type="hidden" name="latest_commit_id" value="{{$.root.AfterCommitID}}">
+		<input type="hidden" name="before_commit_id" value="{{$.root.BeforeCommitID}}">
+		<input type="hidden" name="after_commit_id" value="{{$.root.AfterCommitID}}">
 		<input type="hidden" name="side" value="{{if $.Side}}{{$.Side}}{{end}}">
 		<input type="hidden" name="line" value="{{if $.Line}}{{$.Line}}{{end}}">
 		<input type="hidden" name="path" value="{{if $.File}}{{$.File}}{{end}}">

templates/repo/issue/view_content/comments.tmpl

@@ -448,10 +448,8 @@
 
 				{{if and .Review .Review.CodeComments}}
 				<div class="timeline-item event">
-					{{range $filename, $lines := .Review.CodeComments}}
+					{{range $filename, $comms := .Review.CodeComments}}
-						{{range $line, $comms := $lines}}
+						{{template "repo/issue/view_content/conversation" dict "." $ "comments" $comms}}
-							{{template "repo/issue/view_content/conversation" dict "." $ "comments" $comms}}
-						{{end}}
 					{{end}}
 				</div>
 				{{end}}

tests/integration/pull_diff_test.go

@@ -25,10 +25,6 @@ func TestPullDiff_CommitRangePRDiff(t *testing.T) {
 	doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/4ca8bcaf27e28504df7bf996819665986b01c847..23576dd018294e476c06e569b6b0f170d0558705", true, []string{"test2.txt", "test3.txt", "test4.txt"})
 }
 
-func TestPullDiff_StartingFromBaseToCommitPRDiff(t *testing.T) {
-	doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", true, []string{"test1.txt", "test2.txt", "test3.txt"})
-}
-
 func doTestPRDiff(t *testing.T, prDiffURL string, reviewBtnDisabled bool, expectedFilenames []string) {
 	defer tests.PrepareTestEnv(t)()
 

tests/integration/pull_review_test.go

@@ -4,6 +4,7 @@
 package integration
 
 import (
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -17,6 +18,7 @@ import (
 	"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/modules/test"
 	issue_service "code.gitea.io/gitea/services/issue"
 	repo_service "code.gitea.io/gitea/services/repository"
@@ -276,3 +278,243 @@ func testIssueClose(t *testing.T, session *TestSession, owner, repo, issueNumber
 	req = NewRequestWithValues(t, "POST", closeURL, options)
 	return session.MakeRequest(t, req, http.StatusOK)
 }
+
+func Test_ReviewCodeComment(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+
+		// Create the repo.
+		repo, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user2, user2, repo_service.CreateRepoOptions{
+			Name:             "test_codecomment",
+			Readme:           "Default",
+			AutoInit:         true,
+			ObjectFormatName: git.Sha1ObjectFormat.Name(),
+			DefaultBranch:    "master",
+		}, true)
+		assert.NoError(t, err)
+
+		// add README.md to default branch
+		_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
+			OldBranch: repo.DefaultBranch,
+			Files: []*files_service.ChangeRepoFile{
+				{
+					Operation:     "update",
+					TreePath:      "README.md",
+					ContentReader: strings.NewReader("# 111\n# 222\n"),
+				},
+			},
+		})
+		assert.NoError(t, err)
+
+		var pr *issues_model.PullRequest
+		t.Run("Create Pull Request", func(t *testing.T) {
+			// create a new branch to prepare for pull request
+			_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
+				NewBranch: "branch_codecomment1",
+				Files: []*files_service.ChangeRepoFile{
+					{
+						Operation: "update",
+						TreePath:  "README.md",
+						// add 5 lines to the file
+						ContentReader: strings.NewReader("# 111\n# 222\n# 333\n# 444\n# 555\n# 666\n# 777\n"),
+					},
+				},
+			})
+			assert.NoError(t, err)
+
+			// Create a pull request.
+			session := loginUser(t, "user2")
+			testPullCreate(t, session, "user2", "test_codecomment", false, repo.DefaultBranch, "branch_codecomment1", "Test Pull Request1")
+
+			pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "branch_codecomment1"})
+		})
+
+		t.Run("Create Code Comment", func(t *testing.T) {
+			session := loginUser(t, "user2")
+
+			// Grab the CSRF token.
+			req := NewRequest(t, "GET", path.Join("user2", "test_codecomment", "pulls", "1"))
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+
+			// Create a code comment on the pull request.
+			commentURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files/reviews/comments", pr.Index)
+			options := map[string]string{
+				"_csrf":            htmlDoc.GetCSRF(),
+				"origin":           "diff",
+				"content":          "code comment on right line 4",
+				"side":             "proposed",
+				"line":             "4",
+				"path":             "README.md",
+				"single_review":    "true",
+				"reply":            "0",
+				"before_commit_id": "",
+				"after_commit_id":  "",
+			}
+			req = NewRequestWithValues(t, "POST", commentURL, options)
+			session.MakeRequest(t, req, http.StatusOK)
+
+			// Check if the comment was created.
+			comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{
+				Type:    issues_model.CommentTypeCode,
+				IssueID: pr.IssueID,
+			})
+			assert.Equal(t, "code comment on right line 4", comment.Content)
+			assert.Equal(t, "README.md", comment.TreePath)
+			assert.Equal(t, int64(4), comment.Line)
+			gitRepo, err := gitrepo.OpenRepository(t.Context(), repo)
+			assert.NoError(t, err)
+			defer gitRepo.Close()
+			commitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
+			assert.NoError(t, err)
+			assert.Equal(t, commitID, comment.CommitSHA)
+
+			// load the files page and confirm the comment is there
+			filesPageURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files", pr.Index)
+			req = NewRequest(t, "GET", filesPageURL)
+			resp = session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc = NewHTMLParser(t, resp.Body)
+			commentHTML := htmlDoc.Find(fmt.Sprintf("#issuecomment-%d", comment.ID))
+			assert.NotNil(t, commentHTML)
+			assert.Equal(t, "code comment on right line 4", strings.TrimSpace(commentHTML.Find(".comment-body .render-content").Text()))
+
+			// the last line of this comment line number is 4
+			parentTr := commentHTML.ParentsFiltered("tr").First()
+			assert.NotNil(t, parentTr)
+			previousTr := parentTr.PrevAllFiltered("tr").First()
+			val, _ := previousTr.Attr("data-line-type")
+			assert.Equal(t, "add", val)
+			td := previousTr.Find("td.lines-num-new")
+			val, _ = td.Attr("data-line-num")
+			assert.Equal(t, "4", val)
+		})
+
+		t.Run("Pushing new commit to the pull request to add lines", func(t *testing.T) {
+			// create a new branch to prepare for pull request
+			_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
+				OldBranch: "branch_codecomment1",
+				Files: []*files_service.ChangeRepoFile{
+					{
+						Operation: "update",
+						TreePath:  "README.md",
+						// add 1 line before the code comment line 4
+						ContentReader: strings.NewReader("# 111\n# 222\n# 333\n# 334\n# 444\n# 555\n# 666\n# 777\n"),
+					},
+				},
+			})
+			assert.NoError(t, err)
+
+			session := loginUser(t, "user2")
+			comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{
+				Type:    issues_model.CommentTypeCode,
+				IssueID: pr.IssueID,
+			})
+
+			// load the files page and confirm the comment's line number is dynamically adjusted
+			filesPageURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files", pr.Index)
+			req := NewRequest(t, "GET", filesPageURL)
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+			commentHTML := htmlDoc.Find(fmt.Sprintf("#issuecomment-%d", comment.ID))
+			assert.NotNil(t, commentHTML)
+			assert.Equal(t, "code comment on right line 4", strings.TrimSpace(commentHTML.Find(".comment-body .render-content").Text()))
+
+			// the last line of this comment line number is 4
+			parentTr := commentHTML.ParentsFiltered("tr").First()
+			assert.NotNil(t, parentTr)
+			previousTr := parentTr.PrevAllFiltered("tr").First()
+			val, _ := previousTr.Attr("data-line-type")
+			assert.Equal(t, "add", val)
+			td := previousTr.Find("td.lines-num-new")
+			val, _ = td.Attr("data-line-num")
+			assert.Equal(t, "5", val) // one line have inserted in this commit, so the line number should be 5 now
+		})
+
+		t.Run("Pushing new commit to the pull request to delete lines", func(t *testing.T) {
+			// create a new branch to prepare for pull request
+			_, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
+				OldBranch: "branch_codecomment1",
+				Files: []*files_service.ChangeRepoFile{
+					{
+						Operation: "update",
+						TreePath:  "README.md",
+						// delete the second line before the code comment line 4
+						ContentReader: strings.NewReader("# 111\n# 333\n# 334\n# 444\n# 555\n# 666\n# 777\n"),
+					},
+				},
+			})
+			assert.NoError(t, err)
+
+			session := loginUser(t, "user2")
+			comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{
+				Type:    issues_model.CommentTypeCode,
+				IssueID: pr.IssueID,
+			})
+
+			// load the files page and confirm the comment's line number is dynamically adjusted
+			filesPageURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files", pr.Index)
+			req := NewRequest(t, "GET", filesPageURL)
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+			commentHTML := htmlDoc.Find(fmt.Sprintf("#issuecomment-%d", comment.ID))
+			assert.NotNil(t, commentHTML)
+			assert.Equal(t, "code comment on right line 4", strings.TrimSpace(commentHTML.Find(".comment-body .render-content").Text()))
+
+			// the last line of this comment line number is 4
+			parentTr := commentHTML.ParentsFiltered("tr").First()
+			assert.NotNil(t, parentTr)
+			previousTr := parentTr.PrevAllFiltered("tr").First()
+			val, _ := previousTr.Attr("data-line-type")
+			assert.Equal(t, "add", val)
+			td := previousTr.Find("td.lines-num-new")
+			val, _ = td.Attr("data-line-num")
+			assert.Equal(t, "4", val) // one line have inserted and one line deleted before this line in this commit, so the line number should be 4 now
+
+			// add a new comment on the deleted line
+			commentURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files/reviews/comments", pr.Index)
+			options := map[string]string{
+				"_csrf":            htmlDoc.GetCSRF(),
+				"origin":           "diff",
+				"content":          "code comment on left line 2",
+				"side":             "previous",
+				"line":             "2",
+				"path":             "README.md",
+				"single_review":    "true",
+				"reply":            "0",
+				"before_commit_id": "",
+				"after_commit_id":  "",
+			}
+			req = NewRequestWithValues(t, "POST", commentURL, options)
+			session.MakeRequest(t, req, http.StatusOK)
+			// Check if the comment was created.
+			commentLast := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{
+				Type:    issues_model.CommentTypeCode,
+				IssueID: pr.IssueID,
+				Content: "code comment on left line 2",
+			})
+			assert.Equal(t, "code comment on left line 2", commentLast.Content)
+			assert.Equal(t, "README.md", commentLast.TreePath)
+			assert.Equal(t, int64(-2), commentLast.Line)
+			assert.Equal(t, pr.MergeBase, commentLast.BeforeCommitID)
+
+			// load the files page and confirm the comment's line number is dynamically adjusted
+			filesPageURL = fmt.Sprintf("/user2/test_codecomment/pulls/%d/files", pr.Index)
+			req = NewRequest(t, "GET", filesPageURL)
+			resp = session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc = NewHTMLParser(t, resp.Body)
+			commentHTML = htmlDoc.Find(fmt.Sprintf("#issuecomment-%d", commentLast.ID))
+			assert.NotNil(t, commentHTML)
+			assert.Equal(t, "code comment on left line 2", strings.TrimSpace(commentHTML.Find(".comment-body .render-content").Text()))
+
+			// the last line of this comment line number is 4
+			parentTr = commentHTML.ParentsFiltered("tr").First()
+			assert.NotNil(t, parentTr)
+			previousTr = parentTr.PrevAllFiltered("tr").First()
+			val, _ = previousTr.Attr("data-line-type")
+			assert.Equal(t, "del", val)
+			td = previousTr.Find("td.lines-num-old")
+			val, _ = td.Attr("data-line-num")
+			assert.Equal(t, "2", val)
+		})
+	})
+}

web_src/js/components/DiffCommitSelector.vue

@@ -32,6 +32,7 @@ export default defineComponent({
       locale: {
         filter_changes_by_commit: el.getAttribute('data-filter_changes_by_commit'),
       } as Record<string, string>,
+      merge_base: el.getAttribute('data-merge-base'),
       commits: [] as Array<Commit>,
       hoverActivated: false,
       lastReviewCommitSha: '',
@@ -179,9 +180,6 @@ export default defineComponent({
      * When a commit is clicked with shift this enables the range
      * selection. Second click (with shift) defines the end of the
      * range. This opens the diff of this range
-     * Exception: first commit is the first commit of this PR. Then
-     * the diff from beginning of PR up to the second clicked commit is
-     * opened
      */
     commitClickedShift(commit: Commit) {
       this.hoverActivated = !this.hoverActivated;
@@ -189,18 +187,21 @@ export default defineComponent({
       // Second click -> determine our range and open links accordingly
       if (!this.hoverActivated) {
         // find all selected commits and generate a link
-        if (this.commits[0].selected) {
+        const firstSelected = this.commits.findIndex((x) => x.selected);
-          // first commit is selected - generate a short url with only target sha
+        let start: string;
-          const lastCommitIdx = this.commits.findLastIndex((x) => x.selected);
+        if (firstSelected === 0) {
-          if (lastCommitIdx === this.commits.length - 1) {
+          start = this.merge_base;
-            // user selected all commits - just show the normal diff page
+        } else {
-            window.location.assign(`${this.issueLink}/files${this.queryParams}`);
+          start = this.commits[firstSelected - 1].id;
-          } else {
+        }
-            window.location.assign(`${this.issueLink}/files/${this.commits[lastCommitIdx].id}${this.queryParams}`);
+        const end = this.commits.findLast((x) => x.selected).id;
-          }
+        if (start === end) {
+          // if the start and end are the same, we show this single commit
+          window.location.assign(`${this.issueLink}/commits/${start}${this.queryParams}`);
+        } else if (start === this.merge_base && end === this.commits.at(-1).id) {
+          // if the first commit is selected and the last commit is selected, we show all commits
+          window.location.assign(`${this.issueLink}/files${this.queryParams}`);
         } else {
-          const start = this.commits[this.commits.findIndex((x) => x.selected) - 1].id;
-          const end = this.commits.findLast((x) => x.selected).id;
           window.location.assign(`${this.issueLink}/files/${start}..${end}${this.queryParams}`);
         }
       }