Simplify Git blame implementation

.github/workflows/go.yml

@@ -19,7 +19,7 @@ jobs:
     name: Test
     strategy:
       matrix:
-        go-version: [1.13.x, 1.14.x]
+        go-version: [1.14.x, 1.15.x]
         platform: [ubuntu-latest, macos-latest, windows-latest]
     runs-on: ${{ matrix.platform }}
     steps:

blame.go

@@ -4,143 +4,16 @@
 
 package git
 
-import (
-	"bufio"
-	"bytes"
-	"strconv"
-	"strings"
-	"time"
-)
-
-type (
-	Blame struct {
-		commits map[int]*Commit
-	}
-	BlameOptions struct {
-	}
-)
-
-// BlameFile returns map of line number and the Commit changed that line.
-func (r *Repository) BlameFile(rev, file string, opts ...BlameOptions) (*Blame, error) {
-	cmd := NewCommand("blame", "-p", rev, "--", file)
-	stdout, err := cmd.RunInDir(r.path)
-	if err != nil {
-		return nil, err
-	}
-	return BlameContent(stdout)
-}
-
-// BlameContent parse content of `git blame` in porcelain format
-func BlameContent(content []byte) (*Blame, error) {
-	var commits = make(map[[20]byte]*Commit)
-	var commit = &Commit{}
-	var details = make(map[string]string)
-	var result = createBlame()
-	scanner := bufio.NewScanner(bytes.NewReader(content))
-	for scanner.Scan() {
-		line := scanner.Text()
-		if string(line[0]) != "\t" {
-			words := strings.Fields(line)
-			sha, err := NewIDFromString(words[0])
-			if err == nil {
-				// SHA and rows numbers line
-				commit = getCommit(sha, commits)
-				commit.fill(details)
-				details = make(map[string]string) // empty all details
-				i, err := strconv.Atoi(words[2])
-				if err != nil {
-					return nil, err
-				}
-				result.commits[i] = commit
-			} else {
-				// commit details line
-				switch words[0] {
-				case "summary":
-					commit.Message = line[len(words[0])+1:]
-				case "previous":
-					commit.parents = []*SHA1{MustIDFromString(words[1])}
-				default:
-					if len(words) > 1 {
-						details[words[0]] = line[len(words[0])+1:]
-					}
-				}
-			}
-		} else {
-			// needed for last line in blame
-			commit.fill(details)
-		}
-	}
-
-	return result, nil
-}
-
-func createBlame() *Blame {
-	var blame = Blame{}
-	blame.commits = make(map[int]*Commit)
-	return &blame
+// Blame contains information of a Git file blame.
+type Blame struct {
+	lines []*Commit
 }
 
-// Return commit from map or creates a new one
-func getCommit(sha *SHA1, commits map[[20]byte]*Commit) *Commit {
-	commit, ok := commits[sha.bytes]
-	if !ok {
-		commit = &Commit{
-			ID: sha,
-		}
-		commits[sha.bytes] = commit
-	}
-
-	return commit
-}
-
-func (c *Commit) fill(data map[string]string) {
-	author, ok := data["author"]
-	if ok && c.Author == nil {
-		t, err := parseBlameTime(data, "author")
-		if err != nil {
-			c.Author = &Signature{
-				Name:  author,
-				Email: data["author-mail"],
-			}
-		} else {
-			c.Author = &Signature{
-				Name:  author,
-				Email: data["author-mail"],
-				When:  t,
-			}
-		}
-	}
-	committer, ok := data["committer"]
-	if ok && c.Committer == nil {
-		t, err := parseBlameTime(data, "committer")
-		if err != nil {
-			c.Committer = &Signature{
-				Name:  committer,
-				Email: data["committer-mail"],
-			}
-		} else {
-			c.Committer = &Signature{
-				Name:  committer,
-				Email: data["committer-mail"],
-				When:  t,
-			}
-		}
-	}
-}
-
-func parseBlameTime(data map[string]string, prefix string) (time.Time, error) {
-	atoi, err := strconv.ParseInt(data[prefix+"-time"], 10, 64)
-	if err != nil {
-		return time.Time{}, err
-	}
-	t := time.Unix(atoi, 0)
-
-	if len(data["author-tz"]) == 5 {
-		hours, ok1 := strconv.ParseInt(data[prefix+"-tz"][:3], 10, 0)
-		mins, ok2 := strconv.ParseInt(data[prefix+"-tz"][3:5], 10, 0)
-		if ok1 == nil && ok2 == nil {
-			t = t.In(time.FixedZone("Fixed", int((hours*60+mins)*60)))
-		}
+// Line returns the commit by given line number (1-based).
+// It returns nil when no such line.
+func (b *Blame) Line(i int) *Commit {
+	if i <= 0 || len(b.lines) < i {
+		return nil
 	}
-	return t, nil
+	return b.lines[i-1]
 }

repo_blame.go

@@ -0,0 +1,53 @@
+// Copyright 2020 The Gogs 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 git
+
+import (
+	"bytes"
+	"time"
+)
+
+// BlameOptions contains optional arguments for blaming a file.
+// Docs: https://git-scm.com/docs/git-blame
+type BlameOptions struct {
+	// The timeout duration before giving up for each shell command execution.
+	// The default timeout duration will be used when not supplied.
+	Timeout time.Duration
+}
+
+// BlameFile returns blame results of the file with the given revision of the repository.
+func (r *Repository) BlameFile(rev, file string, opts ...BlameOptions) (*Blame, error) {
+	var opt BlameOptions
+	if len(opts) > 0 {
+		opt = opts[0]
+	}
+
+	stdout, err := NewCommand("blame", "-l", "-s", rev, "--", file).RunInDirWithTimeout(opt.Timeout, r.path)
+	if err != nil {
+		return nil, err
+	}
+
+	lines := bytes.Split(stdout, []byte{'\n'})
+	blame := &Blame{
+		lines: make([]*Commit, 0, len(lines)),
+	}
+	for _, line := range lines {
+		if len(line) < 40 {
+			break
+		}
+		id := line[:40]
+
+		// Earliest commit is indicated by a leading "^"
+		if id[0] == '^' {
+			id = id[1:]
+		}
+		commit, err := r.CatFileCommit(string(id), CatFileCommitOptions{Timeout: opt.Timeout}) //nolint
+		if err != nil {
+			return nil, err
+		}
+		blame.lines = append(blame.lines, commit)
+	}
+	return blame, nil
+}

repo_blame_test.go

@@ -0,0 +1,40 @@
+// Copyright 2020 The Gogs 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 git
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRepository_BlameFile(t *testing.T) {
+	t.Run("bad file", func(t *testing.T) {
+		_, err := testrepo.BlameFile("", "404.txt")
+		assert.Error(t, err)
+	})
+
+	blame, err := testrepo.BlameFile("cfc3b2993f74726356887a5ec093de50486dc617", "README.txt")
+	assert.Nil(t, err)
+
+	// Assert representative commits
+	// https://github.com/gogs/git-module-testrepo/blame/master/README.txt
+	tests := []struct {
+		line  int
+		expID string
+	}{
+		{line: 1, expID: "755fd577edcfd9209d0ac072eed3b022cbe4d39b"},
+		{line: 3, expID: "a13dba1e469944772490909daa58c53ac8fa4b0d"},
+		{line: 5, expID: "755fd577edcfd9209d0ac072eed3b022cbe4d39b"},
+		{line: 13, expID: "8d2636da55da593c421e1cb09eea502a05556a69"},
+	}
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("Line %d", test.line), func(t *testing.T) {
+			line := blame.Line(test.line)
+			assert.Equal(t, test.expID, line.ID.String())
+		})
+	}
+}

repo_pull.go

@@ -10,7 +10,7 @@ import (
 )
 
 // MergeBaseOptions contains optional arguments for getting merge base.
-// // Docs: https://git-scm.com/docs/git-merge-base
+// Docs: https://git-scm.com/docs/git-merge-base
 type MergeBaseOptions struct {
 	// The timeout duration before giving up for each shell command execution.
 	// The default timeout duration will be used when not supplied.

utils.go

@@ -10,7 +10,7 @@ import (
 	"sync"
 )
 
-// objectCache provides thread-safe cache opeations.
+// objectCache provides thread-safe cache operations.
 // TODO(@unknwon): Use sync.Map once requires Go 1.13.
 type objectCache struct {
 	lock  sync.RWMutex