git-codereview: accept any commit spelling for mail and submit
Currently the mail and submit subcommands only accept commit hashes.
This makes them inconsistent with other git commands (git is hard
enough without having to remember another convention) and makes
working with multi-commit branches cumbersome.
Change mail and submit to accept any standard commit spelling. In
particular, this makes it possible to use HEAD-relative names: "git
mail HEAD" sends the whole current branch. We still require that the
named commit be pending on the current branch to avoid accidentally
naming a completely unrelated commit.
Change-Id: If84d315e0fc32118a5283adfbca7a3b56f3c098c
Reviewed-on: https://go-review.googlesource.com/19271
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index 1f08337..5ba9d35 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -307,22 +307,24 @@
return readGerritChange(id)
}
-const minHashLen = 4 // git minimum hash length accepted on command line
-
-// CommitByHash finds a unique pending commit by its hash prefix.
-// It dies if the hash cannot be resolved to a pending commit,
-// using the action ("mail", "submit") in the failure message.
-func (b *Branch) CommitByHash(action, hash string) *Commit {
- if len(hash) < minHashLen {
- dief("cannot %s: commit hash %q must be at least %d digits long", action, hash, minHashLen)
+// CommitByRev finds a unique pending commit by its git <rev>.
+// It dies if rev cannot be resolved to a commit or that commit is not
+// pending on b using the action ("mail", "submit") in the failure message.
+func (b *Branch) CommitByRev(action, rev string) *Commit {
+ // Parse rev to a commit hash.
+ hash, err := cmdOutputErr("git", "rev-parse", "--verify", rev+"^{commit}")
+ if err != nil {
+ msg := strings.TrimPrefix(trim(err.Error()), "fatal: ")
+ dief("cannot %s: %s", action, msg)
}
+ hash = trim(hash)
+
+ // Check that hash is a pending commit.
var c *Commit
for _, c1 := range b.Pending() {
- if strings.HasPrefix(c1.Hash, hash) {
- if c != nil {
- dief("cannot %s: commit hash %q is ambiguous in the current branch", action, hash)
- }
+ if c1.Hash == hash {
c = c1
+ break
}
}
if c == nil {
@@ -348,7 +350,7 @@
if action == "submit" {
extra = " or use submit -i"
}
- dief("cannot %s: multiple changes pending; must specify commit hash on command line%s:%s", action, extra, buf.String())
+ dief("cannot %s: multiple changes pending; must specify commit on command line%s:%s", action, extra, buf.String())
}
return work[0]
}
diff --git a/git-codereview/mail.go b/git-codereview/mail.go
index 0354836..c83a5d8 100644
--- a/git-codereview/mail.go
+++ b/git-codereview/mail.go
@@ -25,7 +25,7 @@
flags.Var(ccList, "cc", "comma-separated list of people to CC:")
flags.Usage = func() {
- fmt.Fprintf(stderr(), "Usage: %s mail %s [-r reviewer,...] [-cc mail,...] [-topic topic] [-trybot] [commit-hash]\n", os.Args[0], globalFlags)
+ fmt.Fprintf(stderr(), "Usage: %s mail %s [-r reviewer,...] [-cc mail,...] [-topic topic] [-trybot] [commit]\n", os.Args[0], globalFlags)
}
flags.Parse(args)
if len(flags.Args()) > 1 {
@@ -37,7 +37,7 @@
var c *Commit
if len(flags.Args()) == 1 {
- c = b.CommitByHash("mail", flags.Arg(0))
+ c = b.CommitByRev("mail", flags.Arg(0))
} else {
c = b.DefaultCommit("mail")
}
diff --git a/git-codereview/mail_test.go b/git-codereview/mail_test.go
index e615b76..fa006db 100644
--- a/git-codereview/mail_test.go
+++ b/git-codereview/mail_test.go
@@ -54,6 +54,35 @@
testMain(t, "mail", "-diff")
}
+func TestMailMultiple(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ srv := newGerritServer(t)
+ defer srv.done()
+
+ gt.work(t)
+ gt.work(t)
+ gt.work(t)
+
+ testMainDied(t, "mail")
+ testPrintedStderr(t, "cannot mail: multiple changes pending")
+
+ // Mail first two and test non-HEAD mail.
+ h := CurrentBranch().Pending()[1].ShortHash
+ testMain(t, "mail", "HEAD^")
+ testRan(t,
+ "git push -q origin "+h+":refs/for/master",
+ "git tag -f work.mailed "+h)
+
+ // Mail HEAD.
+ h = CurrentBranch().Pending()[0].ShortHash
+ testMain(t, "mail", "HEAD")
+ testRan(t,
+ "git push -q origin HEAD:refs/for/master",
+ "git tag -f work.mailed "+h)
+}
+
var reviewerLog = []string{
"Fake 1 <r1@fake.com>",
"Fake 1 <r1@fake.com>",
diff --git a/git-codereview/review.go b/git-codereview/review.go
index f4db94a..15766ef 100644
--- a/git-codereview/review.go
+++ b/git-codereview/review.go
@@ -77,9 +77,11 @@
Every other operation except help also does this,
if they are not already installed.
- mail [-f] [-r reviewer,...] [-cc mail,...]
+ mail [-f] [-r reviewer,...] [-cc mail,...] [commit]
Upload change commit to the code review server and send mail
requesting a code review.
+ If there are multiple commits on this branch, upload commits
+ up to and including the named commit.
If -f is specified, upload even if there are staged changes.
The -r and -cc flags identify the email addresses of people to
do the code review and to be CC'ed about the code review.
@@ -95,7 +97,7 @@
If -l is specified, only use locally available information.
If -s is specified, show short output.
- submit [-i | commit-hash...]
+ submit [-i | commit...]
Push the pending change to the Gerrit server and tell Gerrit to
submit it to the master branch.
diff --git a/git-codereview/submit.go b/git-codereview/submit.go
index 809bbe7..d163914 100644
--- a/git-codereview/submit.go
+++ b/git-codereview/submit.go
@@ -18,7 +18,7 @@
var interactive bool
flags.BoolVar(&interactive, "i", false, "interactively select commits to submit")
flags.Usage = func() {
- fmt.Fprintf(stderr(), "Usage: %s submit %s [-i | commit-hash...]\n", os.Args[0], globalFlags)
+ fmt.Fprintf(stderr(), "Usage: %s submit %s [-i | commit...]\n", os.Args[0], globalFlags)
}
flags.Parse(args)
if interactive && flags.NArg() > 0 {
@@ -35,11 +35,11 @@
return
}
for _, hash := range hashes {
- cs = append(cs, b.CommitByHash("submit", hash))
+ cs = append(cs, b.CommitByRev("submit", hash))
}
} else if args := flags.Args(); len(args) >= 1 {
for _, arg := range args {
- cs = append(cs, b.CommitByHash("submit", arg))
+ cs = append(cs, b.CommitByRev("submit", arg))
}
} else {
cs = append(cs, b.DefaultCommit("submit"))
diff --git a/git-codereview/submit_test.go b/git-codereview/submit_test.go
index ed6ddc2..29111fc 100644
--- a/git-codereview/submit_test.go
+++ b/git-codereview/submit_test.go
@@ -177,6 +177,17 @@
testMain(t, "submit", cl1.CurrentRevision, cl2.CurrentRevision)
}
+func TestSubmitMultipleNamed(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ srv := newGerritServer(t)
+ defer srv.done()
+
+ _, _ = testSubmitMultiple(t, gt, srv)
+ testMain(t, "submit", "HEAD^", "HEAD")
+}
+
func TestSubmitInteractive(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("see golang.org/issue/13406")
