diff --git a/modules/git/fastimport.go b/modules/git/fastimport.go index 6bf0c5e32e9..d3e98e521f5 100644 --- a/modules/git/fastimport.go +++ b/modules/git/fastimport.go @@ -27,6 +27,7 @@ type FastImportFile struct { type FastImportCommit struct { Ref string + Parent string Message string Files []FastImportFile @@ -72,6 +73,9 @@ func ForceFastImport(ctx context.Context, repo RepositoryFacade, commits []FastI msg := util.IfZero(c.Message, fmt.Sprintf("test commit %d", i+1)) _, _ = fmt.Fprintf(buf, "data %d\n%s\n", len(msg), msg) + if c.Parent != "" { + _, _ = fmt.Fprintf(buf, "from %s\n", c.Parent) + } for _, f := range c.Files { mode := util.IfZero(f.Mode, EntryModeBlob) diff --git a/services/pull/review.go b/services/pull/review.go index 9972f1a148b..3d423306375 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -275,11 +275,12 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo } // If patch is still empty (unchanged line), generate code context - if patch == "" && commitID != "" { - patch, err = gitdiff.GeneratePatchForUnchangedLine(ctx, gitRepo, commitID, treePath, line, setting.UI.CodeCommentLines) + contextCommitID := util.Iif(line < 0, pr.MergeBase, headCommitID) // the line number is relative to the diff sides, not to the blamed commitID + if patch == "" && contextCommitID != "" { + patch, err = gitdiff.GeneratePatchForUnchangedLine(ctx, gitRepo, contextCommitID, treePath, line, setting.UI.CodeCommentLines) if err != nil { // Log the error but don't fail comment creation - log.Debug("Unable to generate patch for unchanged line (file=%s, line=%d, commit=%s): %v", treePath, line, commitID, err) + log.Debug("Unable to generate patch for unchanged line (file=%s, line=%d, commit=%s): %v", treePath, line, contextCommitID, err) } } } diff --git a/services/pull/review_test.go b/services/pull/review_test.go index 2254e304206..2f9e022b3d3 100644 --- a/services/pull/review_test.go +++ b/services/pull/review_test.go @@ -4,14 +4,18 @@ package pull_test import ( + "fmt" + "strings" "testing" issues_model "gitea.dev/models/issues" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" + "gitea.dev/modules/git" pull_service "gitea.dev/services/pull" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDismissReview(t *testing.T) { @@ -45,3 +49,59 @@ func TestDismissReview(t *testing.T) { assert.Error(t, err) assert.True(t, pull_service.IsErrDismissRequestOnClosedPR(err)) } + +func TestCreateCodeCommentContextOfUnchangedLine(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + ctx := t.Context() + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 6}) + require.NoError(t, pr.LoadBaseRepo(ctx)) + + numberedLines := func(prefix string, count int) string { + var sb strings.Builder + for i := range count { + _, _ = fmt.Fprintf(&sb, "%s %d\n", prefix, i+1) + } + return sb.String() + } + + // "old 10" was last changed by the first commit (line 10), it is line 15 in the merge base and line 17 in the PR head + const treePath = "unchanged-line.txt" + oldLines := numberedLines("old", 20) + baseLines := numberedLines("base", 5) + oldLines + headLines := numberedLines("head", 2) + baseLines + + headRef := pr.GetGitHeadRefName() + require.NoError(t, git.ForceFastImport(ctx, pr.BaseRepo, []git.FastImportCommit{ + {Ref: headRef, Files: []git.FastImportFile{{Path: treePath, Content: oldLines}}}, + {Ref: headRef, Parent: ":1", Files: []git.FastImportFile{{Path: treePath, Content: baseLines}}}, + {Ref: headRef, Parent: ":2", Files: []git.FastImportFile{{Path: treePath, Content: headLines}}}, + })) + + gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo) + require.NoError(t, err) + defer gitRepo.Close() + + headCommit, err := gitRepo.GetCommit(ctx, headRef) + require.NoError(t, err) + mergeBase, err := headCommit.ParentID(0) + require.NoError(t, err) + pr.MergeBase = mergeBase.String() + require.NoError(t, pr.UpdateCols(ctx, "merge_base")) + + for _, c := range []struct { + name string + line int64 + }{ + {"NewSide", 17}, + {"OldSide", -15}, + } { + t.Run(c.name, func(t *testing.T) { + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: pr.IssueID}) + require.NoError(t, issue.LoadRepo(ctx)) + comment, err := pull_service.CreateCodeComment(ctx, doer, gitRepo, issue, c.line, "test", treePath, true, 0, headCommit.ID.String(), nil) + require.NoError(t, err) + assert.True(t, strings.HasSuffix(comment.Patch, "\n old 10\n"), "patch: %q", comment.Patch) + }) + } +}