Fix yet another bug with diff file names (#12771)

Following further testing it has become apparent that the diff line
cannot be used to determine filenames for diffs with any sort of predictability
the answer therefore is to use the other lines that are provided with a diff

Fix #12768

Signed-off-by: Andrew Thornton <art27@cantab.net>

1 files changed, 105 insertions, 46 deletions

diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go
index 538f613b04..4cea5dd9a0 100644
--- a/services/gitdiff/gitdiff.go
+++ b/services/gitdiff/gitdiff.go
@@ -53,6 +53,7 @@ const (
 	DiffFileChange
 	DiffFileDel
 	DiffFileRename
+	DiffFileCopy
 )
 
 // DiffLineExpandDirection represents the DiffLineSection expand direction
@@ -481,7 +482,46 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
 		}
 		line := linebuf.String()
 
-		if strings.HasPrefix(line, "+++ ") || strings.HasPrefix(line, "--- ") || len(line) == 0 {
+		if strings.HasPrefix(line, "--- ") {
+			if line[4] == '"' {
+				fmt.Sscanf(line[4:], "%q", &curFile.OldName)
+			} else {
+				curFile.OldName = line[4:]
+				if strings.Contains(curFile.OldName, " ") {
+					// Git adds a terminal \t if there is a space in the name
+					curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
+				}
+			}
+			if curFile.OldName[0:2] == "a/" {
+				curFile.OldName = curFile.OldName[2:]
+			}
+			continue
+		} else if strings.HasPrefix(line, "+++ ") {
+			if line[4] == '"' {
+				fmt.Sscanf(line[4:], "%q", &curFile.Name)
+			} else {
+				curFile.Name = line[4:]
+				if strings.Contains(curFile.Name, " ") {
+					// Git adds a terminal \t if there is a space in the name
+					curFile.Name = curFile.Name[:len(curFile.Name)-1]
+				}
+			}
+			if curFile.Name[0:2] == "b/" {
+				curFile.Name = curFile.Name[2:]
+			}
+			curFile.IsRenamed = (curFile.Name != curFile.OldName) && !(curFile.IsCreated || curFile.IsDeleted)
+			if curFile.IsDeleted {
+				curFile.Name = curFile.OldName
+				curFile.OldName = ""
+			} else if curFile.IsCreated {
+				curFile.OldName = ""
+			}
+			continue
+		} else if len(line) == 0 {
+			continue
+		}
+
+		if strings.HasPrefix(line, "+++") || strings.HasPrefix(line, "---") || len(line) == 0 {
 			continue
 		}
 
@@ -569,36 +609,10 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
 				break
 			}
 
-			// Note: In case file name is surrounded by double quotes (it happens only in git-shell).
-			// e.g. diff --git "a/xxx" "b/xxx"
-			var a string
-			var b string
-
-			rd := strings.NewReader(line[len(cmdDiffHead):])
-			char, _ := rd.ReadByte()
-			_ = rd.UnreadByte()
-			if char == '"' {
-				fmt.Fscanf(rd, "%q ", &a)
-			} else {
-				fmt.Fscanf(rd, "%s ", &a)
-			}
-			char, _ = rd.ReadByte()
-			_ = rd.UnreadByte()
-			if char == '"' {
-				fmt.Fscanf(rd, "%q", &b)
-			} else {
-				fmt.Fscanf(rd, "%s", &b)
-			}
-			a = a[2:]
-			b = b[2:]
-
 			curFile = &DiffFile{
-				Name:      b,
-				OldName:   a,
-				Index:     len(diff.Files) + 1,
-				Type:      DiffFileChange,
-				Sections:  make([]*DiffSection, 0, 10),
-				IsRenamed: a != b,
+				Index:    len(diff.Files) + 1,
+				Type:     DiffFileChange,
+				Sections: make([]*DiffSection, 0, 10),
 			}
 			diff.Files = append(diff.Files, curFile)
 			curFileLinesCount = 0
@@ -607,6 +621,7 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
 			curFileLFSPrefix = false
 
 			// Check file diff type and is submodule.
+		loop:
 			for {
 				line, err := input.ReadString('\n')
 				if err != nil {
@@ -617,23 +632,67 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
 					}
 				}
 
-				switch {
-				case strings.HasPrefix(line, "new file"):
-					curFile.Type = DiffFileAdd
-					curFile.IsCreated = true
-				case strings.HasPrefix(line, "deleted"):
-					curFile.Type = DiffFileDel
-					curFile.IsDeleted = true
-				case strings.HasPrefix(line, "index"):
-					curFile.Type = DiffFileChange
-				case strings.HasPrefix(line, "similarity index 100%"):
-					curFile.Type = DiffFileRename
-				}
-				if curFile.Type > 0 {
-					if strings.HasSuffix(line, " 160000\n") {
-						curFile.IsSubmodule = true
+				if curFile.Type != DiffFileRename {
+					switch {
+					case strings.HasPrefix(line, "new file"):
+						curFile.Type = DiffFileAdd
+						curFile.IsCreated = true
+					case strings.HasPrefix(line, "deleted"):
+						curFile.Type = DiffFileDel
+						curFile.IsDeleted = true
+					case strings.HasPrefix(line, "index"):
+						curFile.Type = DiffFileChange
+					case strings.HasPrefix(line, "similarity index 100%"):
+						curFile.Type = DiffFileRename
+					}
+					if curFile.Type > 0 && curFile.Type != DiffFileRename {
+						if strings.HasSuffix(line, " 160000\n") {
+							curFile.IsSubmodule = true
+						}
+						break
+					}
+				} else {
+					switch {
+					case strings.HasPrefix(line, "rename from "):
+						if line[12] == '"' {
+							fmt.Sscanf(line[12:], "%q", &curFile.OldName)
+						} else {
+							curFile.OldName = line[12:]
+							curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
+						}
+					case strings.HasPrefix(line, "rename to "):
+						if line[10] == '"' {
+							fmt.Sscanf(line[10:], "%q", &curFile.Name)
+						} else {
+							curFile.Name = line[10:]
+							curFile.Name = curFile.Name[:len(curFile.Name)-1]
+						}
+						curFile.IsRenamed = true
+						break loop
+					case strings.HasPrefix(line, "copy from "):
+						if line[10] == '"' {
+							fmt.Sscanf(line[10:], "%q", &curFile.OldName)
+						} else {
+							curFile.OldName = line[10:]
+							curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
+						}
+					case strings.HasPrefix(line, "copy to "):
+						if line[8] == '"' {
+							fmt.Sscanf(line[8:], "%q", &curFile.Name)
+						} else {
+							curFile.Name = line[8:]
+							curFile.Name = curFile.Name[:len(curFile.Name)-1]
+						}
+						curFile.IsRenamed = true
+						curFile.Type = DiffFileCopy
+						break loop
+					default:
+						if strings.HasSuffix(line, " 160000\n") {
+							curFile.IsSubmodule = true
+						} else {
+							break loop
+						}
 					}
-					break
 				}
 			}
 		}