mirror of
				https://github.com/go-gitea/gitea.git
				synced 2025-10-29 10:57:44 +09:00 
			
		
		
		
	* Fix diff skipping lines Backport #13154 ParsePatch previously just skipped all lines that start with "+++ " or "--- " and makes no attempt to see these lines in context. This PR rewrites ParsePatch to pay attention to context and position within a patch, ensuring that --- and +++ are only skipped if appropriate. This PR also fixes several issues with incomplete files. Fix https://codeberg.org/Codeberg/Community/issues/308 Fix #13153 Signed-off-by: Andrew Thornton <art27@cantab.net> * Add testcase Signed-off-by: Andrew Thornton <art27@cantab.net> * fix comment * simplify error handling Signed-off-by: Andrew Thornton <art27@cantab.net> * never return io.EOF Signed-off-by: Andrew Thornton <art27@cantab.net>
		
			
				
	
	
		
			330 lines
		
	
	
		
			8.9 KiB
		
	
	
	
		
			Go
		
	
	
	
	
	
			
		
		
	
	
			330 lines
		
	
	
		
			8.9 KiB
		
	
	
	
		
			Go
		
	
	
	
	
	
| // Copyright 2014 The Gogs Authors. All rights reserved.
 | |
| // Copyright 2019 The Gitea Authors. All rights reserved.
 | |
| // Use of this source code is governed by a MIT-style
 | |
| // license that can be found in the LICENSE file.
 | |
| 
 | |
| package gitdiff
 | |
| 
 | |
| import (
 | |
| 	"encoding/json"
 | |
| 	"fmt"
 | |
| 	"html/template"
 | |
| 	"strings"
 | |
| 	"testing"
 | |
| 
 | |
| 	"code.gitea.io/gitea/models"
 | |
| 	"code.gitea.io/gitea/modules/git"
 | |
| 	"code.gitea.io/gitea/modules/setting"
 | |
| 
 | |
| 	dmp "github.com/sergi/go-diff/diffmatchpatch"
 | |
| 	"github.com/stretchr/testify/assert"
 | |
| )
 | |
| 
 | |
| func assertEqual(t *testing.T, s1 string, s2 template.HTML) {
 | |
| 	if s1 != string(s2) {
 | |
| 		t.Errorf("%s should be equal %s", s2, s1)
 | |
| 	}
 | |
| }
 | |
| 
 | |
| func TestDiffToHTML(t *testing.T) {
 | |
| 	assertEqual(t, "foo <span class=\"added-code\">bar</span> biz", diffToHTML([]dmp.Diff{
 | |
| 		{Type: dmp.DiffEqual, Text: "foo "},
 | |
| 		{Type: dmp.DiffInsert, Text: "bar"},
 | |
| 		{Type: dmp.DiffDelete, Text: " baz"},
 | |
| 		{Type: dmp.DiffEqual, Text: " biz"},
 | |
| 	}, DiffLineAdd))
 | |
| 
 | |
| 	assertEqual(t, "foo <span class=\"removed-code\">bar</span> biz", diffToHTML([]dmp.Diff{
 | |
| 		{Type: dmp.DiffEqual, Text: "foo "},
 | |
| 		{Type: dmp.DiffDelete, Text: "bar"},
 | |
| 		{Type: dmp.DiffInsert, Text: " baz"},
 | |
| 		{Type: dmp.DiffEqual, Text: " biz"},
 | |
| 	}, DiffLineDel))
 | |
| }
 | |
| 
 | |
| func TestParsePatch_singlefile(t *testing.T) {
 | |
| 	type testcase struct {
 | |
| 		name        string
 | |
| 		gitdiff     string
 | |
| 		wantErr     bool
 | |
| 		addition    int
 | |
| 		deletion    int
 | |
| 		oldFilename string
 | |
| 		filename    string
 | |
| 	}
 | |
| 
 | |
| 	tests := []testcase{
 | |
| 		{
 | |
| 			name: "readme.md2readme.md",
 | |
| 			gitdiff: `diff --git "\\a/README.md" "\\b/README.md"
 | |
| --- "\\a/README.md"
 | |
| +++ "\\b/README.md"
 | |
| @@ -1,3 +1,6 @@
 | |
|  # gitea-github-migrator
 | |
| +
 | |
| + Build Status
 | |
| - Latest Release
 | |
|  Docker Pulls
 | |
| + cut off
 | |
| + cut off
 | |
| `,
 | |
| 			addition:    4,
 | |
| 			deletion:    1,
 | |
| 			filename:    "README.md",
 | |
| 			oldFilename: "README.md",
 | |
| 		},
 | |
| 		{
 | |
| 			name: "A \\ B",
 | |
| 			gitdiff: `diff --git "a/A \\ B" "b/A \\ B"
 | |
| --- "a/A \\ B"
 | |
| +++ "b/A \\ B"
 | |
| @@ -1,3 +1,6 @@
 | |
|  # gitea-github-migrator
 | |
| +
 | |
| + Build Status
 | |
| - Latest Release
 | |
|  Docker Pulls
 | |
| + cut off
 | |
| + cut off`,
 | |
| 			addition:    4,
 | |
| 			deletion:    1,
 | |
| 			filename:    "A \\ B",
 | |
| 			oldFilename: "A \\ B",
 | |
| 		},
 | |
| 		{
 | |
| 			name: "really weird filename",
 | |
| 			gitdiff: `diff --git "\\a/a b/file b/a a/file" "\\b/a b/file b/a a/file"
 | |
| index d2186f1..f5c8ed2 100644
 | |
| --- "\\a/a b/file b/a a/file"	
 | |
| +++ "\\b/a b/file b/a a/file"	
 | |
| @@ -1,3 +1,2 @@
 | |
|  Create a weird file.
 | |
|  
 | |
| -and what does diff do here?
 | |
| \ No newline at end of file`,
 | |
| 			addition:    0,
 | |
| 			deletion:    1,
 | |
| 			filename:    "a b/file b/a a/file",
 | |
| 			oldFilename: "a b/file b/a a/file",
 | |
| 		},
 | |
| 		{
 | |
| 			name: "delete file with blanks",
 | |
| 			gitdiff: `diff --git "\\a/file with blanks" "\\b/file with blanks"
 | |
| deleted file mode 100644
 | |
| index 898651a..0000000
 | |
| --- "\\a/file with blanks"	
 | |
| +++ /dev/null
 | |
| @@ -1,5 +0,0 @@
 | |
| -a blank file
 | |
| -
 | |
| -has a couple o line
 | |
| -
 | |
| -the 5th line is the last
 | |
| `,
 | |
| 			addition:    0,
 | |
| 			deletion:    5,
 | |
| 			filename:    "file with blanks",
 | |
| 			oldFilename: "file with blanks",
 | |
| 		},
 | |
| 		{
 | |
| 			name: "rename a—as",
 | |
| 			gitdiff: `diff --git "a/\360\243\220\265b\342\200\240vs" "b/a\342\200\224as"
 | |
| similarity index 100%
 | |
| rename from "\360\243\220\265b\342\200\240vs"
 | |
| rename to "a\342\200\224as"
 | |
| `,
 | |
| 			addition:    0,
 | |
| 			deletion:    0,
 | |
| 			oldFilename: "𣐵b†vs",
 | |
| 			filename:    "a—as",
 | |
| 		},
 | |
| 		{
 | |
| 			name: "rename with spaces",
 | |
| 			gitdiff: `diff --git "\\a/a b/file b/a a/file" "\\b/a b/a a/file b/b file"
 | |
| similarity index 100%
 | |
| rename from a b/file b/a a/file
 | |
| rename to a b/a a/file b/b file
 | |
| `,
 | |
| 			oldFilename: "a b/file b/a a/file",
 | |
| 			filename:    "a b/a a/file b/b file",
 | |
| 		},
 | |
| 		{
 | |
| 			name: "minuses-and-pluses",
 | |
| 			gitdiff: `diff --git a/minuses-and-pluses b/minuses-and-pluses
 | |
| index 6961180..9ba1a00 100644
 | |
| --- a/minuses-and-pluses
 | |
| +++ b/minuses-and-pluses
 | |
| @@ -1,4 +1,4 @@
 | |
| --- 1st line
 | |
| -++ 2nd line
 | |
| --- 3rd line
 | |
| -++ 4th line
 | |
| +++ 1st line
 | |
| +-- 2nd line
 | |
| +++ 3rd line
 | |
| +-- 4th line
 | |
| `,
 | |
| 			oldFilename: "minuses-and-pluses",
 | |
| 			filename:    "minuses-and-pluses",
 | |
| 			addition:    4,
 | |
| 			deletion:    4,
 | |
| 		},
 | |
| 	}
 | |
| 
 | |
| 	for _, testcase := range tests {
 | |
| 		t.Run(testcase.name, func(t *testing.T) {
 | |
| 			got, err := ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff))
 | |
| 			if (err != nil) != testcase.wantErr {
 | |
| 				t.Errorf("ParsePatch() error = %v, wantErr %v", err, testcase.wantErr)
 | |
| 				return
 | |
| 			}
 | |
| 			gotMarshaled, _ := json.MarshalIndent(got, "  ", "  ")
 | |
| 			if got.NumFiles() != 1 {
 | |
| 				t.Errorf("ParsePath() did not receive 1 file:\n%s", string(gotMarshaled))
 | |
| 				return
 | |
| 			}
 | |
| 			if got.TotalAddition != testcase.addition {
 | |
| 				t.Errorf("ParsePath() does not have correct totalAddition %d, wanted %d", got.TotalAddition, testcase.addition)
 | |
| 			}
 | |
| 			if got.TotalDeletion != testcase.deletion {
 | |
| 				t.Errorf("ParsePath() did not have correct totalDeletion %d, wanted %d", got.TotalDeletion, testcase.deletion)
 | |
| 			}
 | |
| 			file := got.Files[0]
 | |
| 			if file.Addition != testcase.addition {
 | |
| 				t.Errorf("ParsePath() does not have correct file addition %d, wanted %d", file.Addition, testcase.addition)
 | |
| 			}
 | |
| 			if file.Deletion != testcase.deletion {
 | |
| 				t.Errorf("ParsePath() did not have correct file deletion %d, wanted %d", file.Deletion, testcase.deletion)
 | |
| 			}
 | |
| 			if file.OldName != testcase.oldFilename {
 | |
| 				t.Errorf("ParsePath() did not have correct OldName %s, wanted %s", file.OldName, testcase.oldFilename)
 | |
| 			}
 | |
| 			if file.Name != testcase.filename {
 | |
| 				t.Errorf("ParsePath() did not have correct Name %s, wanted %s", file.Name, testcase.filename)
 | |
| 			}
 | |
| 		})
 | |
| 	}
 | |
| 
 | |
| 	var diff = `diff --git "a/README.md" "b/README.md"
 | |
| --- a/README.md
 | |
| +++ b/README.md
 | |
| @@ -1,3 +1,6 @@
 | |
|  # gitea-github-migrator
 | |
| +
 | |
| + Build Status
 | |
| - Latest Release
 | |
|  Docker Pulls
 | |
| + cut off
 | |
| + cut off`
 | |
| 	result, err := ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
 | |
| 	if err != nil {
 | |
| 		t.Errorf("ParsePatch failed: %s", err)
 | |
| 	}
 | |
| 	println(result)
 | |
| 
 | |
| 	var diff2 = `diff --git "a/A \\ B" "b/A \\ B"
 | |
| --- "a/A \\ B"
 | |
| +++ "b/A \\ B"
 | |
| @@ -1,3 +1,6 @@
 | |
|  # gitea-github-migrator
 | |
| +
 | |
| + Build Status
 | |
| - Latest Release
 | |
|  Docker Pulls
 | |
| + cut off
 | |
| + cut off`
 | |
| 	result, err = ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff2))
 | |
| 	if err != nil {
 | |
| 		t.Errorf("ParsePatch failed: %s", err)
 | |
| 	}
 | |
| 	println(result)
 | |
| 
 | |
| 	var diff2a = `diff --git "a/A \\ B" b/A/B
 | |
| --- "a/A \\ B"
 | |
| +++ b/A/B
 | |
| @@ -1,3 +1,6 @@
 | |
|  # gitea-github-migrator
 | |
| +
 | |
| + Build Status
 | |
| - Latest Release
 | |
|  Docker Pulls
 | |
| + cut off
 | |
| + cut off`
 | |
| 	result, err = ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff2a))
 | |
| 	if err != nil {
 | |
| 		t.Errorf("ParsePatch failed: %s", err)
 | |
| 	}
 | |
| 	println(result)
 | |
| 
 | |
| 	var diff3 = `diff --git a/README.md b/README.md
 | |
| --- a/README.md
 | |
| +++ b/README.md
 | |
| @@ -1,3 +1,6 @@
 | |
|  # gitea-github-migrator
 | |
| +
 | |
| + Build Status
 | |
| - Latest Release
 | |
|  Docker Pulls
 | |
| + cut off
 | |
| + cut off`
 | |
| 	result, err = ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff3))
 | |
| 	if err != nil {
 | |
| 		t.Errorf("ParsePatch failed: %s", err)
 | |
| 	}
 | |
| 	println(result)
 | |
| }
 | |
| 
 | |
| func setupDefaultDiff() *Diff {
 | |
| 	return &Diff{
 | |
| 		Files: []*DiffFile{
 | |
| 			{
 | |
| 				Name: "README.md",
 | |
| 				Sections: []*DiffSection{
 | |
| 					{
 | |
| 						Lines: []*DiffLine{
 | |
| 							{
 | |
| 								LeftIdx:  4,
 | |
| 								RightIdx: 4,
 | |
| 							},
 | |
| 						},
 | |
| 					},
 | |
| 				},
 | |
| 			},
 | |
| 		},
 | |
| 	}
 | |
| }
 | |
| func TestDiff_LoadComments(t *testing.T) {
 | |
| 	assert.NoError(t, models.PrepareTestDatabase())
 | |
| 
 | |
| 	issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 2}).(*models.Issue)
 | |
| 	user := models.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User)
 | |
| 	diff := setupDefaultDiff()
 | |
| 	assert.NoError(t, diff.LoadComments(issue, user))
 | |
| 	assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 2)
 | |
| }
 | |
| 
 | |
| func TestDiffLine_CanComment(t *testing.T) {
 | |
| 	assert.False(t, (&DiffLine{Type: DiffLineSection}).CanComment())
 | |
| 	assert.False(t, (&DiffLine{Type: DiffLineAdd, Comments: []*models.Comment{{Content: "bla"}}}).CanComment())
 | |
| 	assert.True(t, (&DiffLine{Type: DiffLineAdd}).CanComment())
 | |
| 	assert.True(t, (&DiffLine{Type: DiffLineDel}).CanComment())
 | |
| 	assert.True(t, (&DiffLine{Type: DiffLinePlain}).CanComment())
 | |
| }
 | |
| 
 | |
| func TestDiffLine_GetCommentSide(t *testing.T) {
 | |
| 	assert.Equal(t, "previous", (&DiffLine{Comments: []*models.Comment{{Line: -3}}}).GetCommentSide())
 | |
| 	assert.Equal(t, "proposed", (&DiffLine{Comments: []*models.Comment{{Line: 3}}}).GetCommentSide())
 | |
| }
 | |
| 
 | |
| func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) {
 | |
| 	git.Debug = true
 | |
| 	for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} {
 | |
| 		diffs, err := GetDiffRangeWithWhitespaceBehavior("./testdata/academic-module", "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9",
 | |
| 			setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior)
 | |
| 		assert.NoError(t, err, fmt.Sprintf("Error when diff with %s", behavior))
 | |
| 		for _, f := range diffs.Files {
 | |
| 			assert.True(t, len(f.Sections) > 0, fmt.Sprintf("%s should have sections", f.Name))
 | |
| 		}
 | |
| 	}
 | |
| }
 |