Skip to content
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

Fix ParsePatch function to work with quoted diff --git strings #6323

Merged
merged 3 commits into from Mar 14, 2019
Merged
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -550,7 +550,13 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
beg := len(cmdDiffHead)
a := line[beg+2 : middle]
b := line[middle+3:]

if hasQuote {
// When /a and /b are surrounded by double quotes, we want to first
// make sure we keep everything the quotes and then strip out the leading /a /b
a = strings.Replace(line[beg:middle], "\"a/", "\"", -1)

This comment has been minimized.

Copy link
@zeripath

zeripath Mar 14, 2019

Contributor

Why don't we just unquote then remove the first two runes. Using replace here is just complicating things.

Consider the, admittedly unusual case, of a file called: /a/a/a/a which is git mv and editted to /b/b/b/b. Your replace will just remove the entire names. (Yes you could change the replace to only replace one time but removing the first two characters is your aim.)

This comment has been minimized.

Copy link
@mrsdizzie

mrsdizzie Mar 14, 2019

Author Contributor

Ah yea that makes more sense/less complicated thx. Slipped my mind to change the string after unquoting. I've changed it to just remove the first 2 characters afterwards as you've suggested thx!

I agree entire integration tests are a good goal but I don't want to complicate or hold up what is otherwise just a couple line bug fix + adding a unit test here.

I'll look into understanding the integration testing more and perhaps help with something in a separate request if I can.

b = strings.Replace(line[middle+1:], "\"b/", "\"", -1)

var err error
a, err = strconv.Unquote(a)
if err != nil {
@@ -637,6 +643,7 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
}
}
}

return diff, nil
}

@@ -5,6 +5,8 @@ import (
"strings"
"testing"

"code.gitea.io/gitea/modules/setting"

dmp "github.com/sergi/go-diff/diffmatchpatch"
"github.com/stretchr/testify/assert"
)
@@ -99,6 +101,59 @@ func ExampleCutDiffAroundLine() {
println(result)
}

func TestParsePatch(t *testing.T) {
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 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{
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.