aboutsummaryrefslogtreecommitdiff
path: root/git-codereview
diff options
context:
space:
mode:
Diffstat (limited to 'git-codereview')
-rw-r--r--git-codereview/branch.go13
-rw-r--r--git-codereview/gofmt.go2
-rw-r--r--git-codereview/pending.go191
-rw-r--r--git-codereview/pending_test.go129
-rw-r--r--git-codereview/review.go4
-rw-r--r--git-codereview/util_test.go1
6 files changed, 221 insertions, 119 deletions
diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index 1193da9..48ba8bb 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -41,7 +41,7 @@ type Commit struct {
// CurrentBranch returns the current branch.
func CurrentBranch() *Branch {
- name := trim(cmdOutput("git", "rev-parse", "--abbrev-ref", "HEAD"))
+ name := strings.TrimPrefix(trim(cmdOutput("git", "rev-parse", "--abbrev-ref", "HEAD")), "heads/")
return &Branch{Name: name}
}
@@ -82,6 +82,13 @@ func (b *Branch) OriginBranch() string {
panic("not reached")
}
+func (b *Branch) FullName() string {
+ if b.Name != "HEAD" {
+ return "refs/heads/" + b.Name
+ }
+ return b.Name
+}
+
// IsLocalOnly reports whether b is a local work branch (only local, not known to remote server).
func (b *Branch) IsLocalOnly() bool {
return "origin/"+b.Name != b.OriginBranch()
@@ -122,7 +129,7 @@ func (b *Branch) loadPending() {
// 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.Name, "--"))
+ all := trim(cmdOutput("git", "log", "--topo-order", "--format=format:%H%x00%h%x00%P%x00%B%x00%s%x00", origin+".."+b.FullName(), "--"))
fields := strings.Split(all, "\x00")
if len(fields) < numField {
return // nothing pending
@@ -175,7 +182,7 @@ func (b *Branch) loadPending() {
}
}
b.commitsAhead = len(b.pending)
- b.commitsBehind = len(trim(cmdOutput("git", "log", "--format=format:x", b.Name+".."+b.OriginBranch(), "--")))
+ b.commitsBehind = len(trim(cmdOutput("git", "log", "--format=format:x", b.FullName()+".."+b.OriginBranch(), "--")))
}
// Submitted reports whether some form of b's pending commit
diff --git a/git-codereview/gofmt.go b/git-codereview/gofmt.go
index 96b5e86..4608a3a 100644
--- a/git-codereview/gofmt.go
+++ b/git-codereview/gofmt.go
@@ -120,7 +120,7 @@ func runGofmt(flags int) (files []string, stderrText string) {
// Find files modified in the index compared to the branchpoint.
branchpt := b.Branchpoint()
- if strings.Contains(cmdOutput("git", "branch", "-r", "--contains", b.Name), "origin/") {
+ if strings.Contains(cmdOutput("git", "branch", "-r", "--contains", b.FullName()), "origin/") {
// This is a branch tag move, not an actual change.
// Use HEAD as branch point, so nothing will appear changed.
// We don't want to think about gofmt on already published
diff --git a/git-codereview/pending.go b/git-codereview/pending.go
index 1a58fe2..3239d23 100644
--- a/git-codereview/pending.go
+++ b/git-codereview/pending.go
@@ -15,8 +15,9 @@ import (
)
var (
- pendingLocal bool // -l flag, use only local operations (no network)
- pendingCurrent bool // -c flag, show only current branch
+ pendingLocal bool // -l flag, use only local operations (no network)
+ pendingCurrentOnly bool // -c flag, show only current branch
+ pendingShort bool // -s flag, short display
)
// A pendingBranch collects information about a single pending branch.
@@ -52,11 +53,12 @@ func (b *pendingBranch) load() {
}
func pending(args []string) {
- flags.BoolVar(&pendingCurrent, "c", false, "show only current branch")
+ flags.BoolVar(&pendingCurrentOnly, "c", false, "show only current branch")
flags.BoolVar(&pendingLocal, "l", false, "use only local information - no network operations")
+ flags.BoolVar(&pendingShort, "s", false, "show short listing")
flags.Parse(args)
if len(flags.Args()) > 0 {
- fmt.Fprintf(stderr(), "Usage: %s pending %s [-l]\n", os.Args[0], globalFlags)
+ fmt.Fprintf(stderr(), "Usage: %s pending %s [-c] [-l] [-s]\n", os.Args[0], globalFlags)
os.Exit(2)
}
@@ -67,13 +69,15 @@ func pending(args []string) {
}
// Build list of pendingBranch structs to be filled in.
+ // The current branch is always first.
var branches []*pendingBranch
- if pendingCurrent {
- branches = []*pendingBranch{{Branch: CurrentBranch(), current: true}}
- } else {
+ branches = []*pendingBranch{{Branch: CurrentBranch(), current: true}}
+ if !pendingCurrentOnly {
current := CurrentBranch().Name
for _, b := range LocalBranches() {
- branches = append(branches, &pendingBranch{Branch: b, current: b.Name == current})
+ if b.Name != current {
+ branches = append(branches, &pendingBranch{Branch: b})
+ }
}
}
@@ -110,45 +114,11 @@ func pending(args []string) {
<-done
}
- // Print output, like:
- // pending 2378abf..d8fcb99 https://go-review.googlesource.com/1620 (current branch, mailed, submitted, 1 behind)
- // git-codereview: expand pending output
- //
- // for pending:
- // - show full commit message
- // - show information about being behind upstream
- // - show list of modified files
- // - for current branch, show staged, unstaged, untracked files
- // - warn about being ahead of upstream on master
- // - warn about being multiple commits ahead of upstream
- //
- // - add same warnings to change
- // - add change -a (mostly unrelated, but prompted by this work)
- //
- // Change-Id: Ie480ba5b66cc07faffca421ee6c9623d35204696
- //
- // Code-Review:
- // +2 Andrew Gerrand, Rob Pike
- // Files in this change:
- // git-codereview/api.go
- // git-codereview/branch.go
- // git-codereview/change.go
- // git-codereview/pending.go
- // git-codereview/review.go
- // git-codereview/submit.go
- // git-codereview/sync.go
- // Files staged:
- // git-codereview/sync.go
- // Files unstaged:
- // git-codereview/submit.go
- // Files untracked:
- // git-codereview/doc.go
- // git-codereview/savedmail.go.txt
- //
+ // Print output.
// If there are multiple changes in the current branch, the output splits them out into separate sections,
// in reverse commit order, to match git log output.
//
- // wbshadow 7a524a1..a496c1e (current branch, all mailed, 23 behind)
+ // wbshadow 7a524a1..a496c1e (current branch, all mailed, 23 behind, tracking master)
// + uncommitted changes
// Files unstaged:
// src/runtime/proc1.go
@@ -189,13 +159,22 @@ func pending(args []string) {
// src/runtime/runtime2.go
// src/runtime/stack1.go
//
- // In multichange mode, the first line only gives information that applies to the entire
- // branch: the name, the commit range, whether this is the current branch, whether
- // all the commits are mailed/submitted, how far behind.
+ // The first line only gives information that applies to the entire branch:
+ // the name, the commit range, whether this is the current branch, whether
+ // all the commits are mailed/submitted, how far behind, what remote branch
+ // it is tracking.
// The individual change sections have per-change information: the hash of that
// commit, the URL on the Gerrit server, whether it is mailed/submitted, the list of
// files in that commit. The uncommitted file modifications are shown as a separate
// section, at the beginning, to fit better into the reverse commit order.
+ //
+ // The short view compresses the listing down to two lines per commit:
+ // wbshadow 7a524a1..a496c1e (current branch, all mailed, 23 behind, tracking master)
+ // + uncommitted changes
+ // Files unstaged:
+ // src/runtime/proc1.go
+ // + a496c1e runtime: add missing write barriers in append's copy of slice data (CL 2064, mailed)
+ // + 95390c7 runtime: add GODEBUG wbshadow for finding missing write barriers (CL 2061, mailed)
var buf bytes.Buffer
printFileList := func(name string, list []string) {
@@ -219,66 +198,79 @@ func pending(args []string) {
if len(work) > 0 {
fmt.Fprintf(&buf, " %.7s..%s", b.branchpoint, work[0].ShortHash)
}
- if len(work) == 1 && work[0].g.Number != 0 {
- fmt.Fprintf(&buf, " %s/%d", auth.url, work[0].g.Number)
- }
var tags []string
if b.current {
tags = append(tags, "current branch")
}
- if allMailed(work) {
- if len(work) == 1 {
- tags = append(tags, "mailed")
- } else if len(work) > 1 {
- tags = append(tags, "all mailed")
- }
+ if allMailed(work) && len(work) > 0 {
+ tags = append(tags, "all mailed")
}
- if allSubmitted(work) {
- if len(work) == 1 {
- tags = append(tags, "submitted")
- } else if len(work) > 1 {
- tags = append(tags, "all submitted")
- }
+ if allSubmitted(work) && len(work) > 0 {
+ tags = append(tags, "all submitted")
}
if b.commitsBehind > 0 {
tags = append(tags, fmt.Sprintf("%d behind", b.commitsBehind))
}
+ if b.OriginBranch() != "origin/master" {
+ tags = append(tags, "tracking "+strings.TrimPrefix(b.OriginBranch(), "origin/"))
+ }
if len(tags) > 0 {
fmt.Fprintf(&buf, " (%s)", strings.Join(tags, ", "))
}
fmt.Fprintf(&buf, "\n")
+ printed := false
if text := b.errors(); text != "" {
- fmt.Fprintf(&buf, "\tERROR: %s\n\n", strings.Replace(strings.TrimSpace(text), "\n", "\n\t", -1))
+ fmt.Fprintf(&buf, "\tERROR: %s\n", strings.Replace(strings.TrimSpace(text), "\n", "\n\t", -1))
+ if !pendingShort {
+ printed = true
+ fmt.Fprintf(&buf, "\n")
+ }
}
- if b.current && len(work) > 1 && len(b.staged)+len(b.unstaged)+len(b.untracked) > 0 {
+ if b.current && len(b.staged)+len(b.unstaged)+len(b.untracked) > 0 {
+ printed = true
fmt.Fprintf(&buf, "+ uncommitted changes\n")
- printFileList("staged", b.staged)
- printFileList("unstaged", b.unstaged)
printFileList("untracked", b.untracked)
- fmt.Fprintf(&buf, "\n")
+ printFileList("unstaged", b.unstaged)
+ printFileList("staged", b.staged)
+ if !pendingShort {
+ fmt.Fprintf(&buf, "\n")
+ }
}
for _, c := range work {
+ printed = true
g := c.g
- if len(work) > 1 {
- fmt.Fprintf(&buf, "+ %s", c.ShortHash)
- if g.Number != 0 {
- fmt.Fprintf(&buf, " %s/%d", auth.url, g.Number)
- }
- var tags []string
- if g.CurrentRevision == c.Hash {
- tags = append(tags, "mailed")
+ msg := strings.TrimRight(c.Message, "\r\n")
+ fmt.Fprintf(&buf, "+ %s", c.ShortHash)
+ var tags []string
+ if pendingShort {
+ if i := strings.Index(msg, "\n"); i >= 0 {
+ msg = msg[:i]
}
- if g.Status == "MERGED" {
- tags = append(tags, "submitted")
+ fmt.Fprintf(&buf, " %s", msg)
+ if g.Number != 0 {
+ tags = append(tags, fmt.Sprintf("CL %d%s", g.Number, codeReviewScores(g)))
}
- if len(tags) > 0 {
- fmt.Fprintf(&buf, " (%s)", strings.Join(tags, ", "))
+ } else {
+ if g.Number != 0 {
+ fmt.Fprintf(&buf, " %s/%d", auth.url, g.Number)
}
- fmt.Fprintf(&buf, "\n")
}
- msg := strings.TrimRight(c.Message, "\r\n")
+ if g.CurrentRevision == c.Hash {
+ tags = append(tags, "mailed")
+ }
+ if g.Status == "MERGED" {
+ tags = append(tags, "submitted")
+ }
+ if len(tags) > 0 {
+ fmt.Fprintf(&buf, " (%s)", strings.Join(tags, ", "))
+ }
+ fmt.Fprintf(&buf, "\n")
+ if pendingShort {
+ continue
+ }
+
fmt.Fprintf(&buf, "\t%s\n", strings.Replace(msg, "\n", "\n\t", -1))
fmt.Fprintf(&buf, "\n")
@@ -312,16 +304,9 @@ func pending(args []string) {
}
printFileList("in this change", c.committed)
- if b.current && len(work) == 1 {
- // staged file list will be printed next
- } else {
- fmt.Fprintf(&buf, "\n")
- }
+ fmt.Fprintf(&buf, "\n")
}
- if b.current && len(work) <= 1 {
- printFileList("staged", b.staged)
- printFileList("unstaged", b.unstaged)
- printFileList("untracked", b.untracked)
+ if pendingShort || !printed {
fmt.Fprintf(&buf, "\n")
}
}
@@ -329,6 +314,32 @@ func pending(args []string) {
stdout().Write(buf.Bytes())
}
+// codeReviewScores reports the code review scores as tags for the short output.
+func codeReviewScores(g *GerritChange) string {
+ label := g.Labels["Code-Review"]
+ if label == nil {
+ return ""
+ }
+ minValue := 10000
+ maxValue := -10000
+ for _, x := range label.All {
+ if minValue > x.Value {
+ minValue = x.Value
+ }
+ if maxValue < x.Value {
+ maxValue = x.Value
+ }
+ }
+ var scores string
+ if minValue < 0 {
+ scores += fmt.Sprintf(" %d", minValue)
+ }
+ if maxValue > 0 {
+ scores += fmt.Sprintf(" %+d", maxValue)
+ }
+ return scores
+}
+
// allMailed reports whether all commits in work have been posted to Gerrit.
func allMailed(work []*Commit) bool {
for _, c := range work {
diff --git a/git-codereview/pending_test.go b/git-codereview/pending_test.go
index f22d8d0..a52cdb8 100644
--- a/git-codereview/pending_test.go
+++ b/git-codereview/pending_test.go
@@ -42,6 +42,7 @@ func TestPendingBasic(t *testing.T) {
testPending(t, `
work REVHASH..REVHASH (current branch)
+ + REVHASH
msg
Change-Id: I123456789
@@ -83,7 +84,26 @@ func TestPendingComplex(t *testing.T) {
write(t, gt.client+"/bfile", "untracked")
testPending(t, `
+ work2 REVHASH..REVHASH (current branch)
+ + uncommitted changes
+ Files untracked:
+ bfile
+ Files unstaged:
+ file
+ file1
+ Files staged:
+ afile1
+ file1
+
+ + REVHASH
+ some changes
+
+ Files in this change:
+ file
+ file1
+
work REVHASH..REVHASH (5 behind)
+ + REVHASH
msg
Change-Id: I123456789
@@ -91,38 +111,41 @@ func TestPendingComplex(t *testing.T) {
Files in this change:
file
+ `)
+
+ testPendingArgs(t, []string{"-c"}, `
work2 REVHASH..REVHASH (current branch)
- some changes
-
- Files in this change:
+ + uncommitted changes
+ Files untracked:
+ bfile
+ Files unstaged:
file
file1
Files staged:
afile1
file1
- Files unstaged:
+
+ + REVHASH
+ some changes
+
+ Files in this change:
file
file1
- Files untracked:
- bfile
`)
- testPendingArgs(t, []string{"-c"}, `
+ testPendingArgs(t, []string{"-c", "-s"}, `
work2 REVHASH..REVHASH (current branch)
- some changes
-
- Files in this change:
+ + uncommitted changes
+ Files untracked:
+ bfile
+ Files unstaged:
file
file1
Files staged:
afile1
file1
- Files unstaged:
- file
- file1
- Files untracked:
- bfile
+ + REVHASH some changes
`)
}
@@ -140,12 +163,21 @@ func TestPendingErrors(t *testing.T) {
ERROR: Branch contains 1 commit not on origin/master.
Do not commit directly to master branch.
+ + REVHASH
v3
Files in this change:
file
`)
+
+ testPendingArgs(t, []string{"-s"}, `
+ master REVHASH..REVHASH (current branch)
+ ERROR: Branch contains 1 commit not on origin/master.
+ Do not commit directly to master branch.
+ + REVHASH v3
+
+ `)
}
func TestPendingMultiChange(t *testing.T) {
@@ -165,12 +197,12 @@ func TestPendingMultiChange(t *testing.T) {
testPending(t, `
work REVHASH..REVHASH (current branch)
+ uncommitted changes
- Files staged:
- file
- Files unstaged:
- file
Files untracked:
file2
+ Files unstaged:
+ file
+ Files staged:
+ file
+ REVHASH
v2
@@ -187,6 +219,20 @@ func TestPendingMultiChange(t *testing.T) {
file
`)
+
+ testPendingArgs(t, []string{"-s"}, `
+ work REVHASH..REVHASH (current branch)
+ + uncommitted changes
+ Files untracked:
+ file2
+ Files unstaged:
+ file
+ Files staged:
+ file
+ + REVHASH v2
+ + REVHASH msg
+
+ `)
}
func TestPendingGerrit(t *testing.T) {
@@ -200,6 +246,7 @@ func TestPendingGerrit(t *testing.T) {
// Test error from Gerrit server.
testPending(t, `
work REVHASH..REVHASH (current branch)
+ + REVHASH
msg
Change-Id: I123456789
@@ -219,6 +266,7 @@ func TestPendingGerrit(t *testing.T) {
trun(t, gt.server, "git", "commit", "-m", "msg")
testPendingArgs(t, []string{"-l"}, `
work REVHASH..REVHASH (current branch)
+ + REVHASH
msg
Change-Id: I123456789
@@ -228,9 +276,16 @@ func TestPendingGerrit(t *testing.T) {
`)
+ testPendingArgs(t, []string{"-l", "-s"}, `
+ work REVHASH..REVHASH (current branch)
+ + REVHASH msg
+
+ `)
+
// Without -l, the 1 behind should appear, as should Gerrit information.
testPending(t, `
- work REVHASH..REVHASH http://127.0.0.1:PORT/1234 (current branch, mailed, submitted, 1 behind)
+ work REVHASH..REVHASH (current branch, all mailed, all submitted, 1 behind)
+ + REVHASH http://127.0.0.1:PORT/1234 (mailed, submitted)
msg
Change-Id: I123456789
@@ -245,9 +300,16 @@ func TestPendingGerrit(t *testing.T) {
`)
+ testPendingArgs(t, []string{"-s"}, `
+ work REVHASH..REVHASH (current branch, all mailed, all submitted, 1 behind)
+ + REVHASH msg (CL 1234 -2 +1, mailed, submitted)
+
+ `)
+
// Since pending did a fetch, 1 behind should show up even with -l.
testPendingArgs(t, []string{"-l"}, `
work REVHASH..REVHASH (current branch, 1 behind)
+ + REVHASH
msg
Change-Id: I123456789
@@ -256,6 +318,11 @@ func TestPendingGerrit(t *testing.T) {
file
`)
+ testPendingArgs(t, []string{"-l", "-s"}, `
+ work REVHASH..REVHASH (current branch, 1 behind)
+ + REVHASH msg
+
+ `)
}
func TestPendingGerritMultiChange(t *testing.T) {
@@ -284,12 +351,12 @@ func TestPendingGerritMultiChange(t *testing.T) {
testPending(t, `
work REVHASH..REVHASH (current branch, all mailed)
+ uncommitted changes
- Files staged:
- file
- Files unstaged:
- file
Files untracked:
file2
+ Files unstaged:
+ file
+ Files staged:
+ file
+ REVHASH http://127.0.0.1:PORT/1234 (mailed)
v2
@@ -318,6 +385,20 @@ func TestPendingGerritMultiChange(t *testing.T) {
file
`)
+
+ testPendingArgs(t, []string{"-s"}, `
+ work REVHASH..REVHASH (current branch, all mailed)
+ + uncommitted changes
+ Files untracked:
+ file2
+ Files unstaged:
+ file
+ Files staged:
+ file
+ + REVHASH v2 (CL 1234 -2 +1, mailed)
+ + REVHASH msg (CL 1234 -2 +1, mailed, submitted)
+
+ `)
}
func testPendingReply(srv *gerritServer, id, rev, status string) {
diff --git a/git-codereview/review.go b/git-codereview/review.go
index b99aee5..964ba1e 100644
--- a/git-codereview/review.go
+++ b/git-codereview/review.go
@@ -88,10 +88,12 @@ Available commands:
mail -diff
Show the changes but do not send mail or upload.
- pending [-l]
+ pending [-c] [-l] [-s]
Show the status of all pending changes and staged, unstaged,
and untracked files in the local repository.
+ If -c is specified, show only changes on the current branch.
If -l is specified, only use locally available information.
+ If -s is specified, show short output.
submit
Push the pending change to the Gerrit server and tell Gerrit to
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index cd62f33..1fd2505 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -64,6 +64,7 @@ func (gt *gitTest) work(t *testing.T) {
if gt.nwork == 0 {
trun(t, gt.client, "git", "checkout", "-b", "work")
trun(t, gt.client, "git", "branch", "--set-upstream-to", "origin/master")
+ trun(t, gt.client, "git", "tag", "work") // make sure commands do the right thing when there is a tag of the same name
}
// make local change on client