Fix number of files, total additions, and deletions on Diff pages

modules/git/repo_compare.go

@@ -6,9 +6,11 @@
 package git
 
 import (
+	"bytes"
 	"container/list"
 	"fmt"
 	"io"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -84,14 +86,97 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string)
 	}
 
 	// Count number of changed files.
-	stdout, err := NewCommand("diff", "--name-only", remoteBranch+"..."+headBranch).RunInDir(repo.Path)
+	// This probably should be removed as we need to use shortstat elsewhere
+	// Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly
+	compareInfo.NumFiles, err = repo.GetDiffNumChangedFiles(remoteBranch, headBranch)
 	if err != nil {
 		return nil, err
 	}
-	compareInfo.NumFiles = len(strings.Split(stdout, "\n")) - 1
 	return compareInfo, nil
 }
 
+type lineCountWriter struct {
+	numLines int
+}
+
+// Write counts the number of newlines in the provided bytestream
+func (l *lineCountWriter) Write(p []byte) (n int, err error) {
+	n = len(p)
+	l.numLines += bytes.Count(p, []byte{'\000'})
+	return
+}
+
+// GetDiffNumChangedFiles counts the number of changed files
+// This is substantially quicker than shortstat but...
+func (repo *Repository) GetDiffNumChangedFiles(base, head string) (int, error) {
+	// Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly
+	w := &lineCountWriter{}
+	stderr := new(bytes.Buffer)
+
+	if err := NewCommand("diff", "-z", "--name-only", base+"..."+head).
+		RunInDirPipeline(repo.Path, w, stderr); err != nil {
+		return 0, fmt.Errorf("%v: Stderr: %s", err, stderr)
+	}
+	return w.numLines, nil
+}
+
+// GetDiffShortStat counts number of changed files, number of additions and deletions
+func (repo *Repository) GetDiffShortStat(base, head string) (numFiles, totalAdditions, totalDeletions int, err error) {
+	return GetDiffShortStat(repo.Path, base+"..."+head)
+}
+
+// GetDiffShortStat counts number of changed files, number of additions and deletions
+func GetDiffShortStat(repoPath string, args ...string) (numFiles, totalAdditions, totalDeletions int, err error) {
+	// Now if we call:
+	// $ git diff --shortstat 1ebb35b98889ff77299f24d82da426b434b0cca0...788b8b1440462d477f45b0088875
+	// we get:
+	// " 9902 files changed, 2034198 insertions(+), 298800 deletions(-)\n"
+	args = append([]string{
+		"diff",
+		"--shortstat",
+	}, args...)
+
+	stdout, err := NewCommand(args...).RunInDir(repoPath)
+	if err != nil {
+		return 0, 0, 0, err
+	}
+
+	return parseDiffStat(stdout)
+}
+
+var shortStatFormat = regexp.MustCompile(
+	`\s*(\d+) files? changed(?:, (\d+) insertions?\(\+\))?(?:, (\d+) deletions?\(-\))?`)
+
+func parseDiffStat(stdout string) (numFiles, totalAdditions, totalDeletions int, err error) {
+	if len(stdout) == 0 || stdout == "\n" {
+		return 0, 0, 0, nil
+	}
+	groups := shortStatFormat.FindStringSubmatch(stdout)
+	if len(groups) != 4 {
+		return 0, 0, 0, fmt.Errorf("unable to parse shortstat: %s groups: %s", stdout, groups)
+	}
+
+	numFiles, err = strconv.Atoi(groups[1])
+	if err != nil {
+		return 0, 0, 0, fmt.Errorf("unable to parse shortstat: %s. Error parsing NumFiles %v", stdout, err)
+	}
+
+	if len(groups[2]) != 0 {
+		totalAdditions, err = strconv.Atoi(groups[2])
+		if err != nil {
+			return 0, 0, 0, fmt.Errorf("unable to parse shortstat: %s. Error parsing NumAdditions %v", stdout, err)
+		}
+	}
+
+	if len(groups[3]) != 0 {
+		totalDeletions, err = strconv.Atoi(groups[3])
+		if err != nil {
+			return 0, 0, 0, fmt.Errorf("unable to parse shortstat: %s. Error parsing NumDeletions %v", stdout, err)
+		}
+	}
+	return
+}
+
 // GetDiffOrPatch generates either diff or formatted patch data between given revisions
 func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, formatted bool) error {
 	if formatted {

modules/repofiles/diff_test.go

@@ -108,6 +108,7 @@ func TestGetDiffPreview(t *testing.T) {
 		},
 		IsIncomplete: false,
 	}
+	expectedDiff.NumFiles = len(expectedDiff.Files)
 
 	t.Run("with given branch", func(t *testing.T) {
 		diff, err := GetDiffPreview(ctx.Repo.Repository, branch, treePath, content)

modules/repofiles/temp_repo.go

@@ -299,6 +299,11 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) {
 			t.repo.FullName(), err, stderr)
 	}
 
+	diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.basePath, "--cached", "HEAD")
+	if err != nil {
+		return nil, err
+	}
+
 	return diff, nil
 }
 

routers/repo/commit.go

@@ -291,7 +291,7 @@ func Diff(ctx *context.Context) {
 	ctx.Data["Author"] = models.ValidateCommitWithEmail(commit)
 	ctx.Data["Diff"] = diff
 	ctx.Data["Parents"] = parents
-	ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0
+	ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0
 
 	if err := models.CalculateTrustStatus(verification, ctx.Repo.Repository, nil); err != nil {
 		ctx.ServerError("CalculateTrustStatus", err)

routers/repo/compare.go

@@ -437,7 +437,7 @@ func PrepareCompareDiff(
 		return false
 	}
 	ctx.Data["Diff"] = diff
-	ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0
+	ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0
 
 	headCommit, err := headGitRepo.GetCommit(headCommitID)
 	if err != nil {

routers/repo/editor.go

@@ -339,7 +339,7 @@ func DiffPreviewPost(ctx *context.Context, form auth.EditPreviewDiffForm) {
 		return
 	}
 
-	if diff.NumFiles() == 0 {
+	if diff.NumFiles == 0 {
 		ctx.PlainText(200, []byte(ctx.Tr("repo.editor.no_changes_to_show")))
 		return
 	}

routers/repo/pull.go

@@ -611,7 +611,7 @@ func ViewPullFiles(ctx *context.Context) {
 	}
 
 	ctx.Data["Diff"] = diff
-	ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0
+	ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0
 
 	baseCommit, err := ctx.Repo.GitRepo.GetCommit(startCommitID)
 	if err != nil {

services/gitdiff/gitdiff.go

@@ -367,9 +367,9 @@ func getCommitFileLineCount(commit *git.Commit, filePath string) int {
 
 // Diff represents a difference between two git trees.
 type Diff struct {
-	TotalAddition, TotalDeletion int
-	Files                        []*DiffFile
-	IsIncomplete                 bool
+	NumFiles, TotalAddition, TotalDeletion int
+	Files                                  []*DiffFile
+	IsIncomplete                           bool
 }
 
 // LoadComments loads comments into each line
@@ -398,11 +398,6 @@ func (diff *Diff) LoadComments(issue *models.Issue, currentUser *models.User) er
 	return nil
 }
 
-// NumFiles returns number of files changes in a diff.
-func (diff *Diff) NumFiles() int {
-	return len(diff.Files)
-}
-
 const cmdDiffHead = "diff --git "
 
 // ParsePatch builds a Diff object from a io.Reader and some
@@ -639,7 +634,7 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
 			}
 		}
 	}
-
+	diff.NumFiles = len(diff.Files)
 	return diff, nil
 }
 
@@ -716,6 +711,11 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
 		return nil, fmt.Errorf("Wait: %v", err)
 	}
 
+	diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(repoPath, beforeCommitID+"..."+afterCommitID)
+	if err != nil {
+		return nil, err
+	}
+
 	return diff, nil
 }