aboutsummaryrefslogtreecommitdiffstats
path: root/services/gitdiff
diff options
context:
space:
mode:
Diffstat (limited to 'services/gitdiff')
-rw-r--r--services/gitdiff/csv.go10
-rw-r--r--services/gitdiff/git_diff_tree.go7
-rw-r--r--services/gitdiff/gitdiff.go109
-rw-r--r--services/gitdiff/gitdiff_test.go6
-rw-r--r--services/gitdiff/highlightdiff.go5
-rw-r--r--services/gitdiff/highlightdiff_test.go6
-rw-r--r--services/gitdiff/submodule.go15
-rw-r--r--services/gitdiff/submodule_test.go3
8 files changed, 60 insertions, 101 deletions
diff --git a/services/gitdiff/csv.go b/services/gitdiff/csv.go
index 8db73c56a3..c10ee14490 100644
--- a/services/gitdiff/csv.go
+++ b/services/gitdiff/csv.go
@@ -134,7 +134,7 @@ func createCsvDiffSingle(reader *csv.Reader, celltype TableDiffCellType) ([]*Tab
return nil, err
}
cells := make([]*TableDiffCell, len(row))
- for j := 0; j < len(row); j++ {
+ for j := range row {
if celltype == TableDiffCellDel {
cells[j] = &TableDiffCell{LeftCell: row[j], Type: celltype}
} else {
@@ -365,11 +365,11 @@ func getColumnMapping(baseCSVReader, headCSVReader *csvReader) ([]int, []int) {
}
// Loops through the baseRow and see if there is a match in the head row
- for i := 0; i < len(baseRow); i++ {
+ for i := range baseRow {
base2HeadColMap[i] = unmappedColumn
baseCell, err := getCell(baseRow, i)
if err == nil {
- for j := 0; j < len(headRow); j++ {
+ for j := range headRow {
if head2BaseColMap[j] == -1 {
headCell, err := getCell(headRow, j)
if err == nil && baseCell == headCell {
@@ -390,7 +390,7 @@ func getColumnMapping(baseCSVReader, headCSVReader *csvReader) ([]int, []int) {
// tryMapColumnsByContent tries to map missing columns by the content of the first lines.
func tryMapColumnsByContent(baseCSVReader *csvReader, base2HeadColMap []int, headCSVReader *csvReader, head2BaseColMap []int) {
- for i := 0; i < len(base2HeadColMap); i++ {
+ for i := range base2HeadColMap {
headStart := 0
for base2HeadColMap[i] == unmappedColumn && headStart < len(head2BaseColMap) {
if head2BaseColMap[headStart] == unmappedColumn {
@@ -424,7 +424,7 @@ func getCell(row []string, column int) (string, error) {
// countUnmappedColumns returns the count of unmapped columns.
func countUnmappedColumns(mapping []int) int {
count := 0
- for i := 0; i < len(mapping); i++ {
+ for i := range mapping {
if mapping[i] == unmappedColumn {
count++
}
diff --git a/services/gitdiff/git_diff_tree.go b/services/gitdiff/git_diff_tree.go
index 035210a31d..ed94bfbfe4 100644
--- a/services/gitdiff/git_diff_tree.go
+++ b/services/gitdiff/git_diff_tree.go
@@ -6,6 +6,7 @@ package gitdiff
import (
"bufio"
"context"
+ "errors"
"fmt"
"io"
"strconv"
@@ -71,7 +72,7 @@ func runGitDiffTree(ctx context.Context, gitRepo *git.Repository, useMergeBase b
func validateGitDiffTreeArguments(gitRepo *git.Repository, useMergeBase bool, baseSha, headSha string) (shouldUseMergeBase bool, resolvedBaseSha, resolvedHeadSha string, err error) {
// if the head is empty its an error
if headSha == "" {
- return false, "", "", fmt.Errorf("headSha is empty")
+ return false, "", "", errors.New("headSha is empty")
}
// if the head commit doesn't exist its and error
@@ -207,7 +208,7 @@ func parseGitDiffTreeLine(line string) (*DiffTreeRecord, error) {
func statusFromLetter(rawStatus string) (status string, score uint8, err error) {
if len(rawStatus) < 1 {
- return "", 0, fmt.Errorf("empty status letter")
+ return "", 0, errors.New("empty status letter")
}
switch rawStatus[0] {
case 'A':
@@ -235,7 +236,7 @@ func statusFromLetter(rawStatus string) (status string, score uint8, err error)
func tryParseStatusScore(rawStatus string) (uint8, error) {
if len(rawStatus) < 2 {
- return 0, fmt.Errorf("status score missing")
+ return 0, errors.New("status score missing")
}
score, err := strconv.ParseUint(rawStatus[1:], 10, 8)
diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go
index b9781cf8d0..7c99e049d5 100644
--- a/services/gitdiff/gitdiff.go
+++ b/services/gitdiff/gitdiff.go
@@ -25,6 +25,7 @@ import (
"code.gitea.io/gitea/modules/analyze"
"code.gitea.io/gitea/modules/charset"
"code.gitea.io/gitea/modules/git"
+ "code.gitea.io/gitea/modules/git/attribute"
"code.gitea.io/gitea/modules/highlight"
"code.gitea.io/gitea/modules/lfs"
"code.gitea.io/gitea/modules/log"
@@ -178,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,
@@ -187,7 +188,7 @@ func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int
LeftIdx: leftLine,
RightIdx: rightLine,
LeftHunkSize: leftHunk,
- RightHunkSize: righHunk,
+ RightHunkSize: rightHunk,
}
}
@@ -213,51 +214,6 @@ func (diffSection *DiffSection) GetLine(idx int) *DiffLine {
return diffSection.Lines[idx]
}
-// GetLine gets a specific line by type (add or del) and file line number
-// This algorithm is not quite right.
-// Actually now we have "Match" field, it is always right, so use it instead in new GetLine
-func (diffSection *DiffSection) getLineLegacy(lineType DiffLineType, idx int) *DiffLine { //nolint:unused
- var (
- difference = 0
- addCount = 0
- delCount = 0
- matchDiffLine *DiffLine
- )
-
-LOOP:
- for _, diffLine := range diffSection.Lines {
- switch diffLine.Type {
- case DiffLineAdd:
- addCount++
- case DiffLineDel:
- delCount++
- default:
- if matchDiffLine != nil {
- break LOOP
- }
- difference = diffLine.RightIdx - diffLine.LeftIdx
- addCount = 0
- delCount = 0
- }
-
- switch lineType {
- case DiffLineDel:
- if diffLine.RightIdx == 0 && diffLine.LeftIdx == idx-difference {
- matchDiffLine = diffLine
- }
- case DiffLineAdd:
- if diffLine.LeftIdx == 0 && diffLine.RightIdx == idx+difference {
- matchDiffLine = diffLine
- }
- }
- }
-
- if addCount == delCount {
- return matchDiffLine
- }
- return nil
-}
-
func defaultDiffMatchPatch() *diffmatchpatch.DiffMatchPatch {
dmp := diffmatchpatch.New()
dmp.DiffEditCost = 100
@@ -334,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)
@@ -539,10 +495,7 @@ func ParsePatch(ctx context.Context, maxLines, maxLineCharacters, maxFiles int,
// OK let's set a reasonable buffer size.
// This should be at least the size of maxLineCharacters or 4096 whichever is larger.
- readerSize := maxLineCharacters
- if readerSize < 4096 {
- readerSize = 4096
- }
+ readerSize := max(maxLineCharacters, 4096)
input := bufio.NewReaderSize(reader, readerSize)
line, err := input.ReadString('\n')
@@ -903,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,
@@ -1231,36 +1185,35 @@ func GetDiffForAPI(ctx context.Context, gitRepo *git.Repository, opts *DiffOptio
return diff, err
}
-func GetDiffForRender(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff, error) {
+func GetDiffForRender(ctx context.Context, repoLink string, gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff, error) {
diff, beforeCommit, afterCommit, err := getDiffBasic(ctx, gitRepo, opts, files...)
if err != nil {
return nil, err
}
- checker, deferrable := gitRepo.CheckAttributeReader(opts.AfterCommitID)
- defer deferrable()
+ checker, err := attribute.NewBatchChecker(gitRepo, opts.AfterCommitID, []string{attribute.LinguistVendored, attribute.LinguistGenerated, attribute.LinguistLanguage, attribute.GitlabLanguage, attribute.Diff})
+ if err != nil {
+ return nil, err
+ }
+ defer checker.Close()
for _, diffFile := range diff.Files {
isVendored := optional.None[bool]()
isGenerated := optional.None[bool]()
- if checker != nil {
- attrs, err := checker.CheckPath(diffFile.Name)
- if err == nil {
- isVendored = git.AttributeToBool(attrs, git.AttributeLinguistVendored)
- isGenerated = git.AttributeToBool(attrs, git.AttributeLinguistGenerated)
-
- language := git.TryReadLanguageAttribute(attrs)
- if language.Has() {
- diffFile.Language = language.Value()
- }
- } else {
- checker = nil // CheckPath fails, it's not impossible to "check" anymore
+ attrDiff := optional.None[string]()
+ attrs, err := checker.CheckPath(diffFile.Name)
+ if err == nil {
+ isVendored, isGenerated = attrs.GetVendored(), attrs.GetGenerated()
+ language := attrs.GetLanguage()
+ if language.Has() {
+ diffFile.Language = language.Value()
}
+ attrDiff = attrs.Get(attribute.Diff).ToString()
}
// Populate Submodule URLs
if diffFile.SubmoduleDiffInfo != nil {
- diffFile.SubmoduleDiffInfo.PopulateURL(diffFile, beforeCommit, afterCommit)
+ diffFile.SubmoduleDiffInfo.PopulateURL(repoLink, diffFile, beforeCommit, afterCommit)
}
if !isVendored.Has() {
@@ -1277,7 +1230,8 @@ func GetDiffForRender(ctx context.Context, gitRepo *git.Repository, opts *DiffOp
diffFile.Sections = append(diffFile.Sections, tailSection)
}
- if !setting.Git.DisableDiffHighlight {
+ shouldFullFileHighlight := !setting.Git.DisableDiffHighlight && attrDiff.Value() == ""
+ if shouldFullFileHighlight {
if limitedContent.LeftContent != nil && limitedContent.LeftContent.buf.Len() < MaxDiffHighlightEntireFileSize {
diffFile.highlightedLeftLines = highlightCodeLines(diffFile, true /* left */, limitedContent.LeftContent.buf.String())
}
@@ -1339,10 +1293,13 @@ func GetDiffShortStat(gitRepo *git.Repository, beforeCommitID, afterCommitID str
// SyncUserSpecificDiff inserts user-specific data such as which files the user has already viewed on the given diff
// Additionally, the database is updated asynchronously if files have changed since the last review
-func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.PullRequest, gitRepo *git.Repository, diff *Diff, opts *DiffOptions, files ...string) error {
+func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.PullRequest, gitRepo *git.Repository, diff *Diff, opts *DiffOptions) (*pull_model.ReviewState, error) {
review, err := pull_model.GetNewestReviewState(ctx, userID, pull.ID)
- if err != nil || review == nil || review.UpdatedFiles == nil {
- return err
+ if err != nil {
+ return nil, err
+ }
+ if review == nil || len(review.UpdatedFiles) == 0 {
+ return review, nil
}
latestCommit := opts.AfterCommitID
@@ -1350,13 +1307,13 @@ func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.
latestCommit = pull.HeadBranch // opts.AfterCommitID is preferred because it handles PRs from forks correctly and the branch name doesn't
}
- changedFiles, err := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit)
+ changedFiles, errIgnored := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit)
// There are way too many possible errors.
// Examples are various git errors such as the commit the review was based on was gc'ed and hence doesn't exist anymore as well as unrecoverable errors where we should serve a 500 response
// Due to the current architecture and physical limitation of needing to compare explicit error messages, we can only choose one approach without the code getting ugly
// For SOME of the errors such as the gc'ed commit, it would be best to mark all files as changed
// But as that does not work for all potential errors, we simply mark all files as unchanged and drop the error which always works, even if not as good as possible
- if err != nil {
+ if errIgnored != nil {
log.Error("Could not get changed files between %s and %s for pull request %d in repo with path %s. Assuming no changes. Error: %w", review.CommitSHA, latestCommit, pull.Index, gitRepo.Path, err)
}
@@ -1395,11 +1352,11 @@ outer:
err := pull_model.UpdateReviewState(ctx, review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff)
if err != nil {
log.Warn("Could not update review for user %d, pull %d, commit %s and the changed files %v: %v", review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff, err)
- return err
+ return nil, err
}
}
- return nil
+ return review, nil
}
// CommentAsDiff returns c.Patch as *Diff
diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go
index 71394b1915..b84530043a 100644
--- a/services/gitdiff/gitdiff_test.go
+++ b/services/gitdiff/gitdiff_test.go
@@ -416,7 +416,7 @@ index 0000000..6bb8f39
`
diffBuilder.WriteString(diff)
- for i := 0; i < 35; i++ {
+ for i := range 35 {
diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n")
}
diff = diffBuilder.String()
@@ -453,11 +453,11 @@ index 0000000..6bb8f39
diffBuilder.Reset()
diffBuilder.WriteString(diff)
- for i := 0; i < 33; i++ {
+ for i := range 33 {
diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n")
}
diffBuilder.WriteString("+line33")
- for i := 0; i < 512; i++ {
+ for range 512 {
diffBuilder.WriteString("0123456789ABCDEF")
}
diffBuilder.WriteByte('\n')
diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go
index 6e18651d83..e8be063e69 100644
--- a/services/gitdiff/highlightdiff.go
+++ b/services/gitdiff/highlightdiff.go
@@ -14,13 +14,14 @@ import (
// token is a html tag or entity, eg: "<span ...>", "</span>", "&lt;"
func extractHTMLToken(s string) (before, token, after string, valid bool) {
for pos1 := 0; pos1 < len(s); pos1++ {
- if s[pos1] == '<' {
+ switch s[pos1] {
+ case '<':
pos2 := strings.IndexByte(s[pos1:], '>')
if pos2 == -1 {
return "", "", s, false
}
return s[:pos1], s[pos1 : pos1+pos2+1], s[pos1+pos2+1:], true
- } else if s[pos1] == '&' {
+ case '&':
pos2 := strings.IndexByte(s[pos1:], ';')
if pos2 == -1 {
return "", "", s, false
diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go
index c2584dc622..aebe38ae7c 100644
--- a/services/gitdiff/highlightdiff_test.go
+++ b/services/gitdiff/highlightdiff_test.go
@@ -38,15 +38,15 @@ func TestDiffWithHighlight(t *testing.T) {
hcd.placeholderTokenMap['O'], hcd.placeholderTokenMap['C'] = "<span>", "</span>"
assert.Equal(t, "<span></span>", string(hcd.recoverOneDiff("OC")))
assert.Equal(t, "<span></span>", string(hcd.recoverOneDiff("O")))
- assert.Equal(t, "", string(hcd.recoverOneDiff("C")))
+ assert.Empty(t, string(hcd.recoverOneDiff("C")))
})
}
func TestDiffWithHighlightPlaceholder(t *testing.T) {
hcd := newHighlightCodeDiff()
output := hcd.diffLineWithHighlight(DiffLineDel, "a='\U00100000'", "a='\U0010FFFD''")
- assert.Equal(t, "", hcd.placeholderTokenMap[0x00100000])
- assert.Equal(t, "", hcd.placeholderTokenMap[0x0010FFFD])
+ assert.Empty(t, hcd.placeholderTokenMap[0x00100000])
+ assert.Empty(t, hcd.placeholderTokenMap[0x0010FFFD])
expected := fmt.Sprintf(`a='<span class="removed-code">%s</span>'`, "\U00100000")
assert.Equal(t, expected, string(output))
diff --git a/services/gitdiff/submodule.go b/services/gitdiff/submodule.go
index 02ca666544..4347743e3d 100644
--- a/services/gitdiff/submodule.go
+++ b/services/gitdiff/submodule.go
@@ -20,7 +20,7 @@ type SubmoduleDiffInfo struct {
PreviousRefID string
}
-func (si *SubmoduleDiffInfo) PopulateURL(diffFile *DiffFile, leftCommit, rightCommit *git.Commit) {
+func (si *SubmoduleDiffInfo) PopulateURL(repoLink string, diffFile *DiffFile, leftCommit, rightCommit *git.Commit) {
si.SubmoduleName = diffFile.Name
submoduleCommit := rightCommit // If the submodule is added or updated, check at the right commit
if diffFile.IsDeleted {
@@ -30,18 +30,19 @@ func (si *SubmoduleDiffInfo) PopulateURL(diffFile *DiffFile, leftCommit, rightCo
return
}
- submodule, err := submoduleCommit.GetSubModule(diffFile.GetDiffFileName())
+ submoduleFullPath := diffFile.GetDiffFileName()
+ submodule, err := submoduleCommit.GetSubModule(submoduleFullPath)
if err != nil {
- log.Error("Unable to PopulateURL for submodule %q: GetSubModule: %v", diffFile.GetDiffFileName(), err)
+ log.Error("Unable to PopulateURL for submodule %q: GetSubModule: %v", submoduleFullPath, err)
return // ignore the error, do not cause 500 errors for end users
}
if submodule != nil {
- si.SubmoduleFile = git.NewCommitSubmoduleFile(submodule.URL, submoduleCommit.ID.String())
+ si.SubmoduleFile = git.NewCommitSubmoduleFile(repoLink, submoduleFullPath, submodule.URL, submoduleCommit.ID.String())
}
}
func (si *SubmoduleDiffInfo) CommitRefIDLinkHTML(ctx context.Context, commitID string) template.HTML {
- webLink := si.SubmoduleFile.SubmoduleWebLink(ctx, commitID)
+ webLink := si.SubmoduleFile.SubmoduleWebLinkTree(ctx, commitID)
if webLink == nil {
return htmlutil.HTMLFormat("%s", base.ShortSha(commitID))
}
@@ -49,7 +50,7 @@ func (si *SubmoduleDiffInfo) CommitRefIDLinkHTML(ctx context.Context, commitID s
}
func (si *SubmoduleDiffInfo) CompareRefIDLinkHTML(ctx context.Context) template.HTML {
- webLink := si.SubmoduleFile.SubmoduleWebLink(ctx, si.PreviousRefID, si.NewRefID)
+ webLink := si.SubmoduleFile.SubmoduleWebLinkCompare(ctx, si.PreviousRefID, si.NewRefID)
if webLink == nil {
return htmlutil.HTMLFormat("%s...%s", base.ShortSha(si.PreviousRefID), base.ShortSha(si.NewRefID))
}
@@ -57,7 +58,7 @@ func (si *SubmoduleDiffInfo) CompareRefIDLinkHTML(ctx context.Context) template.
}
func (si *SubmoduleDiffInfo) SubmoduleRepoLinkHTML(ctx context.Context) template.HTML {
- webLink := si.SubmoduleFile.SubmoduleWebLink(ctx)
+ webLink := si.SubmoduleFile.SubmoduleWebLinkTree(ctx)
if webLink == nil {
return htmlutil.HTMLFormat("%s", si.SubmoduleName)
}
diff --git a/services/gitdiff/submodule_test.go b/services/gitdiff/submodule_test.go
index 3047b23103..c793969f0e 100644
--- a/services/gitdiff/submodule_test.go
+++ b/services/gitdiff/submodule_test.go
@@ -203,7 +203,6 @@ index 0000000..68972a9
}
for _, testcase := range tests {
- testcase := testcase
t.Run(testcase.name, func(t *testing.T) {
diff, err := ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "")
assert.NoError(t, err)
@@ -228,7 +227,7 @@ func TestSubmoduleInfo(t *testing.T) {
assert.EqualValues(t, "aaaa...bbbb", sdi.CompareRefIDLinkHTML(ctx))
assert.EqualValues(t, "name", sdi.SubmoduleRepoLinkHTML(ctx))
- sdi.SubmoduleFile = git.NewCommitSubmoduleFile("https://github.com/owner/repo", "1234")
+ sdi.SubmoduleFile = git.NewCommitSubmoduleFile("/any/repo-link", "fullpath", "https://github.com/owner/repo", "1234")
assert.EqualValues(t, `<a href="https://github.com/owner/repo/tree/1111">1111</a>`, sdi.CommitRefIDLinkHTML(ctx, "1111"))
assert.EqualValues(t, `<a href="https://github.com/owner/repo/compare/aaaa...bbbb">aaaa...bbbb</a>`, sdi.CompareRefIDLinkHTML(ctx))
assert.EqualValues(t, `<a href="https://github.com/owner/repo">name</a>`, sdi.SubmoduleRepoLinkHTML(ctx))