mirror of
				https://github.com/go-gitea/gitea.git
				synced 2025-10-31 10:56:10 +01:00 
			
		
		
		
	Fix incorrect comment diff hunk parsing, fix github asset ID nil panic (#35046)
* Fix missing the first char when parsing diff hunk header * Fix #35040 * Fix #35049 ---- Introduced in https://github.com/go-gitea/gitea/pull/12047/files#diff-de48c2f70e24ff5603180acf8b5ce9d0356ede8a45bfbf2a485707282ace6d6aR268 Before: <img width="487" height="167" alt="image" src="https://github.com/user-attachments/assets/17524c76-a296-4b4b-a4f9-c5150c41bae5" /> After: <img width="749" height="144" alt="image" src="https://github.com/user-attachments/assets/bcb12c76-c1ae-40f1-81b7-183d15f891db" /> --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
		| @@ -99,9 +99,9 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff | |||||||
| 	return nil | 	return nil | ||||||
| } | } | ||||||
|  |  | ||||||
| // ParseDiffHunkString parse the diffhunk content and return | // ParseDiffHunkString parse the diff hunk content and return | ||||||
| func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHunk int) { | func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightHunk int) { | ||||||
| 	ss := strings.Split(diffhunk, "@@") | 	ss := strings.Split(diffHunk, "@@") | ||||||
| 	ranges := strings.Split(ss[1][1:], " ") | 	ranges := strings.Split(ss[1][1:], " ") | ||||||
| 	leftRange := strings.Split(ranges[0], ",") | 	leftRange := strings.Split(ranges[0], ",") | ||||||
| 	leftLine, _ = strconv.Atoi(leftRange[0][1:]) | 	leftLine, _ = strconv.Atoi(leftRange[0][1:]) | ||||||
| @@ -112,14 +112,19 @@ func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHu | |||||||
| 		rightRange := strings.Split(ranges[1], ",") | 		rightRange := strings.Split(ranges[1], ",") | ||||||
| 		rightLine, _ = strconv.Atoi(rightRange[0]) | 		rightLine, _ = strconv.Atoi(rightRange[0]) | ||||||
| 		if len(rightRange) > 1 { | 		if len(rightRange) > 1 { | ||||||
| 			righHunk, _ = strconv.Atoi(rightRange[1]) | 			rightHunk, _ = strconv.Atoi(rightRange[1]) | ||||||
| 		} | 		} | ||||||
| 	} else { | 	} else { | ||||||
| 		log.Debug("Parse line number failed: %v", diffhunk) | 		log.Debug("Parse line number failed: %v", diffHunk) | ||||||
| 		rightLine = leftLine | 		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] | // Example: @@ -1,8 +1,9 @@ => [..., 1, 8, 1, 9] | ||||||
| @@ -270,6 +275,12 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi | |||||||
| 			oldNumOfLines++ | 			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 | 	// construct the new hunk header | ||||||
| 	newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@", | 	newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@", | ||||||
| 		oldBegin, oldNumOfLines, newBegin, newNumOfLines) | 		oldBegin, oldNumOfLines, newBegin, newNumOfLines) | ||||||
|   | |||||||
| @@ -179,7 +179,7 @@ func (d *DiffLine) GetExpandDirection() DiffLineExpandDirection { | |||||||
| } | } | ||||||
|  |  | ||||||
| func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int) *DiffLineSectionInfo { | 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{ | 	return &DiffLineSectionInfo{ | ||||||
| 		Path:          treePath, | 		Path:          treePath, | ||||||
| @@ -188,7 +188,7 @@ func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int | |||||||
| 		LeftIdx:       leftLine, | 		LeftIdx:       leftLine, | ||||||
| 		RightIdx:      rightLine, | 		RightIdx:      rightLine, | ||||||
| 		LeftHunkSize:  leftHunk, | 		LeftHunkSize:  leftHunk, | ||||||
| 		RightHunkSize: righHunk, | 		RightHunkSize: rightHunk, | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
| @@ -290,7 +290,7 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc | |||||||
| 	// try to find equivalent diff line. ignore, otherwise | 	// try to find equivalent diff line. ignore, otherwise | ||||||
| 	switch diffLine.Type { | 	switch diffLine.Type { | ||||||
| 	case DiffLineSection: | 	case DiffLineSection: | ||||||
| 		return getLineContent(diffLine.Content[1:], locale) | 		return getLineContent(diffLine.Content, locale) | ||||||
| 	case DiffLineAdd: | 	case DiffLineAdd: | ||||||
| 		compareDiffLine := diffSection.GetLine(diffLine.Match) | 		compareDiffLine := diffSection.GetLine(diffLine.Match) | ||||||
| 		return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale) | 		return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale) | ||||||
| @@ -856,6 +856,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact | |||||||
| 			lastLeftIdx = -1 | 			lastLeftIdx = -1 | ||||||
| 			curFile.Sections = append(curFile.Sections, curSection) | 			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) | 			lineSectionInfo := getDiffLineSectionInfo(curFile.Name, line, leftLine-1, rightLine-1) | ||||||
| 			diffLine := &DiffLine{ | 			diffLine := &DiffLine{ | ||||||
| 				Type:        DiffLineSection, | 				Type:        DiffLineSection, | ||||||
|   | |||||||
| @@ -322,7 +322,10 @@ func (g *GithubDownloaderV3) convertGithubRelease(ctx context.Context, rel *gith | |||||||
| 	httpClient := NewMigrationHTTPClient() | 	httpClient := NewMigrationHTTPClient() | ||||||
|  |  | ||||||
| 	for _, asset := range rel.Assets { | 	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{ | 		r.Assets = append(r.Assets, &base.ReleaseAsset{ | ||||||
| 			ID:            asset.GetID(), | 			ID:            asset.GetID(), | ||||||
| 			Name:          asset.GetName(), | 			Name:          asset.GetName(), | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user