Skip to content

Fix incorrect comment diff hunk parsing, fix github asset ID nil panic #35046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions modules/git/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff
return nil
}

// ParseDiffHunkString parse the diffhunk content and return
func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHunk int) {
ss := strings.Split(diffhunk, "@@")
// ParseDiffHunkString parse the diff hunk content and return
func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightHunk int) {
ss := strings.Split(diffHunk, "@@")
ranges := strings.Split(ss[1][1:], " ")
leftRange := strings.Split(ranges[0], ",")
leftLine, _ = strconv.Atoi(leftRange[0][1:])
Expand All @@ -112,14 +112,19 @@ func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHu
rightRange := strings.Split(ranges[1], ",")
rightLine, _ = strconv.Atoi(rightRange[0])
if len(rightRange) > 1 {
righHunk, _ = strconv.Atoi(rightRange[1])
rightHunk, _ = strconv.Atoi(rightRange[1])
}
} else {
log.Debug("Parse line number failed: %v", diffhunk)
log.Debug("Parse line number failed: %v", diffHunk)
rightLine = leftLine
righHunk = leftHunk
rightHunk = leftHunk
}
return leftLine, leftHunk, rightLine, righHunk
if rightLine == 0 {
// FIXME: GIT-DIFF-CUT-BUG search this tag to see details
// this is only a hacky patch, the rightLine&rightHunk might still be incorrect in some cases.
rightLine++
}
return leftLine, leftHunk, rightLine, rightHunk
}

// Example: @@ -1,8 +1,9 @@ => [..., 1, 8, 1, 9]
Expand Down Expand Up @@ -270,6 +275,12 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
oldNumOfLines++
}
}

// "git diff" outputs "@@ -1 +1,3 @@" for "OLD" => "A\nB\nC"
// FIXME: GIT-DIFF-CUT-BUG But there is a bug in CutDiffAroundLine, then the "Patch" stored in the comment model becomes "@@ -1,1 +0,4 @@"
// It may generate incorrect results for difference cases, for example: delete 2 line add 1 line, delete 2 line add 2 line etc, need to double check.
// For example: "L1\nL2" => "A\nB", then the patch shows "L2" as line 1 on the left (deleted part)

// construct the new hunk header
newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@",
oldBegin, oldNumOfLines, newBegin, newNumOfLines)
Expand Down
7 changes: 4 additions & 3 deletions services/gitdiff/gitdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (d *DiffLine) GetExpandDirection() DiffLineExpandDirection {
}

func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int) *DiffLineSectionInfo {
leftLine, leftHunk, rightLine, righHunk := git.ParseDiffHunkString(line)
leftLine, leftHunk, rightLine, rightHunk := git.ParseDiffHunkString(line)

return &DiffLineSectionInfo{
Path: treePath,
Expand All @@ -188,7 +188,7 @@ func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int
LeftIdx: leftLine,
RightIdx: rightLine,
LeftHunkSize: leftHunk,
RightHunkSize: righHunk,
RightHunkSize: rightHunk,
}
}

Expand Down Expand Up @@ -290,7 +290,7 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc
// try to find equivalent diff line. ignore, otherwise
switch diffLine.Type {
case DiffLineSection:
return getLineContent(diffLine.Content[1:], locale)
return getLineContent(diffLine.Content, locale)
case DiffLineAdd:
compareDiffLine := diffSection.GetLine(diffLine.Match)
return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale)
Expand Down Expand Up @@ -856,6 +856,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
lastLeftIdx = -1
curFile.Sections = append(curFile.Sections, curSection)

// FIXME: the "-1" can't be right, these "line idx" are all 1-based, maybe there are other bugs that covers this bug.
lineSectionInfo := getDiffLineSectionInfo(curFile.Name, line, leftLine-1, rightLine-1)
diffLine := &DiffLine{
Type: DiffLineSection,
Expand Down
5 changes: 4 additions & 1 deletion services/migrations/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,10 @@ func (g *GithubDownloaderV3) convertGithubRelease(ctx context.Context, rel *gith
httpClient := NewMigrationHTTPClient()

for _, asset := range rel.Assets {
assetID := *asset.ID // Don't optimize this, for closure we need a local variable
assetID := asset.GetID() // Don't optimize this, for closure we need a local variable TODO: no need to do so in new Golang
if assetID == 0 {
continue
}
r.Assets = append(r.Assets, &base.ReleaseAsset{
ID: asset.GetID(),
Name: asset.GetName(),
Expand Down