diff options
Diffstat (limited to 'git-codereview')
| -rw-r--r-- | git-codereview/branch.go | 13 | ||||
| -rw-r--r-- | git-codereview/gofmt.go | 2 | ||||
| -rw-r--r-- | git-codereview/pending.go | 191 | ||||
| -rw-r--r-- | git-codereview/pending_test.go | 129 | ||||
| -rw-r--r-- | git-codereview/review.go | 4 | ||||
| -rw-r--r-- | git-codereview/util_test.go | 1 |
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 |
