git-codereview: add reword command

Quoting the new docs:

The reword command edits pending commit messages.

    git codereview reword [commit...]

Reword opens the editor on the commit message for each named commit in turn.
When all the editing is finished, it applies the changes to the pending
commits. If no commit is listed, reword applies to the most recent pending
commit.

Reword is similar in effect to running “git codereview rebase-work” and
changing the script action for the named commits to “reword”, or (with no
arguments) to “git commit --amend”, but it only affects the commit messages,
not the state of the git staged index, nor any checked-out files. This more
careful implementation makes it safe to use when there are local changes or,
for example, when tests are running that would be broken by temporary
changes to the checked-out tree, as would happen during “git codereview
rebase-work”.

Change-Id: I38ac939b8530bf237c6cafb911f2b17d22eaca60
Reviewed-on: https://go-review.googlesource.com/c/review/+/279718
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index 787a80d..5d277c8 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -33,13 +33,18 @@
 
 // A Commit describes a single pending commit on a Git branch.
 type Commit struct {
-	Hash      string // commit hash
-	ShortHash string // abbreviated commit hash
-	Parent    string // parent hash
-	Merge     string // for merges, hash of commit being merged into Parent
-	Message   string // commit message
-	Subject   string // first line of commit message
-	ChangeID  string // Change-Id in commit message ("" if missing)
+	Hash      string   // commit hash
+	ShortHash string   // abbreviated commit hash
+	Parent    string   // parent hash (== Parents[0])
+	Parents   []string // all parent hashes (merges have > 1)
+	Tree      string   // tree hash
+	Message   string   // commit message
+	Subject   string   // first line of commit message
+	ChangeID  string   // Change-Id in commit message ("" if missing)
+
+	AuthorName  string // author name (from %an)
+	AuthorEmail string // author email (from %ae)
+	AuthorDate  string // author date as Unix timestamp string (from %at)
 
 	// For use by pending command.
 	g         *GerritChange // associated Gerrit change data
@@ -47,6 +52,16 @@
 	committed []string      // list of files in this commit
 }
 
+// HasParent reports whether hash appears in c.Parents.
+func (c *Commit) HasParent(hash string) bool {
+	for _, p := range c.Parents {
+		if p == hash {
+			return true
+		}
+	}
+	return false
+}
+
 // Config returns the configuration for the branch.
 func (b *Branch) Config() map[string]string {
 	if b.config != nil {
@@ -227,8 +242,10 @@
 
 	// Note: --topo-order means child first, then parent.
 	origin := b.OriginBranch()
-	const numField = 5
-	all := trim(cmdOutput("git", "log", "--topo-order", "--format=format:%H%x00%h%x00%P%x00%B%x00%s%x00", origin+".."+b.FullName(), "--"))
+	const numField = 9
+	all := trim(cmdOutput("git", "log", "--topo-order",
+		"--format=format:%H%x00%h%x00%P%x00%T%x00%B%x00%s%x00%an%x00%ae%x00%at%x00",
+		origin+".."+b.FullName(), "--"))
 	fields := strings.Split(all, "\x00")
 	if len(fields) < numField {
 		return // nothing pending
@@ -236,16 +253,23 @@
 	for i, field := range fields {
 		fields[i] = strings.TrimLeft(field, "\r\n")
 	}
+Log:
 	for i := 0; i+numField <= len(fields); i += numField {
+		parents := strings.Fields(fields[i+2])
 		c := &Commit{
-			Hash:      fields[i],
-			ShortHash: fields[i+1],
-			Parent:    strings.TrimSpace(fields[i+2]), // %P starts with \n for some reason
-			Message:   fields[i+3],
-			Subject:   fields[i+4],
+			Hash:        fields[i],
+			ShortHash:   fields[i+1],
+			Parent:      parents[0],
+			Parents:     parents,
+			Tree:        fields[i+3],
+			Message:     fields[i+4],
+			Subject:     fields[i+5],
+			AuthorName:  fields[i+6],
+			AuthorEmail: fields[i+7],
+			AuthorDate:  fields[i+8],
 		}
-		if j := strings.Index(c.Parent, " "); j >= 0 {
-			c.Parent, c.Merge = c.Parent[:j], c.Parent[j+1:]
+
+		if len(c.Parents) > 1 {
 			// Found merge point.
 			// Merges break the invariant that the last shared commit (the branchpoint)
 			// is the parent of the final commit in the log output.
@@ -254,15 +278,12 @@
 			// even if we later see additional commits on a different branch leading down to
 			// a lower location on the same origin branch.
 			// Check c.Merge (the second parent) too, so we don't depend on the parent order.
-			if strings.Contains(cmdOutput("git", "branch", "-a", "--contains", c.Parent), " remotes/"+origin+"\n") {
-				b.pending = append(b.pending, c)
-				b.branchpoint = c.Parent
-				break
-			}
-			if strings.Contains(cmdOutput("git", "branch", "-a", "--contains", c.Merge), " remotes/"+origin+"\n") {
-				b.pending = append(b.pending, c)
-				b.branchpoint = c.Merge
-				break
+			for _, parent := range c.Parents {
+				if strings.Contains(cmdOutput("git", "branch", "-a", "--contains", parent), " remotes/"+origin+"\n") {
+					b.pending = append(b.pending, c)
+					b.branchpoint = parent
+					break Log
+				}
 			}
 		}
 		for _, line := range lines(c.Message) {
diff --git a/git-codereview/doc.go b/git-codereview/doc.go
index b9eb230..62c42ea 100644
--- a/git-codereview/doc.go
+++ b/git-codereview/doc.go
@@ -48,8 +48,10 @@
 		mail = codereview mail
 		pending = codereview pending
 		rebase-work = codereview rebase-work
+		reword = codereview reword
 		submit = codereview submit
 		sync = codereview sync
+		sync-branch = codereview sync-branch
 
 Single-Commit Work Branches
 
@@ -282,7 +284,10 @@
 Rebase-work
 
 The rebase-work command runs git rebase in interactive mode over pending changes.
-It is shorthand for “git rebase -i $(git codereview branchpoint)”.
+
+	git codereview rebase-work
+
+The command is shorthand for “git rebase -i $(git codereview branchpoint)”.
 It differs from plain “git rebase -i” in that the latter will try to incorporate
 new commits from the origin branch during the rebase;
 “git codereview rebase-work” does not.
@@ -290,6 +295,28 @@
 In multiple-commit workflows, rebase-work is used so often that it can be helpful
 to alias it to “git rw”.
 
+Reword
+
+The reword command edits pending commit messages.
+
+	git codereview reword [commit...]
+
+Reword opens the editor on the commit messages for the named comments.
+When the editing is finished, it applies the changes to the pending commits.
+If no commit is listed, reword applies to all pending commits.
+
+Reword is similar in effect to running “git codereview rebase-work” and changing
+the script action for the named commits to “reword”, or (with no arguments)
+to “git commit --amend”, but it only affects the commit messages, not the state
+of the git staged index, nor any checked-out files. This more careful implementation
+makes it safe to use when there are local changes or, for example, when tests are
+running that would be broken by temporary changes to the checked-out tree,
+as would happen during “git codereview rebase-work”.
+
+Reword is most useful for editing commit messages on a multiple-commit work
+branch, but it can also be useful in single-commit work branches to allow
+editing a commit message without committing staged changes at the same time.
+
 Submit
 
 The submit command pushes the pending change to the Gerrit server and tells
diff --git a/git-codereview/mail.go b/git-codereview/mail.go
index 3b60eb3..b7fc9f9 100644
--- a/git-codereview/mail.go
+++ b/git-codereview/mail.go
@@ -59,7 +59,7 @@
 		return
 	}
 
-	if len(ListFiles(c)) == 0 && c.Merge == "" {
+	if len(ListFiles(c)) == 0 && len(c.Parents) == 1 {
 		dief("cannot mail: commit %s is empty", c.ShortHash)
 	}
 
diff --git a/git-codereview/pending.go b/git-codereview/pending.go
index f3e3369..0f61208 100644
--- a/git-codereview/pending.go
+++ b/git-codereview/pending.go
@@ -321,8 +321,12 @@
 	case "ABANDONED":
 		tags = append(tags, "abandoned")
 	}
-	if c.Merge != "" {
-		tags = append(tags, "merge="+c.Merge[:7])
+	if len(c.Parents) > 1 {
+		var h []string
+		for _, p := range c.Parents[1:] {
+			h = append(h, p[:7])
+		}
+		tags = append(tags, "merge="+strings.Join(h, ","))
 	}
 	if len(tags) > 0 {
 		fmt.Fprintf(w, " (%s)", strings.Join(tags, ", "))
diff --git a/git-codereview/review.go b/git-codereview/review.go
index 5c7f16d..6dcb6a0 100644
--- a/git-codereview/review.go
+++ b/git-codereview/review.go
@@ -33,6 +33,7 @@
 	flags = flag.NewFlagSet("", flag.ExitOnError)
 	flags.Usage = func() {
 		fmt.Fprintf(stderr(), usage, progName, progName)
+		exit(2)
 	}
 	flags.SetOutput(stderr())
 	flags.BoolVar(noRun, "n", false, "print but do not run commands")
@@ -65,6 +66,7 @@
 	mail [-r reviewer,...] [-cc mail,...] [options] [commit]
 	pending [-c] [-l] [-s]
 	rebase-work
+	reword [commit...]
 	submit [-i | commit...]
 	sync
 	sync-branch [-continue]
@@ -109,6 +111,8 @@
 		cmd = cmdPending
 	case "rebase-work":
 		cmd = cmdRebaseWork
+	case "reword":
+		cmd = cmdReword
 	case "submit":
 		cmd = cmdSubmit
 	case "sync":
diff --git a/git-codereview/reword.go b/git-codereview/reword.go
new file mode 100644
index 0000000..b2ad0de
--- /dev/null
+++ b/git-codereview/reword.go
@@ -0,0 +1,234 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+func cmdReword(args []string) {
+	flags.Usage = func() {
+		fmt.Fprintf(stderr(), "Usage: %s reword %s [commit...]\n",
+			progName, globalFlags)
+		exit(2)
+	}
+	flags.Parse(args)
+	args = flags.Args()
+
+	// Check that we understand the structure
+	// before we let the user spend time editing messages.
+	b := CurrentBranch()
+	pending := b.Pending()
+	if len(pending) == 0 {
+		dief("reword: no commits pending")
+	}
+	if b.Name == "HEAD" {
+		dief("reword: no current branch")
+	}
+	var last *Commit
+	for i := len(pending) - 1; i >= 0; i-- {
+		c := pending[i]
+		if last != nil && !c.HasParent(last.Hash) {
+			dief("internal error: confused about pending commit graph: parent %.7s vs %.7s", last.Hash, c.Parents)
+		}
+		last = c
+	}
+
+	headState := func() (head, branch string) {
+		head = trim(cmdOutput("git", "rev-parse", "HEAD"))
+		for _, line := range nonBlankLines(cmdOutput("git", "branch", "-l")) {
+			if strings.HasPrefix(line, "* ") {
+				branch = trim(line[1:])
+				return head, branch
+			}
+		}
+		dief("internal error: cannot find current branch")
+		panic("unreachable")
+	}
+
+	head, branch := headState()
+	if head != last.Hash {
+		dief("internal error: confused about pending commit graph: HEAD vs parent: %.7s vs %.7s", head, last.Hash)
+	}
+	if branch != b.Name {
+		dief("internal error: confused about pending commit graph: branch name %s vs %s", branch, b.Name)
+	}
+
+	// Build list of commits to be reworded.
+	// Do first, in case there are typos on the command line.
+	var cs []*Commit
+	newMsg := make(map[*Commit]string)
+	if len(args) == 0 {
+		for _, c := range pending {
+			cs = append(cs, c)
+		}
+	} else {
+		for _, arg := range args {
+			c := b.CommitByRev("reword", arg)
+			cs = append(cs, c)
+		}
+	}
+	for _, c := range cs {
+		newMsg[c] = ""
+	}
+
+	// Invoke editor to reword all the messages message.
+	// Save the edits to REWORD_MSGS immediately after editor exit
+	// in case we for some reason cannot apply the changes - don't want
+	// to throw away the user's writing.
+	// But we don't use REWORD_MSGS as the actual editor file,
+	// because if there are multiple git rewords happening
+	// (perhaps the user has forgotten about one in another window),
+	// we don't want them to step on each other during editing.
+	var buf bytes.Buffer
+	saveFile := filepath.Join(repoRoot(), ".git/REWORD_MSGS")
+	saveBuf := func() {
+		if err := ioutil.WriteFile(saveFile, buf.Bytes(), 0666); err != nil {
+			dief("cannot save messages: %v", err)
+		}
+	}
+	saveBuf() // make sure it works before we let the user edit anything
+	printf("editing messages (new texts logged in %s in case of failure)", saveFile)
+	note := "edited messages saved in " + saveFile
+
+	if len(cs) == 1 {
+		c := cs[0]
+		edited := editor(c.Message)
+		if edited == "" {
+			dief("edited message is empty")
+		}
+		newMsg[c] = edited
+		fmt.Fprintf(&buf, "# %s\n\n%s\n\n", c.Subject, edited)
+		saveBuf()
+	} else {
+		// Edit all at once.
+		var ed bytes.Buffer
+		ed.WriteString(rewordProlog)
+		byHash := make(map[string]*Commit)
+		for _, c := range cs {
+			if strings.HasPrefix(c.Message, "# ") || strings.Contains(c.Message, "\n# ") {
+				// Will break our framing.
+				// Should be pretty rare since 'git commit' and 'git commit --amend'
+				// delete lines beginning with # after editing sessions.
+				dief("commit %.7s has a message line beginning with # - cannot reword with other commits", c.Hash)
+			}
+			hash := c.Hash[:7]
+			byHash[hash] = c
+			// Two blank lines before #, one after.
+			// Lots of space to make it easier to see the boundaries
+			// between commit messages.
+			fmt.Fprintf(&ed, "\n\n# %s %s\n\n%s\n", hash, c.Subject, c.Message)
+		}
+		edited := editor(ed.String())
+		if edited == "" {
+			dief("edited text is empty")
+		}
+
+		// Save buffer for user before going further.
+		buf.WriteString(edited)
+		saveBuf()
+
+		for i, text := range strings.Split("\n"+edited, "\n# ") {
+			if i == 0 {
+				continue
+			}
+			text = "# " + text // restore split separator
+
+			// Pull out # hash header line and body.
+			hdr, body, _ := cut(text, "\n")
+
+			// Cut blank lines at start and end of body but keep newline-terminated.
+			for body != "" {
+				line, rest, _ := cut(body, "\n")
+				if line != "" {
+					break
+				}
+				body = rest
+			}
+			body = strings.TrimRight(body, " \t\n")
+			if body != "" {
+				body += "\n"
+			}
+
+			// Look up hash.
+			f := strings.Fields(hdr)
+			if len(f) < 2 {
+				dief("edited text has # line with no commit hash\n%s", note)
+			}
+			c := byHash[f[1]]
+			if c == nil {
+				dief("cannot find commit for header: %s\n%s", strings.TrimSpace(hdr), note)
+			}
+			newMsg[c] = body
+		}
+	}
+
+	// Rebuild the commits the way git would,
+	// but without doing any git checkout that
+	// would affect the files in the working directory.
+	var newHash string
+	last = nil
+	for i := len(pending) - 1; i >= 0; i-- {
+		c := pending[i]
+		if (newMsg[c] == "" || newMsg[c] == c.Message) && newHash == "" {
+			// Have not started making changes yet. Leave exactly as is.
+			last = c
+			continue
+		}
+		// Rebuilding.
+		msg := newMsg[c]
+		if msg == "" {
+			msg = c.Message
+		}
+		if last != nil && newHash != "" && !c.HasParent(last.Hash) {
+			dief("internal error: confused about pending commit graph")
+		}
+		gitArgs := []string{"commit-tree", "-p"}
+		for _, p := range c.Parents {
+			if last != nil && newHash != "" && p == last.Hash {
+				p = newHash
+			}
+			gitArgs = append(gitArgs, p)
+		}
+		gitArgs = append(gitArgs, "-m", msg, c.Tree)
+		os.Setenv("GIT_AUTHOR_NAME", c.AuthorName)
+		os.Setenv("GIT_AUTHOR_EMAIL", c.AuthorEmail)
+		os.Setenv("GIT_AUTHOR_DATE", c.AuthorDate)
+		newHash = trim(cmdOutput("git", gitArgs...))
+		last = c
+	}
+	if newHash == "" {
+		// No messages changed.
+		return
+	}
+
+	// Attempt swap of HEAD but leave index and working copy alone.
+	// No obvious way to make it atomic, but check for races.
+	head, branch = headState()
+	if head != pending[0].Hash {
+		dief("cannot reword: commits changed underfoot\n%s", note)
+	}
+	if branch != b.Name {
+		dief("cannot reword: branch changed underfoot\n%s", note)
+	}
+	run("git", "reset", "--soft", newHash)
+}
+
+func cut(s, sep string) (before, after string, ok bool) {
+	i := strings.Index(s, sep)
+	if i < 0 {
+		return s, "", false
+	}
+	return s[:i], s[i+len(sep):], true
+}
+
+var rewordProlog = `Rewording multiple commit messages.
+The # lines separate the different commits and must be left unchanged.
+`
diff --git a/git-codereview/reword_test.go b/git-codereview/reword_test.go
new file mode 100644
index 0000000..c370b37
--- /dev/null
+++ b/git-codereview/reword_test.go
@@ -0,0 +1,68 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"os"
+	"strings"
+	"testing"
+)
+
+func TestReword(t *testing.T) {
+	gt := newGitTest(t)
+	defer gt.done()
+
+	gt.work(t)
+	gt.work(t)
+	gt.work(t)
+	trun(t, gt.client, "git", "tag", "MSG3")
+	gt.work(t)
+	trun(t, gt.client, "git", "tag", "MSG4")
+	const fakeName = "Grace R. Emlin"
+	os.Setenv("GIT_AUTHOR_NAME", fakeName)
+	gt.work(t)
+	os.Unsetenv("GIT_AUTHOR_NAME")
+
+	write(t, gt.client+"/file", "pending work", 0644) // would make git checkout unhappy
+
+	testMainDied(t, "rebase-work")
+	testNoStdout(t)
+	testPrintedStderr(t, "cannot rebase with uncommitted work")
+
+	os.Setenv("GIT_EDITOR", "sed -i.bak -e s/msg/MESSAGE/")
+
+	testMain(t, "reword", "MSG3", "MSG4")
+	testNoStdout(t)
+	testPrintedStderr(t, "editing messages (new texts logged in",
+		".git/REWORD_MSGS in case of failure)")
+
+	testMain(t, "pending", "-c", "-l", "-s")
+	testNoStderr(t)
+	testPrintedStdout(t,
+		"msg #2",
+		"MESSAGE #3",
+		"MESSAGE #4",
+		"msg #5",
+	)
+
+	testMain(t, "reword") // reword all
+	testNoStdout(t)
+	testPrintedStderr(t, "editing messages (new texts logged in",
+		".git/REWORD_MSGS in case of failure)")
+
+	testMain(t, "pending", "-c", "-l", "-s")
+	testNoStderr(t)
+	testPrintedStdout(t,
+		"MESSAGE #2",
+		"MESSAGE #3",
+		"MESSAGE #4",
+		"MESSAGE #5",
+	)
+
+	out := trun(t, gt.client, "git", "log", "-n1")
+	if !strings.Contains(out, fakeName) {
+		t.Fatalf("reword lost author name (%s): %v\n", fakeName, out)
+	}
+}
diff --git a/git-codereview/submit.go b/git-codereview/submit.go
index 5f684ca..fc02713 100644
--- a/git-codereview/submit.go
+++ b/git-codereview/submit.go
@@ -17,6 +17,7 @@
 	flags.BoolVar(&interactive, "i", false, "interactively select commits to submit")
 	flags.Usage = func() {
 		fmt.Fprintf(stderr(), "Usage: %s submit %s [-i | commit...]\n", progName, globalFlags)
+		exit(2)
 	}
 	flags.Parse(args)
 	if interactive && flags.NArg() > 0 {