diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 1a86a62fae..0383e4ca9e 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -276,20 +276,24 @@ func Diff(ctx *context.Context) { userName := ctx.Repo.Owner.Name repoName := ctx.Repo.Repository.Name commitID := ctx.PathParam("sha") - var ( - gitRepo *git.Repository - err error - ) + + diffBlobExcerptData := &gitdiff.DiffBlobExcerptData{ + BaseLink: ctx.Repo.RepoLink + "/blob_excerpt", + DiffStyle: ctx.FormString("style"), + AfterCommitID: commitID, + } + gitRepo := ctx.Repo.GitRepo + var gitRepoStore gitrepo.Repository = ctx.Repo.Repository if ctx.Data["PageIsWiki"] != nil { - gitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo()) + var err error + gitRepoStore = ctx.Repo.Repository.WikiStorageRepo() + gitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, gitRepoStore) if err != nil { ctx.ServerError("Repo.GitRepo.GetCommit", err) return } - defer gitRepo.Close() - } else { - gitRepo = ctx.Repo.GitRepo + diffBlobExcerptData.BaseLink = ctx.Repo.RepoLink + "/wiki/blob_excerpt" } commit, err := gitRepo.GetCommit(commitID) @@ -324,7 +328,7 @@ func Diff(ctx *context.Context) { ctx.NotFound(err) return } - diffShortStat, err := gitdiff.GetDiffShortStat(ctx, ctx.Repo.Repository, gitRepo, "", commitID) + diffShortStat, err := gitdiff.GetDiffShortStat(ctx, gitRepoStore, gitRepo, "", commitID) if err != nil { ctx.ServerError("GetDiffShortStat", err) return @@ -360,6 +364,7 @@ func Diff(ctx *context.Context) { ctx.Data["Title"] = commit.Summary() + " ยท " + base.ShortSha(commitID) ctx.Data["Commit"] = commit ctx.Data["Diff"] = diff + ctx.Data["DiffBlobExcerptData"] = diffBlobExcerptData if !fileOnly { diffTree, err := gitdiff.GetDiffTree(ctx, gitRepo, false, parentCommitID, commitID) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 9262703078..08bd0a7e74 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -14,6 +14,7 @@ import ( "net/http" "net/url" "path/filepath" + "sort" "strings" "code.gitea.io/gitea/models/db" @@ -43,6 +44,7 @@ import ( "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/gitdiff" pull_service "code.gitea.io/gitea/services/pull" + user_service "code.gitea.io/gitea/services/user" ) const ( @@ -638,6 +640,11 @@ func PrepareCompareDiff( } ctx.Data["DiffShortStat"] = diffShortStat ctx.Data["Diff"] = diff + ctx.Data["DiffBlobExcerptData"] = &gitdiff.DiffBlobExcerptData{ + BaseLink: ci.HeadRepo.Link() + "/blob_excerpt", + DiffStyle: ctx.FormString("style"), + AfterCommitID: headCommitID, + } ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0 if !fileOnly { @@ -865,6 +872,28 @@ func CompareDiff(ctx *context.Context) { ctx.HTML(http.StatusOK, tplCompare) } +// attachCommentsToLines attaches comments to their corresponding diff lines +func attachCommentsToLines(section *gitdiff.DiffSection, lineComments map[int64][]*issues_model.Comment) { + 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 + }) + } +} + +// attachHiddenCommentIDs calculates and attaches hidden comment IDs to expand buttons +func attachHiddenCommentIDs(section *gitdiff.DiffSection, lineComments map[int64][]*issues_model.Comment) { + for _, line := range section.Lines { + gitdiff.FillHiddenCommentIDsForDiffLine(line, lineComments) + } +} + // ExcerptBlob render blob excerpt contents func ExcerptBlob(ctx *context.Context) { commitID := ctx.PathParam("sha") @@ -874,19 +903,26 @@ func ExcerptBlob(ctx *context.Context) { idxRight := ctx.FormInt("right") leftHunkSize := ctx.FormInt("left_hunk_size") rightHunkSize := ctx.FormInt("right_hunk_size") - anchor := ctx.FormString("anchor") direction := ctx.FormString("direction") filePath := ctx.FormString("path") gitRepo := ctx.Repo.GitRepo + + diffBlobExcerptData := &gitdiff.DiffBlobExcerptData{ + BaseLink: ctx.Repo.RepoLink + "/blob_excerpt", + DiffStyle: ctx.FormString("style"), + AfterCommitID: commitID, + } + if ctx.Data["PageIsWiki"] == true { var err error - gitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo()) + gitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository.WikiStorageRepo()) if err != nil { ctx.ServerError("OpenRepository", err) return } - defer gitRepo.Close() + diffBlobExcerptData.BaseLink = ctx.Repo.RepoLink + "/wiki/blob_excerpt" } + chunkSize := gitdiff.BlobExcerptChunkSize commit, err := gitRepo.GetCommit(commitID) if err != nil { @@ -947,10 +983,43 @@ func ExcerptBlob(ctx *context.Context) { section.Lines = append(section.Lines, lineSection) } } + + diffBlobExcerptData.PullIssueIndex = ctx.FormInt64("pull_issue_index") + if diffBlobExcerptData.PullIssueIndex > 0 { + if !ctx.Repo.CanRead(unit.TypePullRequests) { + ctx.NotFound(nil) + return + } + + issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, diffBlobExcerptData.PullIssueIndex) + if err != nil { + log.Error("GetIssueByIndex error: %v", err) + } else if issue.IsPull { + // FIXME: DIFF-CONVERSATION-DATA: the following data assignment is fragile + ctx.Data["Issue"] = issue + ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool { + return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee) + } + // and "diff/comment_form.tmpl" (reply comment) needs them + ctx.Data["PageIsPullFiles"] = true + ctx.Data["AfterCommitID"] = diffBlobExcerptData.AfterCommitID + + allComments, err := issues_model.FetchCodeComments(ctx, issue, ctx.Doer, ctx.FormBool("show_outdated")) + if err != nil { + log.Error("FetchCodeComments error: %v", err) + } else { + if lineComments, ok := allComments[filePath]; ok { + attachCommentsToLines(section, lineComments) + attachHiddenCommentIDs(section, lineComments) + } + } + } + } + ctx.Data["section"] = section ctx.Data["FileNameHash"] = git.HashFilePathForWebUI(filePath) - ctx.Data["AfterCommitID"] = commitID - ctx.Data["Anchor"] = anchor + ctx.Data["DiffBlobExcerptData"] = diffBlobExcerptData + ctx.HTML(http.StatusOK, tplBlobExcerpt) } diff --git a/routers/web/repo/compare_test.go b/routers/web/repo/compare_test.go new file mode 100644 index 0000000000..61472dc71e --- /dev/null +++ b/routers/web/repo/compare_test.go @@ -0,0 +1,40 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "testing" + + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/services/gitdiff" + + "github.com/stretchr/testify/assert" +) + +func TestAttachCommentsToLines(t *testing.T) { + section := &gitdiff.DiffSection{ + Lines: []*gitdiff.DiffLine{ + {LeftIdx: 5, RightIdx: 10}, + {LeftIdx: 6, RightIdx: 11}, + }, + } + + lineComments := map[int64][]*issues_model.Comment{ + -5: {{ID: 100, CreatedUnix: 1000}}, // left side comment + 10: {{ID: 200, CreatedUnix: 2000}}, // right side comment + 11: {{ID: 300, CreatedUnix: 1500}, {ID: 301, CreatedUnix: 2500}}, // multiple comments + } + + attachCommentsToLines(section, lineComments) + + // First line should have left and right comments + assert.Len(t, section.Lines[0].Comments, 2) + assert.Equal(t, int64(100), section.Lines[0].Comments[0].ID) + assert.Equal(t, int64(200), section.Lines[0].Comments[1].ID) + + // Second line should have two comments, sorted by creation time + assert.Len(t, section.Lines[1].Comments, 2) + assert.Equal(t, int64(300), section.Lines[1].Comments[0].ID) + assert.Equal(t, int64(301), section.Lines[1].Comments[1].ID) +} diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index f61571231f..cfe9a7ae02 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -827,6 +827,12 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) { } ctx.Data["Diff"] = diff + ctx.Data["DiffBlobExcerptData"] = &gitdiff.DiffBlobExcerptData{ + BaseLink: ctx.Repo.RepoLink + "/blob_excerpt", + PullIssueIndex: pull.Index, + DiffStyle: ctx.FormString("style"), + AfterCommitID: afterCommitID, + } ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0 if ctx.IsSigned && ctx.Doer != nil { diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index db0f565b52..96aea8308c 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -22,19 +22,21 @@ 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" - repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/analyze" + "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git/attribute" "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/highlight" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/svg" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/util" @@ -67,18 +69,6 @@ const ( DiffFileCopy ) -// DiffLineExpandDirection represents the DiffLineSection expand direction -type DiffLineExpandDirection uint8 - -// DiffLineExpandDirection possible values. -const ( - DiffLineExpandNone DiffLineExpandDirection = iota + 1 - DiffLineExpandSingle - DiffLineExpandUpDown - DiffLineExpandUp - DiffLineExpandDown -) - // DiffLine represents a line difference in a DiffSection. type DiffLine struct { LeftIdx int // line number, 1-based @@ -99,6 +89,8 @@ type DiffLineSectionInfo struct { RightIdx int LeftHunkSize int RightHunkSize int + + HiddenCommentIDs []int64 // IDs of hidden comments in this section } // DiffHTMLOperation is the HTML version of diffmatchpatch.Diff @@ -153,8 +145,7 @@ func (d *DiffLine) GetLineTypeMarker() string { return "" } -// GetBlobExcerptQuery builds query string to get blob excerpt -func (d *DiffLine) GetBlobExcerptQuery() string { +func (d *DiffLine) getBlobExcerptQuery() string { query := fmt.Sprintf( "last_left=%d&last_right=%d&"+ "left=%d&right=%d&"+ @@ -167,19 +158,88 @@ func (d *DiffLine) GetBlobExcerptQuery() string { return query } -// GetExpandDirection gets DiffLineExpandDirection -func (d *DiffLine) GetExpandDirection() DiffLineExpandDirection { +func (d *DiffLine) getExpandDirection() string { if d.Type != DiffLineSection || d.SectionInfo == nil || d.SectionInfo.LeftIdx-d.SectionInfo.LastLeftIdx <= 1 || d.SectionInfo.RightIdx-d.SectionInfo.LastRightIdx <= 1 { - return DiffLineExpandNone + return "" } if d.SectionInfo.LastLeftIdx <= 0 && d.SectionInfo.LastRightIdx <= 0 { - return DiffLineExpandUp + return "up" } else if d.SectionInfo.RightIdx-d.SectionInfo.LastRightIdx > BlobExcerptChunkSize && d.SectionInfo.RightHunkSize > 0 { - return DiffLineExpandUpDown + return "updown" } else if d.SectionInfo.LeftHunkSize <= 0 && d.SectionInfo.RightHunkSize <= 0 { - return DiffLineExpandDown + return "down" } - return DiffLineExpandSingle + return "single" +} + +type DiffBlobExcerptData struct { + BaseLink string + IsWikiRepo bool + PullIssueIndex int64 + DiffStyle string + AfterCommitID string +} + +func (d *DiffLine) RenderBlobExcerptButtons(fileNameHash string, data *DiffBlobExcerptData) template.HTML { + dataHiddenCommentIDs := strings.Join(base.Int64sToStrings(d.SectionInfo.HiddenCommentIDs), ",") + anchor := fmt.Sprintf("diff-%sK%d", fileNameHash, d.SectionInfo.RightIdx) + + makeButton := func(direction, svgName string) template.HTML { + style := util.IfZero(data.DiffStyle, "unified") + link := data.BaseLink + "/" + data.AfterCommitID + fmt.Sprintf("?style=%s&direction=%s&anchor=%s", url.QueryEscape(style), direction, url.QueryEscape(anchor)) + "&" + d.getBlobExcerptQuery() + if data.PullIssueIndex > 0 { + link += fmt.Sprintf("&pull_issue_index=%d", data.PullIssueIndex) + } + return htmlutil.HTMLFormat( + ``, + link, dataHiddenCommentIDs, svg.RenderHTML(svgName), + ) + } + var content template.HTML + + if len(d.SectionInfo.HiddenCommentIDs) > 0 { + tooltip := fmt.Sprintf("%d hidden comment(s)", len(d.SectionInfo.HiddenCommentIDs)) + content += htmlutil.HTMLFormat(`%d`, tooltip, len(d.SectionInfo.HiddenCommentIDs)) + } + + expandDirection := d.getExpandDirection() + if expandDirection == "up" || expandDirection == "updown" { + content += makeButton("up", "octicon-fold-up") + } + if expandDirection == "updown" || expandDirection == "down" { + content += makeButton("down", "octicon-fold-down") + } + if expandDirection == "single" { + content += makeButton("single", "octicon-fold") + } + return htmlutil.HTMLFormat(`
%s
`, expandDirection, content) +} + +// FillHiddenCommentIDsForDiffLine finds comment IDs that are in the hidden range of an expand button +func FillHiddenCommentIDsForDiffLine(line *DiffLine, lineComments map[int64][]*issues_model.Comment) { + if line.Type != DiffLineSection { + return + } + + var hiddenCommentIDs []int64 + for commentLineNum, comments := range lineComments { + if commentLineNum < 0 { + // ATTENTION: BLOB-EXCERPT-COMMENT-RIGHT: skip left-side, unchanged lines always use "right (proposed)" side for comments + continue + } + lineNum := int(commentLineNum) + isEndOfFileExpansion := line.SectionInfo.RightHunkSize == 0 + inRange := lineNum > line.SectionInfo.LastRightIdx && + (isEndOfFileExpansion && lineNum <= line.SectionInfo.RightIdx || + !isEndOfFileExpansion && lineNum < line.SectionInfo.RightIdx) + + if inRange { + for _, comment := range comments { + hiddenCommentIDs = append(hiddenCommentIDs, comment.ID) + } + } + } + line.SectionInfo.HiddenCommentIDs = hiddenCommentIDs } func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int) *DiffLineSectionInfo { @@ -485,6 +545,8 @@ func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, c sort.SliceStable(line.Comments, func(i, j int) bool { return line.Comments[i].CreatedUnix < line.Comments[j].CreatedUnix }) + // Mark expand buttons that have comments in hidden lines + FillHiddenCommentIDsForDiffLine(line, lineCommits) } } } @@ -1281,7 +1343,7 @@ type DiffShortStat struct { NumFiles, TotalAddition, TotalDeletion int } -func GetDiffShortStat(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, beforeCommitID, afterCommitID string) (*DiffShortStat, error) { +func GetDiffShortStat(ctx context.Context, repoStorage gitrepo.Repository, gitRepo *git.Repository, beforeCommitID, afterCommitID string) (*DiffShortStat, error) { afterCommit, err := gitRepo.GetCommit(afterCommitID) if err != nil { return nil, err @@ -1293,7 +1355,7 @@ func GetDiffShortStat(ctx context.Context, repo *repo_model.Repository, gitRepo } diff := &DiffShortStat{} - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = gitrepo.GetDiffShortStatByCmdArgs(ctx, repo, nil, actualBeforeCommitID.String(), afterCommitID) + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = gitrepo.GetDiffShortStatByCmdArgs(ctx, repoStorage, nil, actualBeforeCommitID.String(), afterCommitID) if err != nil { return nil, err } @@ -1386,6 +1448,75 @@ func CommentAsDiff(ctx context.Context, c *issues_model.Comment) (*Diff, error) return diff, nil } +// GeneratePatchForUnchangedLine creates a patch showing code context for an unchanged line +func GeneratePatchForUnchangedLine(gitRepo *git.Repository, commitID, treePath string, line int64, contextLines int) (string, error) { + commit, err := gitRepo.GetCommit(commitID) + if err != nil { + return "", fmt.Errorf("GetCommit: %w", err) + } + + entry, err := commit.GetTreeEntryByPath(treePath) + if err != nil { + return "", fmt.Errorf("GetTreeEntryByPath: %w", err) + } + + blob := entry.Blob() + dataRc, err := blob.DataAsync() + if err != nil { + return "", fmt.Errorf("DataAsync: %w", err) + } + defer dataRc.Close() + + return generatePatchForUnchangedLineFromReader(dataRc, treePath, line, contextLines) +} + +// generatePatchForUnchangedLineFromReader is the testable core logic that generates a patch from a reader +func generatePatchForUnchangedLineFromReader(reader io.Reader, treePath string, line int64, contextLines int) (string, error) { + // Calculate line range (commented line + lines above it) + commentLine := int(line) + if line < 0 { + commentLine = int(-line) + } + startLine := max(commentLine-contextLines, 1) + endLine := commentLine + + // Read only the needed lines efficiently + scanner := bufio.NewScanner(reader) + currentLine := 0 + var lines []string + for scanner.Scan() { + currentLine++ + if currentLine >= startLine && currentLine <= endLine { + lines = append(lines, scanner.Text()) + } + if currentLine > endLine { + break + } + } + if err := scanner.Err(); err != nil { + return "", fmt.Errorf("scanner error: %w", err) + } + + if len(lines) == 0 { + return "", fmt.Errorf("no lines found in range %d-%d", startLine, endLine) + } + + // Generate synthetic patch + var patchBuilder strings.Builder + patchBuilder.WriteString(fmt.Sprintf("diff --git a/%s b/%s\n", treePath, treePath)) + patchBuilder.WriteString(fmt.Sprintf("--- a/%s\n", treePath)) + patchBuilder.WriteString(fmt.Sprintf("+++ b/%s\n", treePath)) + patchBuilder.WriteString(fmt.Sprintf("@@ -%d,%d +%d,%d @@\n", startLine, len(lines), startLine, len(lines))) + + for _, lineContent := range lines { + patchBuilder.WriteString(" ") + patchBuilder.WriteString(lineContent) + patchBuilder.WriteString("\n") + } + + return patchBuilder.String(), nil +} + // CommentMustAsDiff executes AsDiff and logs the error instead of returning func CommentMustAsDiff(ctx context.Context, c *issues_model.Comment) *Diff { if c == nil { diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index 7b64b6b5f8..51fb9b58d6 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -640,3 +640,346 @@ func TestNoCrashes(t *testing.T) { ParsePatch(t.Context(), setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "") } } + +func TestGeneratePatchForUnchangedLineFromReader(t *testing.T) { + tests := []struct { + name string + content string + treePath string + line int64 + contextLines int + want string + wantErr bool + }{ + { + name: "single line with context", + content: "line1\nline2\nline3\nline4\nline5\n", + treePath: "test.txt", + line: 3, + contextLines: 1, + want: `diff --git a/test.txt b/test.txt +--- a/test.txt ++++ b/test.txt +@@ -2,2 +2,2 @@ + line2 + line3 +`, + }, + { + name: "negative line number (left side)", + content: "line1\nline2\nline3\nline4\nline5\n", + treePath: "test.txt", + line: -3, + contextLines: 1, + want: `diff --git a/test.txt b/test.txt +--- a/test.txt ++++ b/test.txt +@@ -2,2 +2,2 @@ + line2 + line3 +`, + }, + { + name: "line near start of file", + content: "line1\nline2\nline3\n", + treePath: "test.txt", + line: 2, + contextLines: 5, + want: `diff --git a/test.txt b/test.txt +--- a/test.txt ++++ b/test.txt +@@ -1,2 +1,2 @@ + line1 + line2 +`, + }, + { + name: "first line with context", + content: "line1\nline2\nline3\n", + treePath: "test.txt", + line: 1, + contextLines: 3, + want: `diff --git a/test.txt b/test.txt +--- a/test.txt ++++ b/test.txt +@@ -1,1 +1,1 @@ + line1 +`, + }, + { + name: "zero context lines", + content: "line1\nline2\nline3\n", + treePath: "test.txt", + line: 2, + contextLines: 0, + want: `diff --git a/test.txt b/test.txt +--- a/test.txt ++++ b/test.txt +@@ -2,1 +2,1 @@ + line2 +`, + }, + { + name: "multi-line context", + content: "package main\n\nfunc main() {\n fmt.Println(\"Hello\")\n}\n", + treePath: "main.go", + line: 4, + contextLines: 2, + want: `diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -2,3 +2,3 @@ + + func main() { + fmt.Println("Hello") +`, + }, + { + name: "empty file", + content: "", + treePath: "empty.txt", + line: 1, + contextLines: 1, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reader := strings.NewReader(tt.content) + got, err := generatePatchForUnchangedLineFromReader(reader, tt.treePath, tt.line, tt.contextLines) + if tt.wantErr { + assert.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, strings.ReplaceAll(tt.want, "", " "), got) + } + }) + } +} + +func TestCalculateHiddenCommentIDsForLine(t *testing.T) { + tests := []struct { + name string + line *DiffLine + lineComments map[int64][]*issues_model.Comment + expected []int64 + }{ + { + name: "comments in hidden range", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 10, + RightIdx: 20, + }, + }, + lineComments: map[int64][]*issues_model.Comment{ + 15: {{ID: 100}, {ID: 101}}, + 12: {{ID: 102}}, + }, + expected: []int64{100, 101, 102}, + }, + { + name: "comments outside hidden range", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 10, + RightIdx: 20, + }, + }, + lineComments: map[int64][]*issues_model.Comment{ + 5: {{ID: 100}}, + 25: {{ID: 101}}, + }, + expected: nil, + }, + { + name: "negative line numbers (left side)", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 10, + RightIdx: 20, + }, + }, + lineComments: map[int64][]*issues_model.Comment{ + -15: {{ID: 100}}, // Left-side comment, should NOT be counted + 15: {{ID: 101}}, // Right-side comment, should be counted + }, + expected: []int64{101}, // Only right-side comment + }, + { + name: "boundary conditions - normal expansion (both boundaries exclusive)", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 10, + RightIdx: 20, + RightHunkSize: 5, // Normal case: next section has content + }, + }, + lineComments: map[int64][]*issues_model.Comment{ + 10: {{ID: 100}}, // at LastRightIdx (visible line), should NOT be included + 20: {{ID: 101}}, // at RightIdx (visible line), should NOT be included + 11: {{ID: 102}}, // just inside range, should be included + 19: {{ID: 103}}, // just inside range, should be included + }, + expected: []int64{102, 103}, + }, + { + name: "boundary conditions - end of file expansion (RightIdx inclusive)", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 54, + RightIdx: 70, + RightHunkSize: 0, // End of file: no more content after + }, + }, + lineComments: map[int64][]*issues_model.Comment{ + 54: {{ID: 54}}, // at LastRightIdx (visible line), should NOT be included + 70: {{ID: 70}}, // at RightIdx (last hidden line), SHOULD be included + 60: {{ID: 60}}, // inside range, should be included + }, + expected: []int64{60, 70}, // Lines 60 and 70 are hidden + }, + { + name: "real-world scenario - start of file with hunk", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 0, // No previous visible section + RightIdx: 26, // Line 26 is first visible line of hunk + RightHunkSize: 9, // Normal hunk with content + }, + }, + lineComments: map[int64][]*issues_model.Comment{ + 1: {{ID: 1}}, // Line 1 is hidden + 26: {{ID: 26}}, // Line 26 is visible (hunk start) - should NOT be hidden + 10: {{ID: 10}}, // Line 10 is hidden + 15: {{ID: 15}}, // Line 15 is hidden + }, + expected: []int64{1, 10, 15}, // Lines 1, 10, 15 are hidden; line 26 is visible + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + FillHiddenCommentIDsForDiffLine(tt.line, tt.lineComments) + assert.ElementsMatch(t, tt.expected, tt.line.SectionInfo.HiddenCommentIDs) + }) + } +} + +func TestDiffLine_RenderBlobExcerptButtons(t *testing.T) { + tests := []struct { + name string + line *DiffLine + fileNameHash string + data *DiffBlobExcerptData + expectContains []string + expectNotContain []string + }{ + { + name: "expand up button with hidden comments", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 0, + RightIdx: 26, + LeftIdx: 26, + LastLeftIdx: 0, + LeftHunkSize: 0, + RightHunkSize: 0, + HiddenCommentIDs: []int64{100}, + }, + }, + fileNameHash: "abc123", + data: &DiffBlobExcerptData{ + BaseLink: "/repo/blob_excerpt", + AfterCommitID: "commit123", + DiffStyle: "unified", + }, + expectContains: []string{ + "octicon-fold-up", + "direction=up", + "code-comment-more", + "1 hidden comment(s)", + }, + }, + { + name: "expand up and down buttons with pull request", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 10, + RightIdx: 50, + LeftIdx: 10, + LastLeftIdx: 5, + LeftHunkSize: 5, + RightHunkSize: 5, + HiddenCommentIDs: []int64{200, 201}, + }, + }, + fileNameHash: "def456", + data: &DiffBlobExcerptData{ + BaseLink: "/repo/blob_excerpt", + AfterCommitID: "commit456", + DiffStyle: "split", + PullIssueIndex: 42, + }, + expectContains: []string{ + "octicon-fold-down", + "octicon-fold-up", + "direction=down", + "direction=up", + `data-hidden-comment-ids=",200,201,"`, // use leading and trailing commas to ensure exact match by CSS selector `attr*=",id,"` + "pull_issue_index=42", + "2 hidden comment(s)", + }, + }, + { + name: "no hidden comments", + line: &DiffLine{ + Type: DiffLineSection, + SectionInfo: &DiffLineSectionInfo{ + LastRightIdx: 10, + RightIdx: 20, + LeftIdx: 10, + LastLeftIdx: 5, + LeftHunkSize: 5, + RightHunkSize: 5, + HiddenCommentIDs: nil, + }, + }, + fileNameHash: "ghi789", + data: &DiffBlobExcerptData{ + BaseLink: "/repo/blob_excerpt", + AfterCommitID: "commit789", + }, + expectContains: []string{ + "code-expander-button", + }, + expectNotContain: []string{ + "code-comment-more", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.line.RenderBlobExcerptButtons(tt.fileNameHash, tt.data) + resultStr := string(result) + + for _, expected := range tt.expectContains { + assert.Contains(t, resultStr, expected, "Expected to contain: %s", expected) + } + + for _, notExpected := range tt.expectNotContain { + assert.NotContains(t, resultStr, notExpected, "Expected NOT to contain: %s", notExpected) + } + }) + } +} diff --git a/services/pull/review.go b/services/pull/review.go index 357a816593..3977e351ca 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -22,6 +22,7 @@ import ( "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" ) @@ -283,6 +284,15 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo log.Error("Error whilst generating patch: %v", err) return nil, err } + + // If patch is still empty (unchanged line), generate code context + if patch == "" && commitID != "" { + patch, err = gitdiff.GeneratePatchForUnchangedLine(gitRepo, commitID, 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) + } + } } return issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{ Type: issues_model.CommentTypeCode, diff --git a/templates/repo/diff/blob_excerpt.tmpl b/templates/repo/diff/blob_excerpt.tmpl index 4089d8fb33..c9aac6d61d 100644 --- a/templates/repo/diff/blob_excerpt.tmpl +++ b/templates/repo/diff/blob_excerpt.tmpl @@ -1,28 +1,10 @@ -{{$blobExcerptLink := print $.RepoLink (Iif $.PageIsWiki "/wiki" "") "/blob_excerpt/" (PathEscape $.AfterCommitID) (QueryBuild "?" "anchor" $.Anchor)}} +{{$diffBlobExcerptData := .DiffBlobExcerptData}} +{{$canCreateComment := and ctx.RootData.SignedUserID $diffBlobExcerptData.PullIssueIndex}} {{if $.IsSplitStyle}} {{range $k, $line := $.section.Lines}} - + {{if eq .GetType 4}} - {{$expandDirection := $line.GetExpandDirection}} - -
- {{if or (eq $expandDirection 3) (eq $expandDirection 5)}} - - {{end}} - {{if or (eq $expandDirection 3) (eq $expandDirection 4)}} - - {{end}} - {{if eq $expandDirection 2}} - - {{end}} -
- + {{$line.RenderBlobExcerptButtons $.FileNameHash $diffBlobExcerptData}} {{- $inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale -}} {{- template "repo/diff/section_code" dict "diff" $inlineDiff -}} @@ -33,6 +15,12 @@ {{if and $line.LeftIdx $inlineDiff.EscapeStatus.Escaped}}{{end}} {{if $line.LeftIdx}}{{end}} + {{/* ATTENTION: BLOB-EXCERPT-COMMENT-RIGHT: here it intentially use "right" side to comment, because the backend code depends on the assumption that the comment only happens on right side*/}} + {{- if and $canCreateComment $line.RightIdx -}} + + {{- end -}} {{- if $line.LeftIdx -}} {{- template "repo/diff/section_code" dict "diff" $inlineDiff -}} {{- else -}} @@ -43,6 +31,11 @@ {{if and $line.RightIdx $inlineDiff.EscapeStatus.Escaped}}{{end}} {{if $line.RightIdx}}{{end}} + {{- if and $canCreateComment $line.RightIdx -}} + + {{- end -}} {{- if $line.RightIdx -}} {{- template "repo/diff/section_code" dict "diff" $inlineDiff -}} {{- else -}} @@ -51,31 +44,26 @@ {{end}} + {{if $line.Comments}} + + + {{if eq $line.GetCommentSide "previous"}} + {{template "repo/diff/conversation" dict "." $ "comments" $line.Comments}} + {{end}} + + + {{if eq $line.GetCommentSide "proposed"}} + {{template "repo/diff/conversation" dict "." $ "comments" $line.Comments}} + {{end}} + + + {{end}} {{end}} {{else}} {{range $k, $line := $.section.Lines}} - + {{if eq .GetType 4}} - {{$expandDirection := $line.GetExpandDirection}} - -
- {{if or (eq $expandDirection 3) (eq $expandDirection 5)}} - - {{end}} - {{if or (eq $expandDirection 3) (eq $expandDirection 4)}} - - {{end}} - {{if eq $expandDirection 2}} - - {{end}} -
- + {{$line.RenderBlobExcerptButtons $.FileNameHash $diffBlobExcerptData}} {{else}} @@ -83,7 +71,21 @@ {{$inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale}} {{if $inlineDiff.EscapeStatus.Escaped}}{{end}} - {{$inlineDiff.Content}} + + {{- if and $canCreateComment -}} + + {{- end -}} + {{$inlineDiff.Content}} + + {{if $line.Comments}} + + + {{template "repo/diff/conversation" dict "." $ "comments" $line.Comments}} + + + {{end}} {{end}} {{end}} diff --git a/templates/repo/diff/section_split.tmpl b/templates/repo/diff/section_split.tmpl index 9953db5eb2..ab23b1b934 100644 --- a/templates/repo/diff/section_split.tmpl +++ b/templates/repo/diff/section_split.tmpl @@ -1,5 +1,5 @@ {{$file := .file}} -{{$blobExcerptLink := print (or ctx.RootData.CommitRepoLink ctx.RootData.RepoLink) (Iif $.root.PageIsWiki "/wiki" "") "/blob_excerpt/" (PathEscape $.root.AfterCommitID) "?"}} +{{$diffBlobExcerptData := $.root.DiffBlobExcerptData}} @@ -16,26 +16,8 @@ {{if or (ne .GetType 2) (not $hasmatch)}} {{if eq .GetType 4}} - {{$expandDirection := $line.GetExpandDirection}} - -
- {{if or (eq $expandDirection 3) (eq $expandDirection 5)}} - - {{end}} - {{if or (eq $expandDirection 3) (eq $expandDirection 4)}} - - {{end}} - {{if eq $expandDirection 2}} - - {{end}} -
- {{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale}} + {{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale}} + {{$line.RenderBlobExcerptButtons $file.NameHash $diffBlobExcerptData}} {{if $inlineDiff.EscapeStatus.Escaped}}{{end}} {{template "repo/diff/section_code" dict "diff" $inlineDiff}} {{else if and (eq .GetType 3) $hasmatch}}{{/* DEL */}} diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index cb612bc27c..908b14656e 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -1,7 +1,6 @@ {{$file := .file}} -{{$repoLink := or ctx.RootData.CommitRepoLink ctx.RootData.RepoLink}} -{{$afterCommitID := or $.root.AfterCommitID "no-after-commit-id"}}{{/* this tmpl is also used by the PR Conversation page, so the "AfterCommitID" may not exist */}} -{{$blobExcerptLink := print $repoLink (Iif $.root.PageIsWiki "/wiki" "") "/blob_excerpt/" (PathEscape $afterCommitID) "?"}} +{{/* this tmpl is also used by the PR Conversation page, so the "AfterCommitID" and "DiffBlobExcerptData" may not exist */}} +{{$diffBlobExcerptData := $.root.DiffBlobExcerptData}} @@ -14,26 +13,7 @@ {{if eq .GetType 4}} {{if $.root.AfterCommitID}} - {{$expandDirection := $line.GetExpandDirection}} - -
- {{if or (eq $expandDirection 3) (eq $expandDirection 5)}} - - {{end}} - {{if or (eq $expandDirection 3) (eq $expandDirection 4)}} - - {{end}} - {{if eq $expandDirection 2}} - - {{end}} -
- + {{$line.RenderBlobExcerptButtons $file.NameHash $diffBlobExcerptData}} {{else}} {{/* for code file preview page or comment diffs on pull comment pages, do not show the expansion arrows */}} diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index 189b9d6259..01261b0a56 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -1,3 +1,8 @@ +{{/* FIXME: DIFF-CONVERSATION-DATA: in the future this template should be refactor to avoid called by {{... "." $}} +At the moment, two kinds of request handler call this template: +* ExcerptBlob -> blob_excerpt.tmpl -> this +* Other compare and diff pages -> ... -> {section_unified.tmpl|section_split.tmpl} -> this) +The variables in "ctx.Data" are different in each case, making this template fragile, hard to read and maintain. */}} {{if len .comments}} {{$comment := index .comments 0}} {{$invalid := $comment.Invalidated}} diff --git a/web_src/css/review.css b/web_src/css/review.css index 23383c051c..39916d1bd8 100644 --- a/web_src/css/review.css +++ b/web_src/css/review.css @@ -115,6 +115,23 @@ color: var(--color-text); } +.code-expander-buttons { + position: relative; +} + +.code-expander-buttons .code-comment-more { + position: absolute; + line-height: 12px; + padding: 2px 4px; + font-size: 10px; + background-color: var(--color-primary); + color: var(--color-primary-contrast); + border-radius: 8px !important; + left: -12px; + top: 6px; + text-align: center; +} + .code-expander-button { border: none; color: var(--color-text-light); @@ -127,8 +144,8 @@ flex: 1; } -/* expand direction 3 is both ways with two buttons */ -.code-expander-buttons[data-expand-direction="3"] .code-expander-button { +/* expand direction "updown" is both ways with two buttons */ +.code-expander-buttons[data-expand-direction="updown"] .code-expander-button { height: 18px; } diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index bde7ec0324..24d937a252 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -9,7 +9,7 @@ import {submitEventSubmitter, queryElemSiblings, hideElem, showElem, animateOnce import {POST, GET} from '../modules/fetch.ts'; import {createTippy} from '../modules/tippy.ts'; import {invertFileFolding} from './file-fold.ts'; -import {parseDom} from '../utils.ts'; +import {parseDom, sleep} from '../utils.ts'; import {registerGlobalSelectorFunc} from '../modules/observer.ts'; const {i18n} = window.config; @@ -218,21 +218,46 @@ function initRepoDiffShowMore() { }); } -async function loadUntilFound() { - const hashTargetSelector = window.location.hash; - if (!hashTargetSelector.startsWith('#diff-') && !hashTargetSelector.startsWith('#issuecomment-')) { - return; - } +async function onLocationHashChange() { + // try to scroll to the target element by the current hash + const currentHash = window.location.hash; + if (!currentHash.startsWith('#diff-') && !currentHash.startsWith('#issuecomment-')) return; - while (true) { + // avoid reentrance when we are changing the hash to scroll and trigger ":target" selection + const attrAutoScrollRunning = 'data-auto-scroll-running'; + if (document.body.hasAttribute(attrAutoScrollRunning)) return; + + const targetElementId = currentHash.substring(1); + while (currentHash === window.location.hash) { // use getElementById to avoid querySelector throws an error when the hash is invalid // eslint-disable-next-line unicorn/prefer-query-selector - const targetElement = document.getElementById(hashTargetSelector.substring(1)); + const targetElement = document.getElementById(targetElementId); if (targetElement) { + // need to change hash to re-trigger ":target" CSS selector, let's manually scroll to it targetElement.scrollIntoView(); + document.body.setAttribute(attrAutoScrollRunning, 'true'); + window.location.hash = ''; + window.location.hash = currentHash; + setTimeout(() => document.body.removeAttribute(attrAutoScrollRunning), 0); return; } + // If looking for a hidden comment, try to expand the section that contains it + const issueCommentPrefix = '#issuecomment-'; + if (currentHash.startsWith(issueCommentPrefix)) { + const commentId = currentHash.substring(issueCommentPrefix.length); + const expandButton = document.querySelector(`.code-expander-button[data-hidden-comment-ids*=",${commentId},"]`); + if (expandButton) { + // avoid infinite loop, do not re-click the button if already clicked + const attrAutoLoadClicked = 'data-auto-load-clicked'; + if (expandButton.hasAttribute(attrAutoLoadClicked)) return; + expandButton.setAttribute(attrAutoLoadClicked, 'true'); + expandButton.click(); + await sleep(500); // Wait for HTMX to load the content. FIXME: need to drop htmx in the future + continue; // Try again to find the element + } + } + // the button will be refreshed after each "load more", so query it every time const showMoreButton = document.querySelector('#diff-show-more-files'); if (!showMoreButton) { @@ -246,8 +271,8 @@ async function loadUntilFound() { } function initRepoDiffHashChangeListener() { - window.addEventListener('hashchange', loadUntilFound); - loadUntilFound(); + window.addEventListener('hashchange', onLocationHashChange); + onLocationHashChange(); } export function initRepoDiffView() {