git-codereview: pending: show unresolved comment count

Also fix a potential crash parsing a commit with no parents.

Change-Id: I1c289dde45230a3362f54037ea18023278b05ffd
Reviewed-on: https://go-review.googlesource.com/c/review/+/294129
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

4 files changed, 80 insertions, 28 deletions

diff --git a/git-codereview/api.go b/git-codereview/api.go
index afd421f..2adbcce 100644
--- a/git-codereview/api.go
+++ b/git-codereview/api.go
@@ -372,24 +372,26 @@ func readGerritChangesBatch(query string, n int, ch chan gerritChangeResult) {
 	ch <- gerritChangeResult{c, err}
 }
 
-// GerritChange is the JSON struct returned by a Gerrit CL query.
+// GerritChange is the JSON struct for a Gerrit ChangeInfo, returned by a Gerrit CL query.
 type GerritChange struct {
-	ID              string
-	Project         string
-	Branch          string
-	ChangeId        string `json:"change_id"`
-	Subject         string
-	Status          string
-	Created         string
-	Updated         string
-	Insertions      int
-	Deletions       int
-	Number          int `json:"_number"`
-	Owner           *GerritAccount
-	Labels          map[string]*GerritLabel
-	CurrentRevision string `json:"current_revision"`
-	Revisions       map[string]*GerritRevision
-	Messages        []*GerritMessage
+	ID                     string
+	Project                string
+	Branch                 string
+	ChangeId               string `json:"change_id"`
+	Subject                string
+	Status                 string
+	Created                string
+	Updated                string
+	Insertions             int
+	Deletions              int
+	Number                 int `json:"_number"`
+	Owner                  *GerritAccount
+	Labels                 map[string]*GerritLabel
+	CurrentRevision        string `json:"current_revision"`
+	Revisions              map[string]*GerritRevision
+	Messages               []*GerritMessage
+	TotalCommentCount      int `json:"total_comment_count"`
+	UnresolvedCommentCount int `json:"unresolved_comment_count"`
 }
 
 // LabelNames returns the label names for the change, in lexicographic order.
@@ -441,8 +443,53 @@ type GerritRevision struct {
 	Fetch  map[string]*GerritFetch
 }
 
-// GerritFetch is the JSON struct for a Gerrit FetchInfo
+// GerritFetch is the JSON struct for a Gerrit FetchInfo.
 type GerritFetch struct {
 	URL string
 	Ref string
 }
+
+// GerritComment is the JSON struct for a Gerrit CommentInfo.
+type GerritComment struct {
+	PatchSet        string `json:"patch_set"`
+	ID              string
+	Path            string
+	Side            string
+	Parent          string
+	Line            string
+	Range           *GerritCommentRange
+	InReplyTo       string
+	Message         string
+	Updated         string
+	Author          *GerritAccount
+	Tag             string
+	Unresolved      bool
+	ChangeMessageID string `json:"change_message_id"`
+	CommitID        string `json:"commit_id"` // SHA1 hex
+}
+
+// GerritCommentRange is the JSON struct for a Gerrit CommentRange.
+type GerritCommentRange struct {
+	StartLine      int `json:"start_line"`      // 1-based
+	StartCharacter int `json:"start_character"` // 0-based
+	EndLine        int `json:"end_line"`        // 1-based
+	EndCharacter   int `json:"end_character"`   // 0-based
+}
+
+// GerritContextLine is the JSON struct for a Gerrit ContextLine.
+type GerritContextLine struct {
+	LineNumber  int    `json:"line_number"` // 1-based
+	ContextLine string `json:"context_line"`
+}
+
+// GerritCommentInput is the JSON struct for a Gerrit CommentInput.
+type GerritCommentInput struct {
+	ID         string              `json:"id,omitempty"`   // ID of a draft comment to update
+	Path       string              `json:"path,omitempty"` // file to attach comment to
+	Side       string              `json:"side,omitempty"` // REVISION (default) or PARENT
+	Line       int                 `json:"line,omitempty"` // 0 to use range (or else file comment)
+	Range      *GerritCommentRange `json:"range,omitempty"`
+	InReplyTo  string              `json:"in_reply_to,omitempty"` // ID of comment being replied to
+	Message    string              `json:"message,omitempty"`
+	Unresolved *bool               `json:"unresolved,omitempty"` // defaults to parent setting or else false
+}
diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index d25c4c9..37da5fa 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -273,7 +273,6 @@ Log:
 		c := &Commit{
 			Hash:        fields[i],
 			ShortHash:   fields[i+1],
-			Parent:      parents[0],
 			Parents:     parents,
 			Tree:        fields[i+3],
 			Message:     fields[i+4],
@@ -282,7 +281,9 @@ Log:
 			AuthorEmail: fields[i+7],
 			AuthorDate:  fields[i+8],
 		}
-
+		if len(c.Parents) > 0 {
+			c.Parent = c.Parents[0]
+		}
 		if len(c.Parents) > 1 {
 			// Found merge point.
 			// Merges break the invariant that the last shared commit (the branchpoint)
diff --git a/git-codereview/pending.go b/git-codereview/pending.go
index 3ed50ec..c9b99e7 100644
--- a/git-codereview/pending.go
+++ b/git-codereview/pending.go
@@ -332,6 +332,9 @@ func formatCommit(w io.Writer, c *Commit, short bool) {
 		}
 		tags = append(tags, "merge="+strings.Join(h, ","))
 	}
+	if g.UnresolvedCommentCount > 0 {
+		tags = append(tags, fmt.Sprintf("%d unresolved comments", g.UnresolvedCommentCount))
+	}
 	if len(tags) > 0 {
 		fmt.Fprintf(w, " (%s)", strings.Join(tags, ", "))
 	}
diff --git a/git-codereview/pending_test.go b/git-codereview/pending_test.go
index 35e51ac..d46c1d8 100644
--- a/git-codereview/pending_test.go
+++ b/git-codereview/pending_test.go
@@ -230,7 +230,7 @@ func TestPendingGerrit(t *testing.T) {
 
 	`)
 
-	testPendingReply(srv, "I123456789", CurrentBranch().Pending()[0].Hash, "MERGED")
+	testPendingReply(srv, "I123456789", CurrentBranch().Pending()[0].Hash, "MERGED", 0)
 
 	// Test local mode does not talk to any server.
 	// Make client 1 behind server.
@@ -320,8 +320,8 @@ func TestPendingGerritMultiChange(t *testing.T) {
 	srv := newGerritServer(t)
 	defer srv.done()
 
-	testPendingReply(srv, "I123456789", hash1, "MERGED")
-	testPendingReply(srv, "I2345", hash2, "NEW")
+	testPendingReply(srv, "I123456789", hash1, "MERGED", 0)
+	testPendingReply(srv, "I2345", hash2, "NEW", 99)
 
 	testPending(t, `
 		work REVHASH..REVHASH (current branch, all mailed)
@@ -333,7 +333,7 @@ func TestPendingGerritMultiChange(t *testing.T) {
 			Files staged:
 				file
 
-		+ REVHASH http://127.0.0.1:PORT/1234 (mailed)
+		+ REVHASH http://127.0.0.1:PORT/1234 (mailed, 99 unresolved comments)
 			v2
 
 			Change-Id: I2345
@@ -370,7 +370,7 @@ func TestPendingGerritMultiChange(t *testing.T) {
 				file
 			Files staged:
 				file
-		+ REVHASH v2 (CL 1234 -2 +1, mailed)
+		+ REVHASH v2 (CL 1234 -2 +1, mailed, 99 unresolved comments)
 		+ REVHASH msg (CL 1234 -2 +1, mailed, submitted)
 
 	`)
@@ -384,13 +384,13 @@ func TestPendingGerritMultiChange15(t *testing.T) {
 
 	gt.work(t)
 	hash1 := CurrentBranch().Pending()[0].Hash
-	testPendingReply(srv, "I123456789", hash1, "MERGED")
+	testPendingReply(srv, "I123456789", hash1, "MERGED", 0)
 
 	for i := 1; i < 15; i++ {
 		write(t, gt.client+"/file", fmt.Sprintf("v%d", i), 0644)
 		trun(t, gt.client, "git", "commit", "-a", "-m", fmt.Sprintf("v%d\n\nChange-Id: I%010d", i, i))
 		hash2 := CurrentBranch().Pending()[0].Hash
-		testPendingReply(srv, fmt.Sprintf("I%010d", i), hash2, "NEW")
+		testPendingReply(srv, fmt.Sprintf("I%010d", i), hash2, "NEW", 0)
 	}
 
 	testPendingArgs(t, []string{"-s"}, `
@@ -414,12 +414,13 @@ func TestPendingGerritMultiChange15(t *testing.T) {
 	`)
 }
 
-func testPendingReply(srv *gerritServer, id, rev, status string) {
+func testPendingReply(srv *gerritServer, id, rev, status string, unresolved int) {
 	srv.setJSON(id, `{
 		"id": "proj~main~`+id+`",
 		"project": "proj",
 		"current_revision": "`+rev+`",
 		"status": "`+status+`",
+		"unresolved_comment_count":`+fmt.Sprint(unresolved)+`,
 		"_number": 1234,
 		"owner": {"_id": 42},
 		"labels": {