address pr comments

Signed-off-by: Arieh Schneier <15041913+AriehSchneier@users.noreply.github.com>

_examples/blame/main.go

@@ -0,0 +1,48 @@
+package main
+
+import (
+	"fmt"
+	"os"
+
+	"github.com/go-git/go-git/v5"
+	. "github.com/go-git/go-git/v5/_examples"
+)
+
+// Basic example of how to blame a repository.
+func main() {
+	CheckArgs("<url>", "<file_to_blame>")
+	url := os.Args[1]
+	path := os.Args[2]
+
+	tmp, err := os.MkdirTemp("", "go-git-blame-*")
+	CheckIfError(err)
+
+	defer os.RemoveAll(tmp)
+
+	// Clone the given repository.
+	Info("git clone %s %s", url, tmp)
+	r, err := git.PlainClone(
+		tmp,
+		false,
+		&git.CloneOptions{
+			URL:  url,
+			Tags: git.NoTags,
+		},
+	)
+	CheckIfError(err)
+
+	// Retrieve the branch's HEAD, to then get the HEAD commit.
+	ref, err := r.Head()
+	CheckIfError(err)
+
+	c, err := r.CommitObject(ref.Hash())
+	CheckIfError(err)
+
+	Info("git blame %s", path)
+
+	// Blame the given file/path.
+	br, err := git.Blame(c, path)
+	CheckIfError(err)
+
+	fmt.Printf("%s", br.String())
+}

blame.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"strconv"
-	"strings"
 	"time"
 	"unicode/utf8"
 
@@ -80,11 +79,12 @@ func Blame(c *object.Commit, path string) (*BlameResult, error) {
 		false,
 		0,
 	})
+	items := make([]*queueItem, 0)
 	for {
-		items := make([]*queueItem, 0)
+		items = items[:0]
 		for {
 			if b.q.Len() == 0 {
-				return nil, errors.New("No items left on the blame queue? This should be impossible")
+				return nil, errors.New("invalid state: no items left on the blame queue")
 			}
 			item := b.q.Pop()
 			items = append(items, item)
@@ -126,6 +126,8 @@ func Blame(c *object.Commit, path string) (*BlameResult, error) {
 type Line struct {
 	// Author is the email address of the last author that modified the line.
 	Author string
+	// AuthorName is the name of the last author that modified the line.
+	AuthorName string
 	// Text is the original text of the line.
 	Text string
 	// Date is when the original text of the line was introduced
@@ -134,20 +136,21 @@ type Line struct {
 	Hash plumbing.Hash
 }
 
-func newLine(author, text string, date time.Time, hash plumbing.Hash) *Line {
+func newLine(author, authorName, text string, date time.Time, hash plumbing.Hash) *Line {
 	return &Line{
-		Author: author,
-		Text:   text,
-		Hash:   hash,
-		Date:   date,
+		Author:     author,
+		AuthorName: authorName,
+		Text:       text,
+		Hash:       hash,
+		Date:       date,
 	}
 }
 
 func newLines(contents []string, commits []*object.Commit) ([]*Line, error) {
 	result := make([]*Line, 0, len(contents))
 	for i := range contents {
 		result = append(result, newLine(
-			commits[i].Author.Email, contents[i],
+			commits[i].Author.Email, commits[i].Author.Name, contents[i],
 			commits[i].Author.When, commits[i].Hash,
 		))
 	}
@@ -178,7 +181,7 @@ func (b *blame) addBlames(curItems []*queueItem) (bool, error) {
 	curItem := curItems[0]
 
 	// Simple optimisation to merge paths, there is potential to go a bit further here and check for any duplicates
-	// not only if they are all the same
+	// not only if they are all the same.
 	if len(curItems) == 1 {
 		curItems = nil
 	} else if curItem.IdenticalToChild {
@@ -332,7 +335,7 @@ func (b *blame) addBlames(curItems []*queueItem) (bool, error) {
 					prevl += hLines
 					continue out
 				default:
-					panic("unreachable")
+					return false, errors.New("invalid state: invalid hunk Type")
 				}
 			}
 		}
@@ -445,52 +448,29 @@ func applyNeeds(child *queueItem, needsMap []lineMap, identicalToChild bool, par
 	return false, nil
 }
 
-// GoString prints the results of a Blame using git-blame's style.
-func (b *blame) GoString() string {
+// String prints the results of a Blame using git-blame's style.
+func (b BlameResult) String() string {
 	var buf bytes.Buffer
 
-	file, err := b.fRev.File(b.path)
-	if err != nil {
-		panic("PrettyPrint: internal error in repo.Data")
-	}
-	contents, err := file.Contents()
-	if err != nil {
-		panic("PrettyPrint: internal error in repo.Data")
-	}
-
-	lines := strings.Split(contents, "\n")
 	// max line number length
-	mlnl := len(strconv.Itoa(len(lines)))
+	mlnl := len(strconv.Itoa(len(b.Lines)))
 	// max author length
 	mal := b.maxAuthorLength()
-	format := fmt.Sprintf("%%s (%%-%ds %%%dd) %%s\n",
-		mal, mlnl)
+	format := fmt.Sprintf("%%s (%%-%ds %%s %%%dd) %%s\n", mal, mlnl)
 
-	fVs := b.lineToCommit
-	for ln, v := range fVs {
-		_, _ = fmt.Fprintf(&buf, format, v.Hash.String()[:8],
-			prettyPrintAuthor(fVs[ln]), ln+1, lines[ln])
+	for ln := range b.Lines {
+		_, _ = fmt.Fprintf(&buf, format, b.Lines[ln].Hash.String()[:8],
+			b.Lines[ln].AuthorName, b.Lines[ln].Date.Format("2006-01-02 15:04:05 -0700"), ln+1, b.Lines[ln].Text)
 	}
 	return buf.String()
 }
 
-// utility function to pretty print the author.
-func prettyPrintAuthor(c *object.Commit) string {
-	return fmt.Sprintf("%s %s", c.Author.Name, c.Author.When.Format("2006-01-02"))
-}
-
 // utility function to calculate the number of runes needed
 // to print the longest author name in the blame of a file.
-func (b *blame) maxAuthorLength() int {
-	fVs := b.lineToCommit
-	memo := make(map[plumbing.Hash]struct{}, len(fVs))
+func (b BlameResult) maxAuthorLength() int {
 	m := 0
-	for ln := range fVs {
-		if _, ok := memo[fVs[ln].Hash]; ok {
-			continue
-		}
-		memo[fVs[ln].Hash] = struct{}{}
-		m = max(m, utf8.RuneCountInString(prettyPrintAuthor(fVs[ln])))
+	for ln := range b.Lines {
+		m = max(m, utf8.RuneCountInString(b.Lines[ln].AuthorName))
 	}
 	return m
 }
@@ -501,6 +481,7 @@ func min(a, b int) int {
 	}
 	return b
 }
+
 func max(a, b int) int {
 	if a > b {
 		return a
@@ -526,6 +507,7 @@ type queueItem struct {
 	IdenticalToChild        bool
 	ParentNo                int
 }
+
 type priorityQueueImp []*queueItem
 
 func (pq *priorityQueueImp) Len() int { return len(*pq) }

blame_test.go

@@ -81,10 +81,11 @@ func (s *BlameSuite) mockBlame(c *C, t blameTest, r *Repository) (blame *BlameRe
 		commit, err := r.CommitObject(plumbing.NewHash(t.blames[i]))
 		c.Assert(err, IsNil)
 		l := &Line{
-			Author: commit.Author.Email,
-			Text:   lines[i],
-			Date:   commit.Author.When,
-			Hash:   commit.Hash,
+			Author:     commit.Author.Email,
+			AuthorName: commit.Author.Name,
+			Text:       lines[i],
+			Date:       commit.Author.When,
+			Hash:       commit.Hash,
 		}
 		blamedLines = append(blamedLines, l)
 	}
@@ -146,11 +147,11 @@ var blameTests = [...]blameTest{
 		repeat("6ecf0ef2c2dffb796033e5a02219af86ec6584e5", 7),
 	)},
 	/*
-		// This fails due to the different diff tool being used to create the patches
+		// This fails due to the different diff tool being used to create the patches.
 		// For example in commit d4b48a39aba7d3bd3e8abef2274a95b112d1ae73 when "function echo_status()" is added:
 		// - 'git diff' adds the new "}\n\n" to the end of function and keeps the "}\n\n" beforehand blamed to the previous commit
 		// - our diff adds the new "}\n\n" before the function and reuses the existing "}\n\n" to close the new function
-		// the resultant file is the same, but it causes blame not to match
+		// the resultant file is the same, but it causes blame not to match.
 		{"https://github.com/spinnaker/spinnaker.git", "f39d86f59a0781f130e8de6b2115329c1fbe9545", "InstallSpinnaker.sh", concat(
 			repeat("ce9f123d790717599aaeb76bc62510de437761be", 2),
 			repeat("a47d0aaeda421f06df248ad65bd58230766bf118", 1),
@@ -345,9 +346,9 @@ var blameTests = [...]blameTest{
 		repeat("a24001f6938d425d0e7504bdf5d27fc866a85c3d", 185),
 	)},
 	/*
-		// This fails due to the different diff tool being used to create the patches
+		// This fails due to the different diff tool being used to create the patches.
 		// For commit c89dab0d42f1856d157357e9010f8cc6a12f5b1f our diff tool keeps an existing newline as moved in the file, whereas
-		// 'git diff' says the existing newline was deleted and a new one created
+		// 'git diff' says the existing newline was deleted and a new one created.
 		{"https://github.com/spinnaker/spinnaker.git", "f39d86f59a0781f130e8de6b2115329c1fbe9545", "pylib/spinnaker/configurator.py", concat(
 			repeat("a24001f6938d425d0e7504bdf5d27fc866a85c3d", 53),
 			repeat("c89dab0d42f1856d157357e9010f8cc6a12f5b1f", 1),

plumbing/object/commit.go

@@ -376,15 +376,15 @@ func (c *Commit) Verify(armoredKeyRing string) (*openpgp.Entity, error) {
 	return openpgp.CheckArmoredDetachedSignature(keyring, er, signature, nil)
 }
 
-// Less defines a compare function to determine which commit is 'earlier'
+// Less defines a compare function to determine which commit is 'earlier' by:
 // - First use Committer.When
 // - If Committer.When are equal then use Author.When
 // - If Author.When also equal then compare the string value of the hash
 func (c *Commit) Less(rhs *Commit) bool {
 	return c.Committer.When.Before(rhs.Committer.When) ||
 		(c.Committer.When.Equal(rhs.Committer.When) &&
 			(c.Author.When.Before(rhs.Author.When) ||
-				(c.Author.When.Equal(rhs.Author.When) && c.Hash.String() < rhs.Hash.String())))
+				(c.Author.When.Equal(rhs.Author.When) && bytes.Compare(c.Hash[:], rhs.Hash[:]) < 0)))
 }
 
 func indent(t string) string {