diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 7c1efff1..adaddb48 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -468,11 +468,9 @@ const ( pr create pr diff pr edit - pr list pr merge pr reopen pr status - pr view search issue search pr status @@ -488,7 +486,6 @@ const ( label milestone milestone issue - pr pr attachment pr comment time diff --git a/internal/app/discussion_view.go b/internal/app/discussion_view.go index 9515fe20..e6148aa5 100644 --- a/internal/app/discussion_view.go +++ b/internal/app/discussion_view.go @@ -4,8 +4,10 @@ package app import ( + "cmp" "fmt" "slices" + "strconv" "strings" "time" @@ -14,35 +16,89 @@ import ( "github.com/spf13/cobra" ) -// discussionView holds the fields `issue view` and `pr view` render. +// discussionView is what issue view and pr view show: a terminal.View with the discussion's +// comments, which --comments adds before the footer. type discussionView struct { - kind, title, state, author, action, createdAt, url, body string - number, comments int64 - labels []services.Label - fields [][2]string + view terminal.View + number, comments int64 + // summary describes the author and history after the state badge, relative to now, such as + // "bob opened about 2 hours ago"; fields are the view's fields at that time. + summary func(now time.Time) string + fields func(now time.Time) []terminal.Field // extraComments adds kind-specific entries, such as pull request reviews, to --comments. extraComments func(*cobra.Command, target, int64) ([]discussionComment, error) } -// discussionComment is one --comments entry; heading and meta are empty for plain comments. +// discussionComment is one --comments entry. heading words what a review did, such as "approved +// these changes"; plain comments have none, but an ID. meta is a key and value for piped output. type discussionComment struct { - author, createdAt, heading, body string + author, heading, createdAt, body string + id int64 meta [2]string } +// stateBadge shows a discussion state, such as Open or Merged, in its color. +func stateBadge(state string) terminal.Cell { + return terminal.Cell{Text: titleCase(state), Color: stateColors[state]} +} + +// labelCells shows labels in their colors. +func labelCells(labels []services.Label) []terminal.Cell { + cells := make([]terminal.Cell, len(labels)) + for i, label := range labels { + cells[i] = terminal.Cell{Text: label.Name, Color: label.Color} + } + return cells +} + +// whenText words an API time relative to now, or returns it unchanged when it is no time. +func whenText(now time.Time, raw string) string { + if t := terminal.ParseTime(raw); !t.IsZero() { + return terminal.RelativeTime(now, t) + } + return raw +} + func issueView(item services.Issue, repo string, dependencies services.IssueDependencies) discussionView { + due := "" + if item.Deadline != nil { + due = *item.Deadline + } return discussionView{ - kind: "issue", title: item.Title, state: item.State, author: item.Author, action: "opened", - createdAt: item.CreatedAt, url: item.URL, body: item.Body, number: item.Number, comments: item.Comments, labels: item.Labels, - fields: [][2]string{ - {"Assignees", strings.Join(item.Assignees, ", ")}, - {"Milestone", milestoneTitle(item.Milestone)}, - {"Blocked by", issueReferences(repo, dependencies.BlockedBy)}, - {"Blocks", issueReferences(repo, dependencies.Blocks)}, + number: item.Number, comments: item.Comments, + summary: func(now time.Time) string { return item.Author + " opened " + whenText(now, item.CreatedAt) }, + fields: func(now time.Time) []terminal.Field { + return []terminal.Field{ + {Label: "State", Value: item.State, PipedOnly: true}, + {Label: "Author", Value: item.Author, PipedOnly: true}, + {Label: "Labels", Cells: labelCells(item.Labels)}, + {Label: "Assignees", Value: strings.Join(item.Assignees, ", ")}, + {Label: "Milestone", Value: milestoneTitle(item.Milestone)}, + {Label: "Due", Value: dueText(now, due), TTYOnly: true}, + {Label: "Due", Value: due, PipedOnly: true}, + {Label: "Ref", Value: item.Ref}, + {Label: "Blocked by", Value: issueReferences(repo, dependencies.BlockedBy)}, + {Label: "Blocks", Value: issueReferences(repo, dependencies.Blocks)}, + {Label: "Comments", Value: strconv.FormatInt(item.Comments, 10), PipedOnly: true}, + {Label: "Number", Value: strconv.FormatInt(item.Number, 10), PipedOnly: true}, + } + }, + view: terminal.View{ + Title: item.Title, ID: "#" + strconv.FormatInt(item.Number, 10), Badges: []terminal.Cell{stateBadge(item.State)}, + Body: item.Body, Kind: "issue", URL: item.URL, }, } } +// dueText words a due date for people: "Dec 24, 2026 (in about 2 months)". +func dueText(now time.Time, raw string) string { + due := terminal.ParseTime(raw) + if due.IsZero() { + return raw + } + return due.UTC().Format("Jan 2, 2006") + " (" + terminal.RelativeTime(now, due) + ")" +} + // issueReferences lists issues as #N, qualifying those from other repositories. func issueReferences(repo string, refs []services.IssueReference) string { names := make([]string, len(refs)) @@ -59,52 +115,146 @@ func issueReferences(repo string, refs []services.IssueReference) string { return strings.Join(names, ", ") } -func pullView(item services.Pull) discussionView { - state := pullState(item) - v := discussionView{ - kind: "pull request", title: item.Title, state: state, author: item.Author, - action: fmt.Sprintf("wants to merge into %s from %s •", item.Base, item.Head), - createdAt: item.CreatedAt, url: item.URL, body: item.Body, number: item.Number, comments: item.Comments, labels: item.Labels, - fields: [][2]string{ - {"Reviewers", strings.Join(slices.Concat(item.Reviewers, item.ReviewerTeams), ", ")}, - {"Assignees", strings.Join(item.Assignees, ", ")}, - {"Milestone", milestoneTitle(item.Milestone)}, - }, - extraComments: pullReviewComments, - } - if state == "open" { - mergeable := "no" - if item.Mergeable { - mergeable = "yes" - } - v.fields = append(v.fields, [2]string{"Mergeable", mergeable}) - } - return v +// pullDetails are what pr view reads besides the pull request: the reviews, and the checks of an +// open pull request's head. +type pullDetails struct { + reviews []services.Review + checks []services.Check } -// pullReviewComments returns submitted review bodies and inline review comments. -func pullReviewComments(cmd *cobra.Command, target target, n int64) ([]discussionComment, error) { - reviews, inline, err := (services.Reviews{Client: target.client, Repository: target.Repo}).Thread(cmd.Context(), n) - if err != nil { - return nil, err +func pullView(item services.Pull, repo string, details pullDetails) discussionView { + state := pullState(item) + head := pullHead(item, repo) + summary := func(now time.Time) string { + return fmt.Sprintf("%s wants to merge into %s from %s • %s", item.Author, item.Base, head, whenText(now, item.CreatedAt)) } + if item.Merged { + summary = func(now time.Time) string { + merged := cmp.Or(item.MergedBy, item.Author) + " merged into " + item.Base + " from " + head + if item.MergedAt != nil { + merged += " • " + whenText(now, *item.MergedAt) + } + return merged + } + } + mergeable := "" + switch { + case state == "draft": + mergeable = fmt.Sprintf("no — draft; mark it ready with: tea pr ready %d", item.Number) + case state != "open": + case item.Mergeable: + mergeable = "yes" + default: + mergeable = "no — conflicts with " + item.Base + } + checks, checksHint := checkSummary(details.checks), "" + if checks != "" { + checksHint = fmt.Sprintf("%s (tea pr checks %d)", checks, item.Number) + } + reviews := details.reviews + return discussionView{ + number: item.Number, comments: item.Comments, summary: summary, + view: terminal.View{ + Title: item.Title, ID: "#" + strconv.FormatInt(item.Number, 10), Badges: []terminal.Cell{stateBadge(state)}, + Body: item.Body, Kind: "pull request", URL: item.URL, + }, + fields: func(time.Time) []terminal.Field { + return []terminal.Field{ + {Label: "State", Value: state, PipedOnly: true}, + {Label: "Author", Value: item.Author, PipedOnly: true}, + {Label: "Base", Value: item.Base, PipedOnly: true}, + {Label: "Head", Value: head, PipedOnly: true}, + {Label: "Labels", Cells: labelCells(item.Labels)}, + {Label: "Assignees", Value: strings.Join(item.Assignees, ", ")}, + {Label: "Reviewers", Value: reviewerSummary(item, details.reviews)}, + {Label: "Milestone", Value: milestoneTitle(item.Milestone)}, + {Label: "Checks", Value: checksHint, TTYOnly: true}, + {Label: "Checks", Value: checks, PipedOnly: true}, + {Label: "Mergeable", Value: mergeable}, + {Label: "Comments", Value: strconv.FormatInt(item.Comments, 10), PipedOnly: true}, + {Label: "Number", Value: strconv.FormatInt(item.Number, 10), PipedOnly: true}, + } + }, + extraComments: func(cmd *cobra.Command, t target, n int64) ([]discussionComment, error) { + return pullReviewComments(cmd, t, n, reviews) + }, + } +} + +// reviewerSummary lists each reviewer with their latest review state, then the requested reviewers +// and teams that have not reviewed yet: "bob (Approved), carol (Requested)". +func reviewerSummary(item services.Pull, reviews []services.Review) string { + var names []string + seen := map[string]bool{} + for _, review := range services.LatestReviews(reviews) { + seen[strings.ToLower(review.Author)] = true + names = append(names, review.Author+" ("+reviewStateCell(review).Text+")") + } + for _, name := range slices.Concat(item.Reviewers, item.ReviewerTeams) { + if !seen[strings.ToLower(name)] { + names = append(names, name+" (Requested)") + } + } + return strings.Join(names, ", ") +} + +// checkSummary counts checks by outcome: "1 failing, 2 pending, 3 successful", or "" without checks. +func checkSummary(checks []services.Check) string { + var failing, pending, passed int + for _, check := range checks { + switch { + case check.Pending(): + pending++ + case check.Passed(): + passed++ + default: + failing++ + } + } + var parts []string + for _, count := range []struct { + n int + word string + }{{failing, "failing"}, {pending, "pending"}, {passed, "successful"}} { + if count.n > 0 { + parts = append(parts, fmt.Sprintf("%d %s", count.n, count.word)) + } + } + return strings.Join(parts, ", ") +} + +// reviewHeadings word what a review did in --comments, by its state. +var reviewHeadings = map[string]string{ + "APPROVED": "approved these changes", "REQUEST_CHANGES": "requested changes", "COMMENT": "reviewed", +} + +// pullReviewComments returns the submitted reviews, including those without a body, and the inline +// review comments of pull request n. +func pullReviewComments(cmd *cobra.Command, t target, n int64, reviews []services.Review) ([]discussionComment, error) { var result []discussionComment for _, review := range reviews { - if strings.TrimSpace(review.Body) == "" || review.State == "PENDING" { + heading, ok := reviewHeadings[review.State] + if !ok { continue } + if review.Dismissed { + heading += " (dismissed)" + } result = append(result, discussionComment{ - author: review.Author, createdAt: review.SubmittedAt, body: review.Body, - heading: "reviewed (" + review.State + ")", meta: [2]string{"review", review.State}, + author: review.Author, createdAt: review.SubmittedAt, body: review.Body, heading: heading, meta: [2]string{"review", review.State}, }) } + inline, err := (services.Reviews{Client: t.client, Repository: t.Repo}).Inline(cmd.Context(), n, reviews) + if err != nil { + return nil, err + } for _, comment := range inline { location := comment.Path if comment.Line > 0 { location = fmt.Sprintf("%s:%d", comment.Path, comment.Line) } result = append(result, discussionComment{ - author: comment.Author, createdAt: comment.CreatedAt, body: comment.Body, + author: comment.Author, createdAt: comment.CreatedAt, body: comment.Body, id: comment.ID, heading: "commented on " + location, meta: [2]string{"path", location}, }) } @@ -118,8 +268,9 @@ func milestoneTitle(m *services.IssueMilestone) string { return m.Title } -// writeDiscussion renders a gh-style detail view: a readable summary on a -// terminal and tab-separated key/value lines followed by the body otherwise. +// writeDiscussion renders an issue or pull request like gh: a terminal.View, then the comments when +// withComments is set, or a hint at them, and the footer. Piped, the comments follow the view as +// key/value blocks. func (a *application) writeDiscussion(cmd *cobra.Command, target target, v discussionView, withComments bool) error { var comments []discussionComment if withComments { @@ -128,7 +279,7 @@ func (a *application) writeDiscussion(cmd *cobra.Command, target target, v discu return err } for _, item := range items { - comments = append(comments, discussionComment{author: item.Author, createdAt: item.CreatedAt, body: item.Body}) + comments = append(comments, discussionComment{author: item.Author, createdAt: item.CreatedAt, body: item.Body, id: item.ID}) } if v.extraComments != nil { extra, err := v.extraComments(cmd, target, v.number) @@ -136,99 +287,66 @@ func (a *application) writeDiscussion(cmd *cobra.Command, target target, v discu return err } comments = append(comments, extra...) - slices.SortStableFunc(comments, func(x, y discussionComment) int { return compareTimes(x.createdAt, y.createdAt) }) + slices.SortStableFunc(comments, func(x, y discussionComment) int { + return terminal.ParseTime(x.createdAt).Compare(terminal.ParseTime(y.createdAt)) + }) } } - v.title, v.body = terminal.SafeLine(v.title), terminal.SafeText(v.body) - for i := range v.fields { - v.fields[i][1] = terminal.SafeLine(v.fields[i][1]) + d := a.display() + now := d.Now + if now.IsZero() { + now = time.Now() } - for i := range comments { - c := &comments[i] - c.heading, c.meta[1], c.body = terminal.SafeLine(c.heading), terminal.SafeLine(c.meta[1]), terminal.SafeText(c.body) - } - color := a.deps.Streams.OutTTY && a.colorEnabled() - labels := make([]string, len(v.labels)) - for i, label := range v.labels { - labels[i] = terminal.Colorize(label.Color, terminal.SafeLine(label.Name), color) - } - var b strings.Builder - if !a.deps.Streams.OutTTY { - fmt.Fprintf(&b, "title:\t%s\nstate:\t%s\nauthor:\t%s\nlabels:\t%s\n", v.title, v.state, v.author, strings.Join(labels, ", ")) - for _, field := range v.fields { - fmt.Fprintf(&b, "%s:\t%s\n", strings.ToLower(field[0]), field[1]) - } - fmt.Fprintf(&b, "comments:\t%d\nnumber:\t%d\nurl:\t%s\n--\n%s\n", v.comments, v.number, v.url, v.body) + view := v.view + view.Fields = v.fields(now) + if !d.TTY { + var b strings.Builder + b.WriteString(view.Render(d)) for _, comment := range comments { - fmt.Fprintf(&b, "--\nauthor:\t%s\ncreated_at:\t%s\n", comment.author, comment.createdAt) - if comment.meta[0] != "" { - fmt.Fprintf(&b, "%s:\t%s\n", comment.meta[0], comment.meta[1]) + fmt.Fprintf(&b, "--\nauthor:\t%s\ncreated_at:\t%s\n", terminal.SafeLine(comment.author), terminal.SafeLine(comment.createdAt)) + if comment.id > 0 { + fmt.Fprintf(&b, "id:\t%d\n", comment.id) } - fmt.Fprintf(&b, "--\n%s\n", comment.body) + if comment.meta[0] != "" { + fmt.Fprintf(&b, "%s:\t%s\n", comment.meta[0], terminal.SafeLine(comment.meta[1])) + } + fmt.Fprintf(&b, "--\n%s\n", terminal.SafeText(comment.body)) } _, err := fmt.Fprint(cmd.OutOrStdout(), b.String()) return err } - now := a.deps.Now() - fmt.Fprintf(&b, "%s #%d\n", terminal.Emphasis(v.title, color), v.number) - fmt.Fprintf(&b, "%s • %s %s %s • %s\n", terminal.Colorize(stateColors[v.state], titleCase(v.state), color), v.author, v.action, relativeTime(now, v.createdAt), plural(int(v.comments), "comment")) - for _, field := range append([][2]string{{"Labels", strings.Join(labels, ", ")}}, v.fields...) { - if field[1] != "" { - fmt.Fprintf(&b, "%s: %s\n", terminal.Emphasis(field[0], color), field[1]) - } + view.Meta = v.summary(now) + " • " + plural(int(v.comments), "comment") + view.URL = "" + muted := func(text string) string { return terminal.Colorize("muted", text, d.Color) } + var b strings.Builder + b.WriteString(view.Render(d)) + if !withComments && v.comments > 0 { + b.WriteString("\n" + muted(plural(int(v.comments), "comment")+" — use --comments to view them") + "\n") } - body := strings.TrimSpace(v.body) - if body == "" { - body = "No description provided" - } - fmt.Fprintf(&b, "\n%s\n\n", body) for _, comment := range comments { - heading := "" + heading := []string{terminal.Emphasis(terminal.SafeLine(comment.author), d.Color)} if comment.heading != "" { - heading = comment.heading + " • " + heading[0] += " " + terminal.SafeLine(comment.heading) + } + heading = append(heading, whenText(now, comment.createdAt)) + if comment.id > 0 && comment.meta[0] == "" { + heading = append(heading, "#"+strconv.FormatInt(comment.id, 10)) + } + body := strings.TrimSpace(comment.body) + if d.Markdown && body != "" { + body = terminal.Markdown(body, d) + } else { + body = terminal.SafeText(body) + } + b.WriteString("\n" + strings.Join(heading, " • ") + "\n") + if body != "" { + b.WriteString(body + "\n") } - fmt.Fprintf(&b, "%s • %s%s\n%s\n\n", terminal.Emphasis(comment.author, color), heading, relativeTime(now, comment.createdAt), strings.TrimSpace(comment.body)) } - fmt.Fprintf(&b, "View this %s on Gitea: %s\n", v.kind, v.url) + b.WriteString("\n" + muted("View this "+v.view.Kind+" on Gitea: "+terminal.SafeLine(v.view.URL)) + "\n") return a.writeHuman(cmd, b.String()) } -// compareTimes orders API timestamps chronologically, falling back to text order. -func compareTimes(x, y string) int { - tx, errX := time.Parse(time.RFC3339, x) - ty, errY := time.Parse(time.RFC3339, y) - if errX != nil || errY != nil { - return strings.Compare(x, y) - } - return tx.Compare(ty) -} - -// relativeTime formats an API timestamp like gh: "about 3 days ago". -func relativeTime(now time.Time, raw string) string { - t, err := time.Parse(time.RFC3339, raw) - if err != nil { - return raw - } - const day = 24 * time.Hour - var n time.Duration - var unit string - switch d := now.Sub(t); { - case d < time.Minute: - return "less than a minute ago" - case d < time.Hour: - n, unit = d/time.Minute, "minute" - case d < day: - n, unit = d/time.Hour, "hour" - case d < 30*day: - n, unit = d/day, "day" - case d < 365*day: - n, unit = d/(30*day), "month" - default: - n, unit = d/(365*day), "year" - } - return "about " + plural(int(n), unit) + " ago" -} - func plural(n int, unit string) string { if n == 1 { return "1 " + unit diff --git a/internal/app/discussion_view_test.go b/internal/app/discussion_view_test.go index 50ac0223..da96d501 100644 --- a/internal/app/discussion_view_test.go +++ b/internal/app/discussion_view_test.go @@ -12,17 +12,23 @@ import ( "github.com/stretchr/testify/require" ) +// plainTTY makes deps render for a terminal without color or markdown, so views compare as text. +func plainTTY(deps *Dependencies, serverURL string) { + deps.Streams.OutTTY = true + deps.Getenv = serverEnv(serverURL, map[string]string{"NO_COLOR": "1", "TEA_MARKDOWN": "raw"}) +} + func TestDiscussionViews(t *testing.T) { server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/api/v1/repos/alice/project/issues/3": - _, _ = w.Write([]byte(`{"number":3,"title":"Crash\u001b[2J","body":"Steps:\r\n\u001b]52;c;aGk=\u0007run\u202e\r","html_url":"https://example.org/i/3","state":"open","user":{"login":"bob"},"created_at":"2026-01-01T00:00:00Z","labels":[{"name":"bug","color":"ff0000"}],"assignees":[{"login":"alice"}],"comments":1}`)) - case "/api/v1/repos/alice/project/issues/3/dependencies", "/api/v1/repos/alice/project/issues/3/blocks": + _, _ = w.Write([]byte(`{"number":3,"title":"Crash\u001b[2J","body":"Steps:\r\n\u001b]52;c;aGk=\u0007run‮\r","html_url":"https://example.org/i/3","state":"open","user":{"login":"bob"},"created_at":"2026-01-01T00:00:00Z","labels":[{"name":"bug","color":"ff0000"}],"assignees":[{"login":"alice"}],"comments":1}`)) + case "/api/v1/repos/alice/project/issues/3/dependencies", "/api/v1/repos/alice/project/issues/3/blocks", "/api/v1/repos/alice/project/pulls/4/reviews": _, _ = w.Write([]byte(`[]`)) case "/api/v1/repos/alice/project/issues/3/comments": - _, _ = w.Write([]byte(`[{"body":"Confirmed\u001b[31m\n\tok","user":{"login":"carol"},"created_at":"2026-01-02T00:00:00Z"}]`)) + _, _ = w.Write([]byte(`[{"id":79,"body":"Confirmed\u001b[31m\n\tok","user":{"login":"carol"},"created_at":"2026-01-02T00:00:00Z"}]`)) case "/api/v1/repos/alice/project/pulls/4": - _, _ = w.Write([]byte(`{"number":4,"title":"Fix","html_url":"https://example.org/p/4","state":"closed","merged":true,"user":{"login":"bob"},"created_at":"2026-01-03T00:00:00Z","base":{"ref":"main"},"head":{"ref":"fix"},"requested_reviewers":[{"login":"alice"}]}`)) + _, _ = w.Write([]byte(`{"number":4,"title":"Fix","html_url":"https://example.org/p/4","state":"closed","merged":true,"merged_at":"2026-01-03T12:00:00Z","merged_by":{"login":"alice"},"user":{"login":"bob"},"created_at":"2026-01-03T00:00:00Z","base":{"ref":"main"},"head":{"ref":"fix"},"requested_reviewers":[{"login":"carol"}]}`)) default: t.Errorf("unexpected %s %s", r.Method, r.URL.Path) } @@ -31,12 +37,19 @@ func TestDiscussionViews(t *testing.T) { deps, out, _ := serverDeps(t, server.URL) deps.Now = func() time.Time { return time.Date(2026, 1, 4, 0, 0, 0, 0, time.UTC) } require.NoError(t, Execute(context.Background(), deps, []string{"issue", "view", "3", "-R", "alice/project", "--comments"})) - require.Equal(t, "title:\tCrash[2J\nstate:\topen\nauthor:\tbob\nlabels:\tbug\nassignees:\talice\nmilestone:\t\nblocked by:\t\nblocks:\t\ncomments:\t1\nnumber:\t3\nurl:\thttps://example.org/i/3\n--\nSteps:\n]52;c;aGk=run\n--\nauthor:\tcarol\ncreated_at:\t2026-01-02T00:00:00Z\n--\nConfirmed[31m\n\tok\n", out.String()) + require.Equal(t, "title:\tCrash[2J\nstate:\topen\nauthor:\tbob\nlabels:\tbug\nassignees:\talice\nmilestone:\t\ndue:\t\nref:\t\nblocked by:\t\nblocks:\t\ncomments:\t1\nnumber:\t3\nurl:\thttps://example.org/i/3\n--\nSteps:\n]52;c;aGk=run\n"+ + "--\nauthor:\tcarol\ncreated_at:\t2026-01-02T00:00:00Z\nid:\t79\n--\nConfirmed[31m\n\tok\n", out.String()) + out.Reset() + plainTTY(&deps, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"issue", "view", "3", "-R", "alice/project"})) + require.Equal(t, "Crash[2J #3\nOpen • bob opened about 3 days ago • 1 comment\nLabels: bug\nAssignees: alice\n\nSteps:\n]52;c;aGk=run\n\n"+ + "1 comment — use --comments to view them\n\nView this issue on Gitea: https://example.org/i/3\n", out.String()) + out.Reset() + require.NoError(t, Execute(context.Background(), deps, []string{"issue", "view", "3", "-R", "alice/project", "--comments"})) + require.Contains(t, out.String(), "\n\ncarol • about 2 days ago • #79\nConfirmed[31m\n\tok\n\nView this issue on Gitea:") out.Reset() - deps.Streams.OutTTY = true - deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"}) require.NoError(t, Execute(context.Background(), deps, []string{"pr", "view", "4", "-R", "alice/project"})) - require.Equal(t, "Fix #4\nMerged • bob wants to merge into main from fix • about 1 day ago • 0 comments\nReviewers: alice\n\nNo description provided\n\nView this pull request on Gitea: https://example.org/p/4\n", out.String()) + require.Equal(t, "Fix #4\nMerged • alice merged into main from fix • about 12 hours ago • 0 comments\nReviewers: carol (Requested)\n\nNo description provided\n\nView this pull request on Gitea: https://example.org/p/4\n", out.String()) } func TestIssueViewDependencies(t *testing.T) { @@ -46,7 +59,7 @@ func TestIssueViewDependencies(t *testing.T) { paths = append(paths, r.URL.Path) switch { case r.URL.Path == "/api/v1/repos/alice/project/issues/3": - _, _ = w.Write([]byte(`{"number":3,"title":"Crash","html_url":"https://example.org/i/3","state":"open","user":{"login":"bob"},"created_at":"2026-01-03T00:00:00Z"}`)) + _, _ = w.Write([]byte(`{"number":3,"title":"Crash","html_url":"https://example.org/i/3","state":"open","user":{"login":"bob"},"created_at":"2026-01-03T00:00:00Z","due_date":"2026-12-24T23:59:59Z","ref":"main"}`)) case !supported: w.WriteHeader(http.StatusNotFound) case r.URL.Query().Get("page") != "1": @@ -62,15 +75,14 @@ func TestIssueViewDependencies(t *testing.T) { defer server.Close() deps, out, _ := serverDeps(t, server.URL) deps.Now = func() time.Time { return time.Date(2026, 1, 4, 0, 0, 0, 0, time.UTC) } - deps.Streams.OutTTY = true - deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"}) + plainTTY(&deps, server.URL) args := []string{"issue", "view", "3", "-R", "alice/project"} require.NoError(t, Execute(context.Background(), deps, args)) - require.Equal(t, "Crash #3\nOpen • bob opened about 1 day ago • 0 comments\nBlocked by: #1, bob/lib#5, hidden\nBlocks: #9\n\nNo description provided\n\nView this issue on Gitea: https://example.org/i/3\n", out.String()) + require.Equal(t, "Crash #3\nOpen • bob opened about 1 day ago • 0 comments\nDue: Dec 24, 2026 (in about 11 months)\nRef: main\nBlocked by: #1, bob/lib#5, hidden\nBlocks: #9\n\nNo description provided\n\nView this issue on Gitea: https://example.org/i/3\n", out.String()) out.Reset() deps.Streams.OutTTY = false require.NoError(t, Execute(context.Background(), deps, args)) - require.Contains(t, out.String(), "\nblocked by:\t#1, bob/lib#5, hidden\nblocks:\t#9\n") + require.Contains(t, out.String(), "\ndue:\t2026-12-24T23:59:59Z\nref:\tmain\nblocked by:\t#1, bob/lib#5, hidden\nblocks:\t#9\n") out.Reset() supported, paths = false, nil require.NoError(t, Execute(context.Background(), deps, args)) @@ -82,20 +94,6 @@ func TestIssueViewDependencies(t *testing.T) { require.Equal(t, []string{"/api/v1/repos/alice/project/issues/3"}, paths) } -func TestRelativeTime(t *testing.T) { - now := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) - for raw, want := range map[string]string{ - "2025-12-31T23:59:30Z": "less than a minute ago", - "2025-12-31T23:59:00Z": "about 1 minute ago", - "2025-12-31T21:00:00Z": "about 3 hours ago", - "2025-11-01T00:00:00Z": "about 2 months ago", - "2023-12-01T00:00:00Z": "about 2 years ago", - "not a time": "not a time", - } { - require.Equal(t, want, relativeTime(now, raw), raw) - } -} - func TestPullViewCommentsIncludeReviews(t *testing.T) { server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -125,19 +123,71 @@ func TestPullViewCommentsIncludeReviews(t *testing.T) { deps, out, _ := serverDeps(t, server.URL) deps.Now = func() time.Time { return time.Date(2026, 1, 4, 0, 0, 0, 0, time.UTC) } require.NoError(t, Execute(context.Background(), deps, []string{"pr", "view", "4", "-R", "alice/project", "--comments"})) - require.Equal(t, "title:\tFix\nstate:\topen\nauthor:\tbob\nlabels:\t\nreviewers:\t\nassignees:\t\nmilestone:\t\nmergeable:\tyes\ncomments:\t1\nnumber:\t4\nurl:\thttps://example.org/p/4\n--\n\n"+ + require.Equal(t, "title:\tFix\nstate:\topen\nauthor:\tbob\nbase:\tmain\nhead:\tfix\nlabels:\t\nassignees:\t\nreviewers:\talice (Approved), dave (Approved)\nmilestone:\t\nchecks:\t\nmergeable:\tyes\ncomments:\t1\nnumber:\t4\nurl:\thttps://example.org/p/4\n--\n\n"+ "--\nauthor:\talice\ncreated_at:\t2026-01-01T12:00:00Z\nreview:\tREQUEST_CHANGES\n--\nPlease rename\n"+ - "--\nauthor:\talice\ncreated_at:\t2026-01-01T12:00:00Z\npath:\tmain.go:12\n--\nTypo here\n"+ + "--\nauthor:\talice\ncreated_at:\t2026-01-01T12:00:00Z\nid:\t9\npath:\tmain.go:12\n--\nTypo here\n"+ "--\nauthor:\tcarol\ncreated_at:\t2026-01-02T00:00:00Z\n--\nLooks close\n"+ + "--\nauthor:\tdave\ncreated_at:\t2026-01-03T00:00:00Z\nreview:\tAPPROVED\n--\n\n"+ "--\nauthor:\talice\ncreated_at:\t2026-01-03T06:00:00Z\nreview:\tAPPROVED\n--\nGreat\n", out.String()) out.Reset() - deps.Streams.OutTTY = true - deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"}) + plainTTY(&deps, server.URL) require.NoError(t, Execute(context.Background(), deps, []string{"pr", "view", "4", "-R", "alice/project", "--comments"})) - require.Equal(t, "Fix #4\nOpen • bob wants to merge into main from fix • about 3 days ago • 1 comment\nMergeable: yes\n\nNo description provided\n\n"+ - "alice • reviewed (REQUEST_CHANGES) • about 2 days ago\nPlease rename\n\n"+ - "alice • commented on main.go:12 • about 2 days ago\nTypo here\n\n"+ + require.Equal(t, "Fix #4\nOpen • bob wants to merge into main from fix • about 3 days ago • 1 comment\nReviewers: alice (Approved), dave (Approved)\nMergeable: yes\n\nNo description provided\n\n"+ + "alice requested changes • about 2 days ago\nPlease rename\n\n"+ + "alice commented on main.go:12 • about 2 days ago\nTypo here\n\n"+ "carol • about 2 days ago\nLooks close\n\n"+ - "alice • reviewed (APPROVED) • about 18 hours ago\nGreat\n\n"+ + "dave approved these changes • about 1 day ago\n\n"+ + "alice approved these changes • about 18 hours ago\nGreat\n\n"+ "View this pull request on Gitea: https://example.org/p/4\n", out.String()) } + +func TestPullViewStates(t *testing.T) { + for _, tc := range []struct { + name, pull, statuses, reviews, want, decision string + }{ + { + name: "draft", + pull: `{"number":15,"title":"WIP: experiment","state":"open","draft":true,"mergeable":false,"user":{"login":"alice"},"base":{"ref":"main"},"head":{"ref":"experiment"}}`, + want: "WIP: experiment #15\nDraft • alice wants to merge into main from experiment • about 1 day ago • 0 comments\nMergeable: no — draft; mark it ready with: tea pr ready 15\n", + decision: "", + }, + { + name: "fork with checks and conflicts", + pull: `{"number":17,"title":"Improve greeting","state":"open","mergeable":false,"user":{"login":"bob"},"base":{"ref":"main"},"head":{"ref":"greet","sha":"abc","repo":{"full_name":"bob/project"}},"requested_reviewers":[{"login":"carol"}]}`, + statuses: `[{"id":1,"context":"ci / test","status":"pending"},{"id":2,"context":"lint","status":"failure"}]`, + reviews: `[{"id":4,"state":"REQUEST_CHANGES","user":{"login":"alice"}}]`, + want: "Improve greeting #17\nOpen • bob wants to merge into main from bob:greet • about 1 day ago • 0 comments\nReviewers: alice (Changes requested), carol (Requested)\nChecks: 1 failing, 1 pending (tea pr checks 17)\nMergeable: no — conflicts with main\n", + decision: "changes_requested", + }, + } { + t.Run(tc.name, func(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/api/v1/repos/alice/project/pulls/15" || r.URL.Path == "/api/v1/repos/alice/project/pulls/17": + _, _ = w.Write([]byte(tc.pull[:len(tc.pull)-1] + `,"created_at":"2026-01-03T00:00:00Z","html_url":"https://example.org/p"}`)) + case r.URL.Query().Get("page") != "1": + _, _ = w.Write([]byte(`[]`)) + case r.URL.Path == "/api/v1/repos/alice/project/statuses/abc": + _, _ = w.Write([]byte(tc.statuses)) + case tc.reviews == "": + _, _ = w.Write([]byte(`[]`)) + default: + _, _ = w.Write([]byte(tc.reviews)) + } + })) + defer server.Close() + deps, out, _ := serverDeps(t, server.URL) + deps.Now = func() time.Time { return time.Date(2026, 1, 4, 0, 0, 0, 0, time.UTC) } + plainTTY(&deps, server.URL) + number := map[string]string{"draft": "15"}[tc.name] + if number == "" { + number = "17" + } + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "view", number, "-R", "alice/project"})) + require.Equal(t, tc.want+"\nNo description provided\n\nView this pull request on Gitea: https://example.org/p\n", out.String()) + out.Reset() + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "view", number, "-R", "alice/project", "--json=draft,review_decision"})) + require.Contains(t, out.String(), `"review_decision": "`+tc.decision+`"`) + }) + } +} diff --git a/internal/app/human_output.go b/internal/app/human_output.go index 40556062..a9f46909 100644 --- a/internal/app/human_output.go +++ b/internal/app/human_output.go @@ -254,7 +254,6 @@ func writeItem[T any](a *application, cmd *cobra.Command, item T, row func(T) [] // column, keyed by command path without the root. Their owners replace an entry with a listSpec. var legacyColumns = map[string][]terminal.Column{ "issue list": numberedColumns, - "pr list": numberedColumns, } // numberedColumns head the rows of issueRow and pullRow. diff --git a/internal/app/human_output_test.go b/internal/app/human_output_test.go index a1d2c539..5e14edf4 100644 --- a/internal/app/human_output_test.go +++ b/internal/app/human_output_test.go @@ -224,7 +224,7 @@ func TestListsFitTheTerminalAndSplitWhenPiped(t *testing.T) { `{"number":5,"title":"支持中文路径 (support CJK paths)","state":"open","user":{"login":""}}]`)) })) defer server.Close() - for _, group := range []string{"issue", "pr"} { + for _, group := range []string{"issue"} { t.Run(group, func(t *testing.T) { deps, out, _ := serverDeps(t, server.URL) args := []string{group, "list", "-R", "alice/project"} diff --git a/internal/app/pull_view_current_test.go b/internal/app/pull_view_current_test.go index 43e9b89d..9e4420df 100644 --- a/internal/app/pull_view_current_test.go +++ b/internal/app/pull_view_current_test.go @@ -15,19 +15,27 @@ import ( func TestPullViewResolvesCurrentBranch(t *testing.T) { server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, "/api/v1/repos/alice/project/pulls", r.URL.Path) - require.Equal(t, "open", r.URL.Query().Get("state")) - if r.URL.Query().Get("page") != "1" { + switch { + case r.URL.Path == "/api/v1/repos/alice/project/pulls/9": + _, _ = fmt.Fprint(w, `{"number":9,"title":"Feature","state":"open","head":{"ref":"feature","sha":"abc"}}`) + case r.URL.Query().Get("page") != "1": + _, _ = fmt.Fprint(w, `[]`) + case r.URL.Path == "/api/v1/repos/alice/project/pulls": + require.Equal(t, "open", r.URL.Query().Get("state")) + _, _ = fmt.Fprint(w, `[{"number":9,"title":"Feature","html_url":"https://example.org/pr/9","state":"open","head":{"ref":"feature","sha":"abc"}}]`) + default: _, _ = fmt.Fprint(w, `[]`) - return } - _, _ = fmt.Fprint(w, `[{"number":9,"title":"Feature","html_url":"https://example.org/pr/9","state":"open","head":{"ref":"feature","sha":"abc"}}]`) })) defer server.Close() deps, out, _ := serverDeps(t, server.URL) + deps.Getenv = serverEnv(server.URL, map[string]string{"TEA_REPO": "alice/project"}) deps.Git = &fakeGit{results: map[string]string{"symbolic-ref --short HEAD": "feature"}} - require.NoError(t, Execute(context.Background(), deps, []string{"pr", "view", "-R", "alice/project", "--json=number,title"})) + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "view", "--json=number,title"})) require.JSONEq(t, `{"number":9,"title":"Feature"}`, out.String()) deps.Git = &fakeGit{results: map[string]string{}} - require.Equal(t, clierr.Usage, clierr.ExitCode(Execute(context.Background(), deps, []string{"pr", "view", "-R", "alice/project"}))) + require.Equal(t, clierr.Usage, clierr.ExitCode(Execute(context.Background(), deps, []string{"pr", "view"}))) + err := Execute(context.Background(), deps, []string{"pr", "view", "-R", "alice/project"}) + require.Equal(t, clierr.Usage, clierr.ExitCode(err)) + require.EqualError(t, err, "pass a pull request number, URL or branch with --repo") } diff --git a/internal/app/pulls.go b/internal/app/pulls.go index 83ba75a3..9441772c 100644 --- a/internal/app/pulls.go +++ b/internal/app/pulls.go @@ -4,113 +4,237 @@ package app import ( + "context" "errors" + "net/http" + "slices" "strconv" + "strings" "gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/services" "gitea.dev/tea/internal/terminal" + "gitea.dev/tea/internal/transport" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) func (a *application) prCommand() *cobra.Command { - group := &cobra.Command{Use: "pr", Short: "Work with pull requests"} + group := &cobra.Command{Use: "pr", Short: "Manage pull requests", Long: `Work with Gitea pull requests: list, view, create, review, check out and merge them. + +Commands that act on one pull request take a number, a URL or a branch name, and without +one select the pull request of the current branch.`} + group.Annotations = pullArgumentsAnnotation(nil) + group.Example = `$ tea pr list --author @me +$ tea pr create --fill +$ tea pr checkout 13 +$ tea pr review --approve +$ tea pr merge --squash --delete-branch` group.AddCommand(a.pullChecksCommand(), a.pullCheckoutCommand(), a.pullCleanCommand(), a.statusCommand(true)) - var limit, page int - var state string - var options services.PullListOptions - var labels []string - list := &cobra.Command{Use: "list", Short: "List pull requests", Args: argsExact(0), RunE: func(cmd *cobra.Command, _ []string) error { - if err := oneOf("state", state, "open", "closed", "all"); err != nil { - return err - } - if options.Milestone < 0 { - return clierr.Wrap(clierr.Usage, errors.New("--milestone must be a nonnegative ID")) - } - if options.Sort != "" { - if err := oneOf("sort", options.Sort, "oldest", "recentupdate", "recentclose", "leastupdate", "mostcomment", "leastcomment", "priority"); err != nil { - return err - } - } - target, err := a.resolve(cmd.Context(), true) - if err != nil { - return err - } - options.State, options.Limit, options.Page = state, limit, page - options.Labels, err = (services.Labels{Client: target.client, Repository: target.Repo}).ResolveIssueNames(cmd.Context(), labels) - if err != nil { - return err - } - items, more, err := (services.Pulls{Client: target.client, Repository: target.Repo}).List(cmd.Context(), options) - if err != nil { - return err - } - if err := writeList(a, cmd, items, pullRow); err != nil { - return err - } - return a.morePagesHint(cmd, more, page) - }} - list.Flags().StringVar(&state, "state", "open", "Filter state: open, closed, all") - pageFlags(list, &limit, &page) - list.Flags().StringVar(&options.Author, "author", "", "Filter by author username") - list.Flags().StringVar(&options.Base, "base", "", "Filter by target branch") - list.Flags().StringVar(&options.Sort, "sort", "", "Order: oldest, recentupdate, recentclose, leastupdate, mostcomment, leastcomment, priority") - list.Flags().Int64Var(&options.Milestone, "milestone", 0, "Filter by milestone ID") - list.Flags().StringSliceVar(&labels, "label", nil, "Filter by label names (repo: or org: resolves ambiguity)") - group.AddCommand(list) - var web, comments bool - view := &cobra.Command{Use: "view [number]", Short: "View a pull request", Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { - target, err := a.resolve(cmd.Context(), true) - if err != nil { - return err - } - service := services.Pulls{Client: target.client, Repository: target.Repo} - var item services.Pull - if len(args) == 1 { - n, err := number(args[0]) - if err != nil { - return err - } - item, err = service.View(cmd.Context(), n) - if err != nil { - return err - } - } else { - head, err := a.currentBranch(cmd.Context()) - if err != nil { - return clierr.Wrap(clierr.Usage, errors.New("supply a pull number outside a branch checkout")) - } - item, err = service.FindOpenByHead(cmd.Context(), head) - if err != nil { - return err - } - } - if web { - return a.openBrowser(cmd, item.URL) - } - if a.options.json != "" { - return a.writeJSON(cmd, item) - } - return a.writeDiscussion(cmd, target, pullView(item), comments) - }} - view.Flags().BoolVar(&web, "web", false, "Open the pull request in a browser") - view.Flags().BoolVarP(&comments, "comments", "c", false, "View pull request comments") - group.AddCommand(view) + group.AddCommand(a.pullListCommand(), a.pullViewCommand()) a.addPullMutations(group) group.AddCommand(a.pullEditCommand()) group.AddCommand(a.issueAttachmentCommand(), a.commentCommand()) return group } -func pullRow(item services.Pull) []terminal.Cell { - state := pullState(item) - return []terminal.Cell{{Text: strconv.FormatInt(item.Number, 10), Link: item.URL}, {Text: item.Title}, {Text: state, Color: stateColors[state]}, {Text: item.Author}} +func (a *application) pullListCommand() *cobra.Command { + var limit, page int + var state, milestone string + var options services.PullListOptions + var labels []string + var web bool + cmd := &cobra.Command{Use: "list", Short: "List pull requests", Long: `List the pull requests of a repository, newest first. + +The reviewers, reviewer_teams and merged_by JSON fields come from separate requests for +each pull request, which tea only makes when --json names them.`, Args: argsExact(0), RunE: func(cmd *cobra.Command, _ []string) error { + if id, err := strconv.ParseInt(milestone, 10, 64); err == nil && id < 1 { + return clierr.Wrap(clierr.Usage, errors.New("--milestone must be a milestone title or a positive ID")) + } + t, err := a.resolve(cmd.Context(), true) + if err != nil { + return err + } + if web { + link := repositoryURL(t) + "/pulls" + switch state { + case "closed", "merged": + link += "?state=closed" + case "all": + link += "?state=all" + } + return a.openBrowser(cmd, link) + } + options.Author, options.Assignee = t.expandMe(options.Author), t.expandMe(options.Assignee) + options.State, options.Limit, options.Page = state, limit, page + if options.Labels, err = (services.Labels{Client: t.client, Repository: t.Repo}).ResolveIssueNames(cmd.Context(), labels); err != nil { + return err + } + if milestone != "" { + if options.Milestone, err = milestoneFilter(cmd.Context(), t, milestone); err != nil { + return err + } + } + service := services.Pulls{Client: t.client, Repository: t.Repo} + window, err := service.ListPage(cmd.Context(), options) + if err != nil { + return err + } + items := window.Items + if a.jsonSelects("reviewers", "reviewer_teams", "merged_by") { + for i, item := range items { + if items[i], err = service.View(cmd.Context(), item.Number); err != nil { + return err + } + } + } + noun := map[string]string{"open": "open pull requests", "closed": "closed pull requests", "merged": "merged pull requests", "all": "pull requests"}[state] + what := noun + " in " + t.Repo + empty := "no " + what + if filtered(cmd, "state", "limit", "page", "sort") { + empty = "no " + noun + " match your filters in " + t.Repo + } + columns := []terminal.Column{{Header: "ID", Prefix: "#"}, {Header: "Title", Flex: true}, {Header: "Branch", Flex: true, Min: 12}, {Header: "State", PipedOnly: state == "open"}, {Header: "Author"}, {Header: "Created"}} + return writeTable(a, cmd, items, listSpec[services.Pull]{ + Columns: columns, + Row: func(item services.Pull) []terminal.Cell { + state := pullState(item) + return []terminal.Cell{ + {Text: strconv.FormatInt(item.Number, 10), Link: item.URL, Color: stateColors[state]}, + {Text: item.Title}, + {Text: pullHead(item, t.Repo)}, + {Text: state, Color: stateColors[state]}, + {Text: item.Author}, + {Time: terminal.ParseTime(item.CreatedAt)}, + } + }, + Empty: empty, Context: what, + }, listPage{Number: page, More: window.More, Total: window.Total, Counted: window.Counted}) + }} + enumFlag(cmd, &state, "state", "s", "open", "Filter by state", "open", "closed", "merged", "all") + pageFlags(cmd, &limit, &page) + cmd.Flags().StringVarP(&options.Author, "author", "A", "", "Filter by author; @me for yourself") + cmd.Flags().StringVarP(&options.Assignee, "assignee", "a", "", "Filter by assignee; @me for yourself") + cmd.Flags().StringVarP(&options.Base, "base", "B", "", "Filter by base branch") + cmd.Flags().StringVarP(&options.Head, "head", "H", "", "Filter by head branch, or OWNER:BRANCH for a fork") + cmd.Flags().BoolVarP(&options.Draft, "draft", "d", false, "Show only drafts") + cmd.Flags().StringVarP(&options.Search, "search", "S", "", "Filter by text in the title or body") + cmd.Flags().StringSliceVarP(&labels, "label", "l", nil, "Filter by labels; repo: or org: picks between same-named labels") + cmd.Flags().StringVarP(&milestone, "milestone", "m", "", "Filter by milestone title or ID") + enumFlag(cmd, &options.Sort, "sort", "", "", "Order", "oldest", "recentupdate", "recentclose", "leastupdate", "mostcomment", "leastcomment", "priority") + cmd.Flags().BoolVarP(&web, "web", "w", false, "Open the pull request list in the browser") + _ = cmd.RegisterFlagCompletionFunc("author", a.completeUsers()) + _ = cmd.RegisterFlagCompletionFunc("assignee", a.completeUsers()) + _ = cmd.RegisterFlagCompletionFunc("base", a.completeBranches()) + _ = cmd.RegisterFlagCompletionFunc("head", a.completeBranches()) + _ = cmd.RegisterFlagCompletionFunc("label", a.completeLabels()) + _ = cmd.RegisterFlagCompletionFunc("milestone", a.completeMilestones()) + cmd.Example = `$ tea pr list +$ tea pr list --author @me --state all +$ tea pr list -l bug -B main --draft +$ tea pr list --state merged -L 10 +$ tea pr list --json number,title,head --jq '.[] | select(.head == "feature/login")'` + return cmd } -// pullState reports merged pull requests as merged rather than closed. +// changedFlags lists the names of the command's own flags that were given. +func changedFlags(cmd *cobra.Command) []string { + var names []string + cmd.LocalNonPersistentFlags().VisitAll(func(flag *pflag.Flag) { + if flag.Changed { + names = append(names, flag.Name) + } + }) + return names +} + +// filtered reports whether a flag other than those named, which do not filter, was given. +func filtered(cmd *cobra.Command, unfiltered ...string) bool { + return slices.ContainsFunc(changedFlags(cmd), func(name string) bool { return !slices.Contains(unfiltered, name) }) +} + +// jsonSelects reports whether --json names one of fields, rather than selecting every field. +func (a *application) jsonSelects(fields ...string) bool { + for field := range strings.SplitSeq(a.options.json, ",") { + if slices.Contains(fields, strings.TrimSpace(field)) { + return true + } + } + return false +} + +// repositoryURL returns the web URL of the target's repository. +func repositoryURL(t target) string { + return strings.TrimSuffix(t.Host, "/") + "/" + t.Repo +} + +// milestoneFilter resolves a milestone given as an ID or a title (D10): a number is an ID first, +// then a title. +func milestoneFilter(ctx context.Context, t target, raw string) (int64, error) { + service := services.Milestones{Client: t.client, Repository: t.Repo} + if id, err := strconv.ParseInt(raw, 10, 64); err == nil && id > 0 { + _, err := service.View(ctx, id) + if err == nil { + return id, nil + } + if apiErr, ok := errors.AsType[*transport.APIError](err); !ok || apiErr.Status != http.StatusNotFound { + return 0, err + } + } + return service.ResolveName(ctx, raw) +} + +func (a *application) pullViewCommand() *cobra.Command { + var web, comments bool + cmd := &cobra.Command{Use: "view [ | | ]", Short: "View a pull request", Annotations: pullArgumentsAnnotation(nil), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { + t, n, err := a.pullSelector(cmd, args) + if err != nil { + return err + } + service := services.Pulls{Client: t.client, Repository: t.Repo} + item, err := service.View(cmd.Context(), n) + if err != nil { + return err + } + if web { + return a.openBrowser(cmd, item.URL) + } + var details pullDetails + if details.reviews, err = (services.Reviews{Client: t.client, Repository: t.Repo}).All(cmd.Context(), n); err != nil { + return err + } + item.ReviewDecision = services.ReviewDecision(services.LatestReviews(details.reviews), len(item.Reviewers)+len(item.ReviewerTeams) > 0) + if a.options.json != "" { + return a.writeJSON(cmd, item) + } + if item.State == "open" && item.HeadSHA != "" { + if details.checks, err = service.CommitChecks(cmd.Context(), item.HeadSHA); err != nil { + return err + } + } + return a.writeDiscussion(cmd, t, pullView(item, t.Repo, details), comments) + }} + cmd.Flags().BoolVarP(&web, "web", "w", false, "Open the pull request in the browser") + cmd.Flags().BoolVarP(&comments, "comments", "c", false, "Show comments and reviews") + cmd.ValidArgsFunction = a.completePulls("all") + cmd.Example = `# View the pull request of the current branch +$ tea pr view + +# View #13 with its comments and reviews +$ tea pr view 13 --comments + +$ tea pr view https://gitea.com/owner/repo/pulls/13 --web +$ tea pr view feature/login --json number,state,review_decision` + return cmd +} + +// pullState reports merged pull requests as merged, and open drafts as draft. func pullState(item services.Pull) string { - if item.Merged { + switch { + case item.Merged: return "merged" + case item.Draft && item.State == "open": + return "draft" } return item.State } diff --git a/internal/app/pulls_test.go b/internal/app/pulls_test.go index d06207d0..fb953823 100644 --- a/internal/app/pulls_test.go +++ b/internal/app/pulls_test.go @@ -6,7 +6,10 @@ package app import ( "context" "net/http" + "slices" + "strings" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -21,3 +24,80 @@ func TestPullListExplicitState(t *testing.T) { require.NoError(t, Execute(context.Background(), deps, []string{"pr", "list", "--state=closed", "-R", "alice/project", "--json"})) require.JSONEq(t, `[]`, out.String()) } + +// pullListServer lists the pull requests of alice/project: #17 from bob's fork, #15 a draft, #14 +// merged and #16 closed, with the total count Gitea reports. +func pullListServer(t *testing.T, states *[]string) *http.Server { + t.Helper() + pulls := map[string]string{ + "open": `[{"number":17,"title":"Improve greeting","state":"open","user":{"login":"bob"},"created_at":"2026-10-11T10:00:00Z","head":{"ref":"bob/greeting","repo":{"full_name":"bob/project"}},"html_url":"http://gitea/alice/project/pulls/17"}, + {"number":15,"title":"WIP: experiment","state":"open","draft":true,"user":{"login":"alice"},"created_at":"2026-10-11T09:00:00Z","head":{"ref":"experiment","repo":{"full_name":"alice/project"}}}]`, + "closed": `[{"number":16,"title":"Add usage guide","state":"closed","user":{"login":"alice"},"head":{"ref":"docs/guide"}}, + {"number":14,"title":"Fix README typo","state":"closed","merged":true,"user":{"login":"alice"},"head":{"ref":"fix/typo"}}]`, + } + return &http.Server{Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + state := r.URL.Query().Get("state") + *states = append(*states, state) + if r.URL.Query().Get("page") != "1" { + _, _ = w.Write([]byte(`[]`)) + return + } + w.Header().Set("X-Total-Count", "2") + _, _ = w.Write([]byte(pulls[state])) + })} +} + +func TestPullListTable(t *testing.T) { + var states []string + server := newAuthenticatedServer(pullListServer(t, &states).Handler) + defer server.Close() + deps, out, diagnostics := serverDeps(t, server.URL) + deps.Now = func() time.Time { return time.Date(2026, 10, 11, 12, 0, 0, 0, time.UTC) } + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "list", "-R", "alice/project"})) + require.Equal(t, "17\tImprove greeting\tbob:bob/greeting\topen\tbob\t2026-10-11T10:00:00Z\n15\tWIP: experiment\texperiment\tdraft\talice\t2026-10-11T09:00:00Z\n", out.String()) + + out.Reset() + plainTTY(&deps, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "list", "-R", "alice/project"})) + require.Equal(t, `Showing 2 of 2 open pull requests in alice/project + +ID TITLE BRANCH AUTHOR CREATED +#17 Improve greeting bob:bob/greeting bob about 2 hours ago +#15 WIP: experiment experiment alice about 3 hours ago +`, stripLinks(out.String())) + + out.Reset() + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "list", "-R", "alice/project", "--state", "merged"})) + require.Equal(t, "Showing 1 of 1 merged pull requests in alice/project\n\nID TITLE BRANCH STATE AUTHOR CREATED\n#14 Fix README typo fix/typo merged alice\n", stripLinks(out.String())) + + out.Reset() + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "list", "-R", "alice/project", "-d"})) + require.True(t, strings.HasPrefix(stripLinks(out.String()), "Showing 1 of 1 open pull requests in alice/project\n\nID TITLE BRANCH AUTHOR CREATED\n#15 WIP: experiment"), out.String()) + + out.Reset() + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "list", "-R", "alice/project", "-H", "alice:greeting"})) + require.Empty(t, out.String()) + require.Equal(t, "no open pull requests match your filters in alice/project\n", diagnostics.String()) + require.Equal(t, []string{"open", "closed", "open"}, slices.Compact(states)) +} + +func TestPullListFillsReviewersWhenSelected(t *testing.T) { + views := 0 + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v1/repos/alice/project/pulls/13" { + views++ + _, _ = w.Write([]byte(`{"number":13,"requested_reviewers":[{"login":"bob"}]}`)) + return + } + _, _ = w.Write([]byte(`[{"number":13,"requested_reviewers":[]}]`)) + })) + defer server.Close() + deps, out, _ := serverDeps(t, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "list", "-R", "alice/project", "--json", "number,draft"})) + require.JSONEq(t, `[{"number":13,"draft":false}]`, out.String()) + require.Zero(t, views) + out.Reset() + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "list", "-R", "alice/project", "--json", "number,reviewers"})) + require.JSONEq(t, `[{"number":13,"reviewers":["bob"]}]`, out.String()) + require.Equal(t, 1, views) +} diff --git a/internal/app/validation_test.go b/internal/app/validation_test.go index e0858133..f45e62ea 100644 --- a/internal/app/validation_test.go +++ b/internal/app/validation_test.go @@ -128,8 +128,8 @@ func TestUsageErrorsPrecedeRequests(t *testing.T) { {[]string{"pr", "edit", "7", "--add-reviewers=alice", "--remove-reviewers=Alice"}, "reviewer \"Alice\" is duplicated or present in both add and remove"}, {[]string{"pr", "edit", "7", "--add-reviewer-teams="}, "supply at least one reviewer or team"}, {[]string{"pr", "list", "--sort", "unknown"}, "--sort must be oldest, recentupdate, recentclose, leastupdate, mostcomment, leastcomment, or priority"}, - {[]string{"pr", "list", "--milestone", "-1"}, "--milestone must be a nonnegative ID"}, - {[]string{"pr", "list", "--state", "unknown"}, "--state must be open, closed, or all"}, + {[]string{"pr", "list", "--milestone", "-1"}, "--milestone must be a milestone title or a positive ID"}, + {[]string{"pr", "list", "--state", "unknown"}, "--state must be open, closed, merged, or all"}, {[]string{"pr", "list", "--limit", "0"}, `"-L, --limit" flag: must be 1..1000`}, {[]string{"pr", "review", "1", "--no-input"}, "choose a review type: --approve, --request-changes, or --comment"}, {[]string{"pr", "review", "1", "--comment", "--body="}, "a comment needs a body; pass --body, --body-file or --editor"}, diff --git a/internal/services/checks.go b/internal/services/checks.go index cb97cb51..12b7967e 100644 --- a/internal/services/checks.go +++ b/internal/services/checks.go @@ -6,6 +6,7 @@ package services import ( "context" "errors" + "net/url" "slices" "sort" "time" @@ -51,13 +52,32 @@ func (s Pulls) checkHead(ctx context.Context, n int64) (string, error) { return pull.Head.Sha, nil } -func checkFrom(sha string, status *gitea.Status) Check { +func checkFrom(sha, instance string, status *gitea.Status) Check { return Check{ SHA: sha, ID: status.ID, Name: status.Context, State: string(status.State), Description: status.Description, - URL: status.TargetURL, CreatedAt: timestamp(status.Created), UpdatedAt: timestamp(status.Updated), + URL: absoluteURL(instance, status.TargetURL), CreatedAt: timestamp(status.Created), UpdatedAt: timestamp(status.Updated), } } +// absoluteURL resolves a link that Gitea gives relative to its root, such as the job links of +// Actions statuses, against the instance URL. +func absoluteURL(instance, link string) string { + base, err := url.Parse(instance) + if err != nil || link == "" { + return link + } + ref, err := url.Parse(link) + if err != nil { + return link + } + return base.ResolveReference(ref).String() +} + +// CommitChecks returns the latest status of each check context of a commit. +func (s Pulls) CommitChecks(ctx context.Context, sha string) ([]Check, error) { + return s.checksForCommit(ctx, sha) +} + func (s Pulls) checksForCommit(ctx context.Context, sha string) ([]Check, error) { statuses, err := listAll(func(page int) ([]*gitea.Status, error) { return repoCall(ctx, s.Client, s.Repository, func(ctx context.Context, c *gitea.Client, owner, name string) ([]*gitea.Status, *gitea.Response, error) { @@ -71,7 +91,7 @@ func (s Pulls) checksForCommit(ctx context.Context, sha string) ([]Check, error) for _, status := range statuses { current, exists := latest[status.Context] if !exists || status.ID > current.ID { - latest[status.Context] = checkFrom(sha, status) + latest[status.Context] = checkFrom(sha, s.Client.BaseURL, status) } } result := make([]Check, 0, len(latest)) diff --git a/internal/services/checks_test.go b/internal/services/checks_test.go index 15e58ef8..0acea7cb 100644 --- a/internal/services/checks_test.go +++ b/internal/services/checks_test.go @@ -90,5 +90,16 @@ func TestCheckFromMapsCommitStatuses(t *testing.T) { require.Equal(t, Check{ SHA: "abc", ID: 3, Name: "ci/build", State: "failure", Description: "Broken", URL: "https://ci.test/3", CreatedAt: "2026-10-01T12:00:00Z", UpdatedAt: "2026-10-01T12:05:00Z", - }, checkFrom("abc", status)) + }, checkFrom("abc", "http://instance.test", status)) +} + +func TestCheckLinksAreAbsolute(t *testing.T) { + for _, tc := range []struct{ instance, link, want string }{ + {"http://instance.test", "/alice/x/actions/runs/18/jobs/18", "http://instance.test/alice/x/actions/runs/18/jobs/18"}, + {"https://instance.test/sub", "/sub/alice/x/actions/runs/1", "https://instance.test/sub/alice/x/actions/runs/1"}, + {"http://instance.test", "https://ci.test/3", "https://ci.test/3"}, + {"http://instance.test", "", ""}, + } { + require.Equal(t, tc.want, checkFrom("abc", tc.instance, &gitea.Status{TargetURL: tc.link}).URL, tc.link) + } } diff --git a/internal/services/pulls.go b/internal/services/pulls.go index 86a0f174..d3daefeb 100644 --- a/internal/services/pulls.go +++ b/internal/services/pulls.go @@ -11,6 +11,7 @@ import ( "io" "net/http" "net/url" + "slices" "strconv" "strings" "time" @@ -21,33 +22,40 @@ import ( // Pull is the stable command representation of a pull request. type Pull struct { - Author string `json:"author"` - CreatedAt string `json:"created_at"` - UpdatedAt string `json:"updated_at"` - Assignees []string `json:"assignees"` - Milestone *IssueMilestone `json:"milestone"` - Labels []Label `json:"labels"` - Deadline *string `json:"due_date"` - ID int64 `json:"id"` - Number int64 `json:"number"` - Title string `json:"title"` - Body string `json:"body"` - URL string `json:"url"` - State string `json:"state"` - Merged bool `json:"merged"` - Mergeable bool `json:"mergeable"` - Draft bool `json:"-"` - Base string `json:"base"` - Head string `json:"head"` - HeadSHA string `json:"head_sha"` - HeadRepo string `json:"head_repo,omitempty"` - HeadRemoteDeleted bool `json:"head_remote_deleted"` - HeadCanPush bool `json:"-"` - HeadCloneURL string `json:"-"` - HeadSSHURL string `json:"-"` - Reviewers []string `json:"reviewers"` - ReviewerTeams []string `json:"reviewer_teams"` - Comments int64 `json:"comments"` + Author string `json:"author"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` + Assignees []string `json:"assignees"` + Milestone *IssueMilestone `json:"milestone"` + Labels []Label `json:"labels"` + Deadline *string `json:"due_date"` + ID int64 `json:"id"` + Number int64 `json:"number"` + Title string `json:"title"` + Body string `json:"body"` + URL string `json:"url"` + State string `json:"state"` + Merged bool `json:"merged"` + MergedAt *string `json:"merged_at"` + // MergedBy, Reviewers and ReviewerTeams are only in responses for one pull request; lists + // leave them out. + MergedBy string `json:"merged_by"` + Mergeable bool `json:"mergeable"` + Draft bool `json:"draft"` + // ReviewDecision summarizes the reviews like gh: approved, changes_requested, review_required + // or "". Only commands that read the reviews fill it. + ReviewDecision string `json:"review_decision"` + Base string `json:"base"` + Head string `json:"head"` + HeadSHA string `json:"head_sha"` + HeadRepo string `json:"head_repo,omitempty"` + HeadRemoteDeleted bool `json:"head_remote_deleted"` + HeadCanPush bool `json:"-"` + HeadCloneURL string `json:"-"` + HeadSSHURL string `json:"-"` + Reviewers []string `json:"reviewers"` + ReviewerTeams []string `json:"reviewer_teams"` + Comments int64 `json:"comments"` } func pullHeadBranch(number int64, ref, label string) (branch string, deleted bool) { @@ -74,7 +82,8 @@ func pullFrom(pull *gitea.PullRequest) Pull { Author: login(pull.Poster), CreatedAt: timestamp(deref(pull.Created)), UpdatedAt: timestamp(deref(pull.Updated)), Assignees: convertAll(pull.Assignees, login), Milestone: issueMilestoneFrom(pull.Milestone), Labels: convertAll(pull.Labels, labelFrom), Deadline: optionalTimestamp(pull.Deadline), ID: pull.ID, Number: pull.Index, Title: pull.Title, Body: pull.Body, - URL: pull.HTMLURL, State: string(pull.State), Merged: pull.HasMerged, Mergeable: pull.Mergeable, Draft: pull.Draft, + URL: pull.HTMLURL, State: string(pull.State), Merged: pull.HasMerged, MergedAt: optionalTimestamp(pull.Merged), MergedBy: login(pull.MergedBy), + Mergeable: pull.Mergeable, Draft: pull.Draft, Base: base.Ref, Head: headBranch, HeadSHA: head.Sha, HeadRemoteDeleted: remoteDeleted, Reviewers: convertAll(pull.RequestedReviewers, login), Comments: int64(pull.Comments), ReviewerTeams: convertAll(pull.RequestedReviewersTeams, func(team *gitea.Team) string { return team.Name }), @@ -127,17 +136,71 @@ type Pulls struct { Repository string } -// PullListOptions supplies server-supported pull request filters. +// PullListOptions filters pull requests. State is open, closed, merged or all. type PullListOptions struct { State, Sort, Author, Base string Limit, Page int Milestone int64 Labels []int64 + // Draft, Head (BRANCH or OWNER:BRANCH), Assignee, Search (text in the title or body) and the + // merged state have no server filter; List applies them to every pull request it reads. + Draft bool + Head, Assignee, Search string +} + +// filtered reports whether a filter needs every pull request read. +func (o PullListOptions) filtered() bool { + return o.State == "merged" || o.Draft || o.Head != "" || o.Assignee != "" || o.Search != "" +} + +// matches applies the filters without a server equivalent to item. +func (o PullListOptions) matches(item Pull) bool { + owner, branch, qualified := strings.Cut(o.Head, ":") + if !qualified { + owner, branch = "", o.Head + } + headOwner, _, _ := strings.Cut(item.HeadRepo, "/") + search := strings.ToLower(o.Search) + return (o.State != "merged" || item.Merged) && (!o.Draft || item.Draft) && + (branch == "" || (item.Head == branch && (owner == "" || strings.EqualFold(owner, headOwner)))) && + (o.Assignee == "" || slices.ContainsFunc(item.Assignees, func(name string) bool { return strings.EqualFold(name, o.Assignee) })) && + (search == "" || strings.Contains(strings.ToLower(item.Title), search) || strings.Contains(strings.ToLower(item.Body), search)) } // List returns a page of pull requests, even when the server caps its page size below the // limit, and reports whether more pull requests follow. func (s Pulls) List(ctx context.Context, options PullListOptions) ([]Pull, bool, error) { + page, err := s.ListPage(ctx, options) + return page.Items, page.More, err +} + +// ListPage is List that also reports how many pull requests match, when the server counts them +// or every pull request was read for a filter without a server equivalent. +func (s Pulls) ListPage(ctx context.Context, options PullListOptions) (Page[Pull], error) { + if !options.filtered() { + return s.listPage(ctx, options) + } + query := options + if query.State == "merged" { + query.State = "closed" + } + all, err := listAll(func(page int) ([]Pull, error) { + query.Limit, query.Page = 50, page + window, err := s.listPage(ctx, query) + return window.Items, err + }) + if err != nil { + return Page[Pull]{}, err + } + matching := slices.DeleteFunc(all, func(item Pull) bool { return !options.matches(item) }) + items, err := pageOf(matching, options.Limit, options.Page) + if err != nil { + return Page[Pull]{}, err + } + return Page[Pull]{Items: items, More: options.Page*options.Limit < len(matching), Total: len(matching), Counted: true}, nil +} + +func (s Pulls) listPage(ctx context.Context, options PullListOptions) (Page[Pull], error) { query := url.Values{"state": {options.State}} if options.Sort != "" { query.Set("sort", options.Sort) @@ -156,11 +219,11 @@ func (s Pulls) List(ctx context.Context, options PullListOptions) ([]Pull, bool, } // TODO(sdk): ListPullRequestsOptions lacks the poster, base_branch, and labels filters. - pulls, more, err := rawWindow[*gitea.PullRequest](ctx, s.Client, "repos/"+s.Repository+"/pulls", query, options.Limit, options.Page) + page, err := rawPage[*gitea.PullRequest](ctx, s.Client, "repos/"+s.Repository+"/pulls", query, options.Limit, options.Page) if err != nil { - return nil, false, err + return Page[Pull]{}, err } - return convertAll(pulls, pullFrom), more, nil + return Page[Pull]{Items: convertAll(page.Items, pullFrom), More: page.More, Total: page.Total, Counted: page.Counted}, nil } // View retrieves one pull request. diff --git a/internal/services/reviews.go b/internal/services/reviews.go index a19c1fc6..89dbf751 100644 --- a/internal/services/reviews.go +++ b/internal/services/reviews.go @@ -4,10 +4,12 @@ package services import ( + "cmp" "context" "errors" "fmt" "net/http" + "slices" "sort" "strings" @@ -105,7 +107,7 @@ func (s Reviews) List(ctx context.Context, n int64, limit, page int) ([]Review, // Pending returns the reviews that login started on pull request n and has not submitted yet. func (s Reviews) Pending(ctx context.Context, n int64, login string) ([]Review, error) { - reviews, err := listAll(func(page int) ([]Review, error) { return s.List(ctx, n, 100, page) }) + reviews, err := s.All(ctx, n) if err != nil { return nil, err } @@ -208,14 +210,57 @@ func (s Reviews) Comments(ctx context.Context, n, reviewID int64, limit, page in return pageOf(results, limit, page) } -// Thread returns every review and every inline review comment of a pull request. -func (s Reviews) Thread(ctx context.Context, n int64) ([]Review, []ReviewComment, error) { - reviews, err := listAll(func(page int) ([]Review, error) { return s.List(ctx, n, 100, page) }) - if err != nil { - return nil, nil, err +// All returns every review of a pull request, including requests for a review. +func (s Reviews) All(ctx context.Context, n int64) ([]Review, error) { + return listAll(func(page int) ([]Review, error) { return s.List(ctx, n, 100, page) }) +} + +// Inline returns the inline comments of the given reviews of pull request n. +func (s Reviews) Inline(ctx context.Context, n int64, reviews []Review) ([]ReviewComment, error) { + return s.reviewComments(ctx, n, reviews, true) +} + +// LatestReviews returns each reviewer's latest review, in the order of their first review. Pending +// reviews, which only their author sees, are left out; a later request for a review replaces a review. +func LatestReviews(reviews []Review) []Review { + reviews = slices.Clone(reviews) + slices.SortStableFunc(reviews, func(x, y Review) int { return cmp.Compare(x.ID, y.ID) }) + at := map[string]int{} + var latest []Review + for _, review := range reviews { + if review.Author == "" || review.State == string(gitea.ReviewStatePending) { + continue + } + if i, ok := at[review.Author]; ok { + latest[i] = review + continue + } + at[review.Author] = len(latest) + latest = append(latest, review) } - comments, err := s.reviewComments(ctx, n, reviews, true) - return reviews, comments, err + return latest +} + +// ReviewDecision summarizes the latest reviews like gh: changes_requested when a reviewer's latest +// review requests changes, else approved when one approves, else review_required while reviews +// are requested, else "". Dismissed reviews do not count. +func ReviewDecision(latest []Review, requested bool) string { + decision := "" + for _, review := range latest { + switch { + case review.Dismissed: + case review.State == string(gitea.ReviewStateRequestChanges): + return "changes_requested" + case review.State == string(gitea.ReviewStateApproved): + decision = "approved" + case review.State == string(gitea.ReviewStateRequestReview): + requested = true + } + } + if decision == "" && requested { + return "review_required" + } + return decision } func (s Reviews) reviewComments(ctx context.Context, n int64, reviews []Review, skipEmpty bool) ([]ReviewComment, error) {