Skip to content

Commit 4b85c50

Browse files
brymutbrymut
committed
fix(gitdiff): remove patching logic duplication
1 parent b234f31 commit 4b85c50

File tree

2 files changed

+23
-118
lines changed

2 files changed

+23
-118
lines changed

‎services/gitdiff/gitdiff.go‎

Lines changed: 18 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,15 +1391,10 @@ outer:
13911391

13921392
// CommentAsDiff returns c.Patch as *Diff
13931393
func CommentAsDiff(ctx context.Context, c *issues_model.Comment) (*Diff, error) {
1394-
// If patch is empty, generate it
1395-
if c.Patch == "" && c.TreePath != "" && c.CommitSHA != "" && c.Line != 0 {
1396-
return generateCodeContextForComment(ctx, c)
1397-
}
1398-
13991394
diff, err := ParsePatch(ctx, setting.Git.MaxGitDiffLines,
14001395
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.Patch), "")
14011396
if err != nil {
1402-
log.Error("Unable to parse patch for comment ID=%d: %v", c.ID, err)
1397+
log.Error("Unable to parse patch: %v", err)
14031398
return nil, err
14041399
}
14051400
if len(diff.Files) == 0 {
@@ -1412,50 +1407,30 @@ func CommentAsDiff(ctx context.Context, c *issues_model.Comment) (*Diff, error)
14121407
return diff, nil
14131408
}
14141409

1415-
// generateCodeContextForComment creates a synthetic diff showing code context around a comment
1416-
func generateCodeContextForComment(ctx context.Context, c *issues_model.Comment) (*Diff, error) {
1417-
if err := c.LoadIssue(ctx); err != nil {
1418-
return nil, fmt.Errorf("LoadIssue: %w", err)
1419-
}
1420-
if err := c.Issue.LoadRepo(ctx); err != nil {
1421-
return nil, fmt.Errorf("LoadRepo: %w", err)
1422-
}
1423-
if err := c.Issue.LoadPullRequest(ctx); err != nil {
1424-
return nil, fmt.Errorf("LoadPullRequest: %w", err)
1425-
}
1426-
1427-
pr := c.Issue.PullRequest
1428-
if err := pr.LoadBaseRepo(ctx); err != nil {
1429-
return nil, fmt.Errorf("LoadBaseRepo: %w", err)
1430-
}
1431-
1432-
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
1410+
// GeneratePatchForUnchangedLine creates a patch showing code context for an unchanged line
1411+
func GeneratePatchForUnchangedLine(gitRepo *git.Repository, commitID, treePath string, line int64, contextLines int) (string, error) {
1412+
commit, err := gitRepo.GetCommit(commitID)
14331413
if err != nil {
1434-
return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
1414+
return "", fmt.Errorf("GetCommit: %w", err)
14351415
}
1436-
defer closer.Close()
14371416

1438-
// Get the file content at the commit
1439-
commit, err := gitRepo.GetCommit(c.CommitSHA)
1417+
entry, err := commit.GetTreeEntryByPath(treePath)
14401418
if err != nil {
1441-
return nil, fmt.Errorf("GetCommit: %w", err)
1442-
}
1443-
1444-
entry, err := commit.GetTreeEntryByPath(c.TreePath)
1445-
if err != nil {
1446-
return nil, fmt.Errorf("GetTreeEntryByPath: %w", err)
1419+
return "", fmt.Errorf("GetTreeEntryByPath: %w", err)
14471420
}
14481421

14491422
blob := entry.Blob()
14501423
dataRc, err := blob.DataAsync()
14511424
if err != nil {
1452-
return nil, fmt.Errorf("DataAsync: %w", err)
1425+
return "", fmt.Errorf("DataAsync: %w", err)
14531426
}
14541427
defer dataRc.Close()
14551428

14561429
// Calculate line range (commented line + lines above it)
1457-
commentLine := int(c.UnsignedLine())
1458-
contextLines := setting.UI.CodeCommentLines
1430+
commentLine := int(line)
1431+
if line < 0 {
1432+
commentLine = int(-line)
1433+
}
14591434
startLine := max(commentLine-contextLines, 1)
14601435
endLine := commentLine
14611436

@@ -1473,18 +1448,18 @@ func generateCodeContextForComment(ctx context.Context, c *issues_model.Comment)
14731448
}
14741449
}
14751450
if err := scanner.Err(); err != nil {
1476-
return nil, fmt.Errorf("scanner error: %w", err)
1451+
return "", fmt.Errorf("scanner error: %w", err)
14771452
}
14781453

14791454
if len(lines) == 0 {
1480-
return nil, fmt.Errorf("no lines found in range %d-%d", startLine, endLine)
1455+
return "", fmt.Errorf("no lines found in range %d-%d", startLine, endLine)
14811456
}
14821457

14831458
// Generate synthetic patch
14841459
var patchBuilder strings.Builder
1485-
patchBuilder.WriteString(fmt.Sprintf("diff --git a/%s b/%s\n", c.TreePath, c.TreePath))
1486-
patchBuilder.WriteString(fmt.Sprintf("--- a/%s\n", c.TreePath))
1487-
patchBuilder.WriteString(fmt.Sprintf("+++ b/%s\n", c.TreePath))
1460+
patchBuilder.WriteString(fmt.Sprintf("diff --git a/%s b/%s\n", treePath, treePath))
1461+
patchBuilder.WriteString(fmt.Sprintf("--- a/%s\n", treePath))
1462+
patchBuilder.WriteString(fmt.Sprintf("+++ b/%s\n", treePath))
14881463
patchBuilder.WriteString(fmt.Sprintf("@@ -%d,%d +%d,%d @@\n", startLine, len(lines), startLine, len(lines)))
14891464

14901465
for _, lineContent := range lines {
@@ -1493,14 +1468,7 @@ func generateCodeContextForComment(ctx context.Context, c *issues_model.Comment)
14931468
patchBuilder.WriteString("\n")
14941469
}
14951470

1496-
// Parse the synthetic patch
1497-
diff, err := ParsePatch(ctx, setting.Git.MaxGitDiffLines,
1498-
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(patchBuilder.String()), "")
1499-
if err != nil {
1500-
return nil, fmt.Errorf("ParsePatch: %w", err)
1501-
}
1502-
1503-
return diff, nil
1471+
return patchBuilder.String(), nil
15041472
}
15051473

15061474
// CommentMustAsDiff executes AsDiff and logs the error instead of returning

‎services/pull/review.go‎

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

77
import (
8-
"bufio"
98
"context"
109
"errors"
1110
"fmt"
@@ -23,6 +22,7 @@ import (
2322
"code.gitea.io/gitea/modules/optional"
2423
"code.gitea.io/gitea/modules/setting"
2524
"code.gitea.io/gitea/modules/util"
25+
"code.gitea.io/gitea/services/gitdiff"
2626
notify_service "code.gitea.io/gitea/services/notify"
2727
)
2828

@@ -198,70 +198,6 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
198198
return comment, nil
199199
}
200200

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 := max(commentLine-contextLines, 1)
226-
endLine := commentLine
227-
228-
// Read only the needed lines efficiently
229-
scanner := bufio.NewScanner(dataRc)
230-
currentLine := 0
231-
var lines []string
232-
for scanner.Scan() {
233-
currentLine++
234-
if currentLine >= startLine && currentLine <= endLine {
235-
lines = append(lines, scanner.Text())
236-
}
237-
if currentLine > endLine {
238-
break
239-
}
240-
}
241-
if err := scanner.Err(); err != nil {
242-
return "", fmt.Errorf("scanner error: %w", err)
243-
}
244-
245-
if len(lines) == 0 {
246-
return "", fmt.Errorf("no lines found in range %d-%d", startLine, endLine)
247-
}
248-
249-
// Generate synthetic patch
250-
var patchBuilder strings.Builder
251-
patchBuilder.WriteString(fmt.Sprintf("diff --git a/%s b/%s\n", treePath, treePath))
252-
patchBuilder.WriteString(fmt.Sprintf("--- a/%s\n", treePath))
253-
patchBuilder.WriteString(fmt.Sprintf("+++ b/%s\n", treePath))
254-
patchBuilder.WriteString(fmt.Sprintf("@@ -%d,%d +%d,%d @@\n", startLine, len(lines), startLine, len(lines)))
255-
256-
for _, lineContent := range lines {
257-
patchBuilder.WriteString(" ")
258-
patchBuilder.WriteString(lineContent)
259-
patchBuilder.WriteString("\n")
260-
}
261-
262-
return patchBuilder.String(), nil
263-
}
264-
265201
// createCodeComment creates a plain code comment at the specified line / path
266202
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) {
267203
var commitID, patch string
@@ -351,10 +287,11 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
351287

352288
// If patch is still empty (unchanged line), generate code context
353289
if len(patch) == 0 && len(commitID) > 0 {
354-
patch, err = generatePatchForUnchangedLine(gitRepo, commitID, treePath, line, setting.UI.CodeCommentLines)
290+
patch, err = gitdiff.GeneratePatchForUnchangedLine(gitRepo, commitID, treePath, line, setting.UI.CodeCommentLines)
355291
if err != nil {
356-
log.Warn("Error generating patch for unchanged line: %v", err)
357-
// Don't fail comment creation, just leave patch empty
292+
// Log the error but don't fail comment creation
293+
// This can happen for deleted/renamed files or other edge cases
294+
log.Warn("Unable to generate patch for unchanged line (file=%s, line=%d, commit=%s): %v", treePath, line, commitID, err)
358295
}
359296
}
360297
}

0 commit comments

Comments
 (0)