Skip to content

Commit 8c2b61c

Browse files
zeripathzeripath
committed
Finally fix diff names
go-gitea#12771 attempted to fix diff by avoiding the git diff line as it is possible to have an ambiguous line here. go-gitea#12254 attempted to fix diff by assuming that names would quoted if they needed to be and if one was quoted then both would be. Both of these were wrong. I have now discovered `--src-prefix` and `--dst-prefix` which means that we can set this in such a way to force the git diff to always be unambiguous. Therefore this PR rollsback most of the changes in go-gitea#12771 and uses these options to fix this. Signed-off-by: Andrew Thornton <art27@cantab.net>
1 parent da32d0e commit 8c2b61c

File tree

2 files changed

+61
-106
lines changed

2 files changed

+61
-106
lines changed

‎modules/repofiles/temp_repo.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) {
292292
var diff *gitdiff.Diff
293293
var finalErr error
294294

295-
if err := git.NewCommand("diff-index", "--cached", "-p", "HEAD").
295+
if err := git.NewCommand("diff-index", "--src-prefix=\\a/", "--dest-prefix=\\b/", "--cached", "-p", "HEAD").
296296
RunInDirTimeoutEnvFullPipelineFunc(nil, 30*time.Second, t.basePath, stdoutWriter, stderr, nil, func(ctx context.Context, cancel context.CancelFunc) error {
297297
_ = stdoutWriter.Close()
298298
diff, finalErr = gitdiff.ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader)

‎services/gitdiff/gitdiff.go‎

Lines changed: 60 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -482,46 +482,7 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
482482
}
483483
line := linebuf.String()
484484

485-
if strings.HasPrefix(line, "--- ") {
486-
if line[4] == '"' {
487-
fmt.Sscanf(line[4:], "%q", &curFile.OldName)
488-
} else {
489-
curFile.OldName = line[4:]
490-
if strings.Contains(curFile.OldName, " ") {
491-
// Git adds a terminal \t if there is a space in the name
492-
curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
493-
}
494-
}
495-
if curFile.OldName[0:2] == "a/" {
496-
curFile.OldName = curFile.OldName[2:]
497-
}
498-
continue
499-
} else if strings.HasPrefix(line, "+++ ") {
500-
if line[4] == '"' {
501-
fmt.Sscanf(line[4:], "%q", &curFile.Name)
502-
} else {
503-
curFile.Name = line[4:]
504-
if strings.Contains(curFile.Name, " ") {
505-
// Git adds a terminal \t if there is a space in the name
506-
curFile.Name = curFile.Name[:len(curFile.Name)-1]
507-
}
508-
}
509-
if curFile.Name[0:2] == "b/" {
510-
curFile.Name = curFile.Name[2:]
511-
}
512-
curFile.IsRenamed = (curFile.Name != curFile.OldName) && !(curFile.IsCreated || curFile.IsDeleted)
513-
if curFile.IsDeleted {
514-
curFile.Name = curFile.OldName
515-
curFile.OldName = ""
516-
} else if curFile.IsCreated {
517-
curFile.OldName = ""
518-
}
519-
continue
520-
} else if len(line) == 0 {
521-
continue
522-
}
523-
524-
if strings.HasPrefix(line, "+++") || strings.HasPrefix(line, "---") || len(line) == 0 {
485+
if strings.HasPrefix(line, "+++ ") || strings.HasPrefix(line, "--- ") || len(line) == 0 {
525486
continue
526487
}
527488

@@ -609,10 +570,43 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
609570
break
610571
}
611572

573+
// Note: In case file name is surrounded by double quotes (it happens only in git-shell).
574+
// e.g. diff --git "a/xxx" "b/xxx"
575+
var a string
576+
var b string
577+
578+
rd := strings.NewReader(line[len(cmdDiffHead):])
579+
char, _ := rd.ReadByte()
580+
_ = rd.UnreadByte()
581+
if char == '"' {
582+
fmt.Fscanf(rd, "%q ", &a)
583+
fmt.Printf("read %s", a)
584+
if a[0] == '\\' {
585+
a = a[1:]
586+
}
587+
} else {
588+
fmt.Fscanf(rd, "%s ", &a)
589+
}
590+
char, _ = rd.ReadByte()
591+
_ = rd.UnreadByte()
592+
if char == '"' {
593+
fmt.Fscanf(rd, "%q", &b)
594+
if b[0] == '\\' {
595+
b = b[1:]
596+
}
597+
} else {
598+
fmt.Fscanf(rd, "%s", &b)
599+
}
600+
a = a[2:]
601+
b = b[2:]
602+
612603
curFile = &DiffFile{
613-
Index: len(diff.Files) + 1,
614-
Type: DiffFileChange,
615-
Sections: make([]*DiffSection, 0, 10),
604+
Name: b,
605+
OldName: a,
606+
Index: len(diff.Files) + 1,
607+
Type: DiffFileChange,
608+
Sections: make([]*DiffSection, 0, 10),
609+
IsRenamed: a != b,
616610
}
617611
diff.Files = append(diff.Files, curFile)
618612
curFileLinesCount = 0
@@ -621,7 +615,6 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
621615
curFileLFSPrefix = false
622616

623617
// Check file diff type and is submodule.
624-
loop:
625618
for {
626619
line, err := input.ReadString('\n')
627620
if err != nil {
@@ -632,67 +625,29 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
632625
}
633626
}
634627

635-
if curFile.Type != DiffFileRename {
636-
switch {
637-
case strings.HasPrefix(line, "new file"):
638-
curFile.Type = DiffFileAdd
639-
curFile.IsCreated = true
640-
case strings.HasPrefix(line, "deleted"):
641-
curFile.Type = DiffFileDel
642-
curFile.IsDeleted = true
643-
case strings.HasPrefix(line, "index"):
644-
curFile.Type = DiffFileChange
645-
case strings.HasPrefix(line, "similarity index 100%"):
646-
curFile.Type = DiffFileRename
647-
}
648-
if curFile.Type > 0 && curFile.Type != DiffFileRename {
649-
if strings.HasSuffix(line, " 160000\n") {
650-
curFile.IsSubmodule = true
651-
}
652-
break
653-
}
654-
} else {
655-
switch {
656-
case strings.HasPrefix(line, "rename from "):
657-
if line[12] == '"' {
658-
fmt.Sscanf(line[12:], "%q", &curFile.OldName)
659-
} else {
660-
curFile.OldName = line[12:]
661-
curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
662-
}
663-
case strings.HasPrefix(line, "rename to "):
664-
if line[10] == '"' {
665-
fmt.Sscanf(line[10:], "%q", &curFile.Name)
666-
} else {
667-
curFile.Name = line[10:]
668-
curFile.Name = curFile.Name[:len(curFile.Name)-1]
669-
}
670-
curFile.IsRenamed = true
671-
break loop
672-
case strings.HasPrefix(line, "copy from "):
673-
if line[10] == '"' {
674-
fmt.Sscanf(line[10:], "%q", &curFile.OldName)
675-
} else {
676-
curFile.OldName = line[10:]
677-
curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
678-
}
679-
case strings.HasPrefix(line, "copy to "):
680-
if line[8] == '"' {
681-
fmt.Sscanf(line[8:], "%q", &curFile.Name)
682-
} else {
683-
curFile.Name = line[8:]
684-
curFile.Name = curFile.Name[:len(curFile.Name)-1]
685-
}
686-
curFile.IsRenamed = true
687-
curFile.Type = DiffFileCopy
688-
break loop
689-
default:
690-
if strings.HasSuffix(line, " 160000\n") {
691-
curFile.IsSubmodule = true
692-
} else {
693-
break loop
694-
}
628+
switch {
629+
case strings.HasPrefix(line, "copy from "):
630+
curFile.IsRenamed = true
631+
curFile.Type = DiffFileCopy
632+
case strings.HasPrefix(line, "copy to "):
633+
curFile.IsRenamed = true
634+
curFile.Type = DiffFileCopy
635+
case strings.HasPrefix(line, "new file"):
636+
curFile.Type = DiffFileAdd
637+
curFile.IsCreated = true
638+
case strings.HasPrefix(line, "deleted"):
639+
curFile.Type = DiffFileDel
640+
curFile.IsDeleted = true
641+
case strings.HasPrefix(line, "index"):
642+
curFile.Type = DiffFileChange
643+
case strings.HasPrefix(line, "similarity index 100%"):
644+
curFile.Type = DiffFileRename
645+
}
646+
if curFile.Type > 0 {
647+
if strings.HasSuffix(line, " 160000\n") {
648+
curFile.IsSubmodule = true
695649
}
650+
break
696651
}
697652
}
698653
}
@@ -761,7 +716,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
761716
parentCommit, _ := commit.Parent(0)
762717
actualBeforeCommitID = parentCommit.ID.String()
763718
}
764-
diffArgs := []string{"diff", "-M"}
719+
diffArgs := []string{"diff", "--src-prefix=\\a/", "--dst-prefix=\\d/", "-M"}
765720
if len(whitespaceBehavior) != 0 {
766721
diffArgs = append(diffArgs, whitespaceBehavior)
767722
}

0 commit comments

Comments
 (0)