Skip to content

Commit 1b9ac2c

Browse files
brymutbrymut
committed
feat(gitdiff): Use generatePatchForUnchangedLine instead to generate context
1 parent 17be181 commit 1b9ac2c

File tree

2 files changed

+87
-92
lines changed

2 files changed

+87
-92
lines changed

‎services/gitdiff/gitdiff.go‎

Lines changed: 10 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,104 +1389,22 @@ outer:
13891389
return review, nil
13901390
}
13911391

1392+
// CommentAsDiff returns c.Patch as *Diff
13921393
// CommentAsDiff returns c.Patch as *Diff
13931394
func CommentAsDiff(ctx context.Context, c *issues_model.Comment) (*Diff, error) {
1394-
// Try to parse existing patch first
1395-
if c.Patch != "" {
1396-
diff, err := ParsePatch(ctx, setting.Git.MaxGitDiffLines,
1397-
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.Patch), "")
1398-
if err == nil && len(diff.Files) > 0 && len(diff.Files[0].Sections) > 0 {
1399-
return diff, nil
1400-
}
1401-
}
1402-
1403-
// If patch is empty or invalid, generate code context for unchanged line comments
1404-
if c.TreePath != "" && c.CommitSHA != "" && c.Line != 0 {
1405-
return generateCodeContextForComment(ctx, c)
1406-
}
1407-
1408-
return nil, fmt.Errorf("no valid patch or context available for comment ID: %d", c.ID)
1409-
}
1410-
1411-
func generateCodeContextForComment(ctx context.Context, c *issues_model.Comment) (*Diff, error) {
1412-
if err := c.LoadIssue(ctx); err != nil {
1413-
return nil, fmt.Errorf("LoadIssue: %w", err)
1414-
}
1415-
if err := c.Issue.LoadRepo(ctx); err != nil {
1416-
return nil, fmt.Errorf("LoadRepo: %w", err)
1417-
}
1418-
if err := c.Issue.LoadPullRequest(ctx); err != nil {
1419-
return nil, fmt.Errorf("LoadPullRequest: %w", err)
1420-
}
1421-
1422-
pr := c.Issue.PullRequest
1423-
if err := pr.LoadBaseRepo(ctx); err != nil {
1424-
return nil, fmt.Errorf("LoadBaseRepo: %w", err)
1425-
}
1426-
1427-
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
1428-
if err != nil {
1429-
return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
1430-
}
1431-
defer closer.Close()
1432-
1433-
// Get the file content at the commit
1434-
commit, err := gitRepo.GetCommit(c.CommitSHA)
1435-
if err != nil {
1436-
return nil, fmt.Errorf("GetCommit: %w", err)
1437-
}
1438-
1439-
entry, err := commit.GetTreeEntryByPath(c.TreePath)
1440-
if err != nil {
1441-
return nil, fmt.Errorf("GetTreeEntryByPath: %w", err)
1442-
}
1443-
1444-
blob := entry.Blob()
1445-
dataRc, err := blob.DataAsync()
1395+
diff, err := ParsePatch(ctx, setting.Git.MaxGitDiffLines,
1396+
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.Patch), "")
14461397
if err != nil {
1447-
return nil, fmt.Errorf("DataAsync: %w", err)
1448-
}
1449-
defer dataRc.Close()
1450-
1451-
// Read file lines
1452-
scanner := bufio.NewScanner(dataRc)
1453-
var lines []string
1454-
for scanner.Scan() {
1455-
lines = append(lines, scanner.Text())
1456-
}
1457-
if err := scanner.Err(); err != nil {
1458-
return nil, fmt.Errorf("scanner error: %w", err)
1459-
}
1460-
1461-
// Validate comment line is within file bounds
1462-
commentLine := int(c.UnsignedLine())
1463-
if commentLine < 1 || commentLine > len(lines) {
1464-
return nil, fmt.Errorf("comment line %d is out of file bounds (1-%d)", commentLine, len(lines))
1398+
log.Error("Unable to parse patch: %v", err)
1399+
return nil, err
14651400
}
1466-
1467-
// Calculate line range to show (commented line + lines above it, matching CutDiffAroundLine behavior)
1468-
contextLines := setting.UI.CodeCommentLines
1469-
startLine := max(commentLine-contextLines, 1)
1470-
endLine := commentLine
1471-
1472-
var patchBuilder strings.Builder
1473-
patchBuilder.WriteString(fmt.Sprintf("diff --git a/%s b/%s\n", c.TreePath, c.TreePath))
1474-
patchBuilder.WriteString(fmt.Sprintf("--- a/%s\n", c.TreePath))
1475-
patchBuilder.WriteString(fmt.Sprintf("+++ b/%s\n", c.TreePath))
1476-
patchBuilder.WriteString(fmt.Sprintf("@@ -%d,%d +%d,%d @@\n", startLine, endLine-startLine+1, startLine, endLine-startLine+1))
1477-
1478-
for i := startLine - 1; i < endLine; i++ {
1479-
patchBuilder.WriteString(" ")
1480-
patchBuilder.WriteString(lines[i])
1481-
patchBuilder.WriteString("\n")
1401+
if len(diff.Files) == 0 {
1402+
return nil, fmt.Errorf("no file found for comment ID: %d", c.ID)
14821403
}
1483-
1484-
diff, err := ParsePatch(ctx, setting.Git.MaxGitDiffLines,
1485-
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(patchBuilder.String()), "")
1486-
if err != nil {
1487-
return nil, fmt.Errorf("ParsePatch: %w", err)
1404+
secs := diff.Files[0].Sections
1405+
if len(secs) == 0 {
1406+
return nil, fmt.Errorf("no sections found for comment ID: %d", c.ID)
14881407
}
1489-
14901408
return diff, nil
14911409
}
14921410

‎services/pull/review.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package pull
66

77
import (
8+
"bufio"
89
"context"
910
"errors"
1011
"fmt"
@@ -197,6 +198,73 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
197198
return comment, nil
198199
}
199200

201+
// generatePatchForUnchangedLine creates a patch showing code context for an unchanged line
202+
func generatePatchForUnchangedLine(gitRepo *git.Repository, commitID, treePath string, line int64, contextLines int) (string, error) {
203+
commit, err := gitRepo.GetCommit(commitID)
204+
if err != nil {
205+
return "", fmt.Errorf("GetCommit: %w", err)
206+
}
207+
208+
entry, err := commit.GetTreeEntryByPath(treePath)
209+
if err != nil {
210+
return "", fmt.Errorf("GetTreeEntryByPath: %w", err)
211+
}
212+
213+
blob := entry.Blob()
214+
dataRc, err := blob.DataAsync()
215+
if err != nil {
216+
return "", fmt.Errorf("DataAsync: %w", err)
217+
}
218+
defer dataRc.Close()
219+
220+
// Calculate line range (commented line + lines above it)
221+
commentLine := int(line)
222+
if line < 0 {
223+
commentLine = int(-line)
224+
}
225+
startLine := commentLine - contextLines
226+
if startLine < 1 {
227+
startLine = 1
228+
}
229+
endLine := commentLine
230+
231+
// Read only the needed lines efficiently
232+
scanner := bufio.NewScanner(dataRc)
233+
currentLine := 0
234+
var lines []string
235+
for scanner.Scan() {
236+
currentLine++
237+
if currentLine >= startLine && currentLine <= endLine {
238+
lines = append(lines, scanner.Text())
239+
}
240+
if currentLine > endLine {
241+
break
242+
}
243+
}
244+
if err := scanner.Err(); err != nil {
245+
return "", fmt.Errorf("scanner error: %w", err)
246+
}
247+
248+
if len(lines) == 0 {
249+
return "", fmt.Errorf("no lines found in range %d-%d", startLine, endLine)
250+
}
251+
252+
// Generate synthetic patch
253+
var patchBuilder strings.Builder
254+
patchBuilder.WriteString(fmt.Sprintf("diff --git a/%s b/%s\n", treePath, treePath))
255+
patchBuilder.WriteString(fmt.Sprintf("--- a/%s\n", treePath))
256+
patchBuilder.WriteString(fmt.Sprintf("+++ b/%s\n", treePath))
257+
patchBuilder.WriteString(fmt.Sprintf("@@ -%d,%d +%d,%d @@\n", startLine, len(lines), startLine, len(lines)))
258+
259+
for _, lineContent := range lines {
260+
patchBuilder.WriteString(" ")
261+
patchBuilder.WriteString(lineContent)
262+
patchBuilder.WriteString("\n")
263+
}
264+
265+
return patchBuilder.String(), nil
266+
}
267+
200268
// createCodeComment creates a plain code comment at the specified line / path
201269
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) {
202270
var commitID, patch string
@@ -283,6 +351,15 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
283351
log.Error("Error whilst generating patch: %v", err)
284352
return nil, err
285353
}
354+
355+
// If patch is still empty (unchanged line), generate code context
356+
if len(patch) == 0 && len(commitID) > 0 {
357+
patch, err = generatePatchForUnchangedLine(gitRepo, commitID, treePath, line, setting.UI.CodeCommentLines)
358+
if err != nil {
359+
log.Warn("Error generating patch for unchanged line: %v", err)
360+
// Don't fail comment creation, just leave patch empty
361+
}
362+
}
286363
}
287364
return issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
288365
Type: issues_model.CommentTypeCode,

0 commit comments

Comments
 (0)