From ca84d4c6a3f80ef29fc253b914441b1315dd7000 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Fri, 17 Apr 2026 14:51:44 -0500 Subject: [PATCH] Add `discussion view --comments` with threaded display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement the --comments flag for discussion view, showing threaded comments with replies. Features: - --comments flag fetches and displays discussion comments - --order flag (oldest/newest) controls comment ordering - Answer badge (✓ Answer) on marked answer comments - Threaded replies with indentation - Truncation messages when more replies exist than fetched - TTY: markdown-rendered comments with author/timestamp/reactions - Non-TTY: stable tab-delimited format for scripting - JSON: populated comment nodes via ExportData Implementation: - GetWithComments uses raw GraphQL to dynamically switch between first/last based on ordering. Fetches 30 comments with 4 replies each. Explicitly reverses for newest-first ordering. - --order without --comments returns a flag error - Reuses existing shared.ReactionGroupList for reaction display Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 247 ++++++++++++++++++++++- pkg/cmd/discussion/view/view.go | 117 ++++++++++- pkg/cmd/discussion/view/view_test.go | 208 +++++++++++++++++++ 3 files changed, 567 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 9cba1f803..e50e85cad 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -396,8 +396,251 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc return &d, nil } -func (c *discussionClient) GetWithComments(_ ghrepo.Interface, _ int, _ int, _ string) (*Discussion, error) { - return nil, fmt.Errorf("not implemented") +func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { + // Build the comments field with first/last based on order. + // "oldest" uses first (chronological), "newest" uses last (reverse chronological). + commentDirection := "first" + if order == "newest" { + commentDirection = "last" + } + + query := fmt.Sprintf(`query DiscussionWithComments($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + hasDiscussionsEnabled + discussion(number: $number) { + id + number + title + body + url + closed + stateReason + author { login ... on User { id name } ... on Bot { id } } + category { id name slug emoji isAnswerable } + labels(first: 20) { nodes { id name color } } + isAnswered + answerChosenAt + answerChosenBy { login ... on User { id name } ... on Bot { id } } + reactionGroups { content users { totalCount } } + createdAt + updatedAt + closedAt + locked + comments(%s: %d) { + totalCount + nodes { + id + url + author { login ... on User { id name } ... on Bot { id } } + body + createdAt + isAnswer + upvoteCount + reactionGroups { content users { totalCount } } + replies(first: 4) { + totalCount + nodes { + id + url + author { login ... on User { id name } ... on Bot { id } } + body + createdAt + isAnswer + upvoteCount + reactionGroups { content users { totalCount } } + } + } + } + } + } + } + }`, commentDirection, commentLimit) + + variables := map[string]interface{}{ + "owner": repo.RepoOwner(), + "name": repo.RepoName(), + "number": number, + } + + type actorJSON struct { + Login string `json:"login"` + ID string `json:"id"` + Name string `json:"name"` + } + + type reactionGroupJSON struct { + Content string `json:"content"` + Users struct { + TotalCount int `json:"totalCount"` + } `json:"users"` + } + + type commentJSON struct { + ID string `json:"id"` + URL string `json:"url"` + Author actorJSON `json:"author"` + Body string `json:"body"` + CreatedAt time.Time `json:"createdAt"` + IsAnswer bool `json:"isAnswer"` + UpvoteCount int `json:"upvoteCount"` + ReactionGroups []reactionGroupJSON `json:"reactionGroups"` + Replies *struct { + TotalCount int `json:"totalCount"` + Nodes []commentJSON `json:"nodes"` + } `json:"replies"` + } + + type response struct { + Repository struct { + HasDiscussionsEnabled bool `json:"hasDiscussionsEnabled"` + Discussion *struct { + ID string `json:"id"` + Number int `json:"number"` + Title string `json:"title"` + Body string `json:"body"` + URL string `json:"url"` + Closed bool `json:"closed"` + StateReason string `json:"stateReason"` + Author actorJSON `json:"author"` + Category struct { + ID string `json:"id"` + Name string `json:"name"` + Slug string `json:"slug"` + Emoji string `json:"emoji"` + IsAnswerable bool `json:"isAnswerable"` + } `json:"category"` + Labels struct { + Nodes []struct { + ID string `json:"id"` + Name string `json:"name"` + Color string `json:"color"` + } `json:"nodes"` + } `json:"labels"` + IsAnswered bool `json:"isAnswered"` + AnswerChosenAt time.Time `json:"answerChosenAt"` + AnswerChosenBy *actorJSON `json:"answerChosenBy"` + ReactionGroups []reactionGroupJSON `json:"reactionGroups"` + CreatedAt time.Time `json:"createdAt"` + UpdatedAt time.Time `json:"updatedAt"` + ClosedAt time.Time `json:"closedAt"` + Locked bool `json:"locked"` + Comments struct { + TotalCount int `json:"totalCount"` + Nodes []commentJSON `json:"nodes"` + } `json:"comments"` + } `json:"discussion"` + } `json:"repository"` + } + + var data response + err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data) + if err != nil { + return nil, err + } + if !data.Repository.HasDiscussionsEnabled { + return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) + } + if data.Repository.Discussion == nil { + return nil, fmt.Errorf("discussion #%d not found in '%s/%s'", number, repo.RepoOwner(), repo.RepoName()) + } + + src := data.Repository.Discussion + + mapActor := func(a actorJSON) DiscussionActor { + return DiscussionActor{ID: a.ID, Login: a.Login, Name: a.Name} + } + + mapReactions := func(groups []reactionGroupJSON) []ReactionGroup { + out := make([]ReactionGroup, len(groups)) + for i, rg := range groups { + out[i] = ReactionGroup{Content: rg.Content, TotalCount: rg.Users.TotalCount} + } + return out + } + + mapComment := func(c commentJSON) DiscussionComment { + dc := DiscussionComment{ + ID: c.ID, + URL: c.URL, + Author: mapActor(c.Author), + Body: c.Body, + CreatedAt: c.CreatedAt, + IsAnswer: c.IsAnswer, + UpvoteCount: c.UpvoteCount, + ReactionGroups: mapReactions(c.ReactionGroups), + } + if c.Replies != nil { + dc.TotalReplies = c.Replies.TotalCount + for _, r := range c.Replies.Nodes { + dc.Replies = append(dc.Replies, DiscussionComment{ + ID: r.ID, + URL: r.URL, + Author: mapActor(r.Author), + Body: r.Body, + CreatedAt: r.CreatedAt, + IsAnswer: r.IsAnswer, + UpvoteCount: r.UpvoteCount, + ReactionGroups: mapReactions(r.ReactionGroups), + }) + } + } + return dc + } + + d := Discussion{ + ID: src.ID, + Number: src.Number, + Title: src.Title, + Body: src.Body, + URL: src.URL, + Closed: src.Closed, + StateReason: src.StateReason, + Author: mapActor(src.Author), + Category: DiscussionCategory{ + ID: src.Category.ID, + Name: src.Category.Name, + Slug: src.Category.Slug, + Emoji: src.Category.Emoji, + IsAnswerable: src.Category.IsAnswerable, + }, + Answered: src.IsAnswered, + AnswerChosenAt: src.AnswerChosenAt, + ReactionGroups: mapReactions(src.ReactionGroups), + CreatedAt: src.CreatedAt, + UpdatedAt: src.UpdatedAt, + ClosedAt: src.ClosedAt, + Locked: src.Locked, + } + + if src.AnswerChosenBy != nil { + a := mapActor(*src.AnswerChosenBy) + d.AnswerChosenBy = &a + } + + d.Labels = make([]DiscussionLabel, len(src.Labels.Nodes)) + for i, l := range src.Labels.Nodes { + d.Labels[i] = DiscussionLabel{ID: l.ID, Name: l.Name, Color: l.Color} + } + + comments := make([]DiscussionComment, len(src.Comments.Nodes)) + for i, c := range src.Comments.Nodes { + comments[i] = mapComment(c) + } + + // When using "last" (newest order), the API returns items in chronological + // order. Reverse them so the newest comment appears first. + if order == "newest" { + for i, j := 0, len(comments)-1; i < j; i, j = i+1, j-1 { + comments[i], comments[j] = comments[j], comments[i] + } + } + + d.Comments = DiscussionCommentList{ + Comments: comments, + TotalCount: src.Comments.TotalCount, + } + + return &d, nil } func (c *discussionClient) ListCategories(repo ghrepo.Interface) ([]DiscussionCategory, error) { diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index c7942d432..782311e59 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -28,6 +28,8 @@ type ViewOptions struct { DiscussionNumber int WebMode bool + Comments bool + Order string Exporter cmdutil.Exporter Now func() time.Time } @@ -46,6 +48,9 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman Long: heredoc.Docf(` Display the title, body, and other information about a discussion. + With %[1]s--comments%[1]s flag, show threaded comments on the discussion. + Use %[1]s--order%[1]s to control comment ordering (oldest or newest first). + With %[1]s--web%[1]s flag, open the discussion in a web browser instead. `, "`"), Example: heredoc.Doc(` @@ -55,11 +60,21 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman # View a discussion by URL $ gh discussion view https://github.com/OWNER/REPO/discussions/123 + # View with comments + $ gh discussion view 123 --comments + + # View with newest comments first + $ gh discussion view 123 --comments --order newest + # Open in browser $ gh discussion view 123 --web `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if cmd.Flags().Changed("order") && !opts.Comments { + return cmdutil.FlagErrorf("--order requires --comments") + } + number, repo, err := shared.ParseDiscussionArg(args[0]) if err != nil { return err @@ -84,6 +99,8 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open a discussion in the browser") + cmd.Flags().BoolVarP(&opts.Comments, "comments", "c", false, "View discussion comments") + cmdutil.StringEnumFlag(cmd, &opts.Order, "order", "", "oldest", []string{"oldest", "newest"}, "Order of comments") cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.DiscussionFields) return cmd @@ -111,7 +128,12 @@ func viewRun(opts *ViewOptions) error { opts.IO.DetectTerminalTheme() opts.IO.StartProgressIndicator() - discussion, err := c.GetByNumber(repo, opts.DiscussionNumber) + var discussion *client.Discussion + if opts.Comments { + discussion, err = c.GetWithComments(repo, opts.DiscussionNumber, 30, opts.Order) + } else { + discussion, err = c.GetByNumber(repo, opts.DiscussionNumber) + } opts.IO.StopProgressIndicator() @@ -132,7 +154,7 @@ func viewRun(opts *ViewOptions) error { return printHumanView(opts, discussion) } - return printRawView(opts.IO.Out, discussion) + return printRawView(opts.IO.Out, discussion, opts.Comments) } func printHumanView(opts *ViewOptions, d *client.Discussion) error { @@ -192,12 +214,29 @@ func printHumanView(opts *ViewOptions, d *client.Discussion) error { fmt.Fprintln(out) } + // Comments section + if opts.Comments && d.Comments.TotalCount > 0 { + fmt.Fprintln(out, cs.Bold("Comments")) + fmt.Fprintln(out) + + for _, c := range d.Comments.Comments { + if err := printHumanComment(opts, out, c, ""); err != nil { + return err + } + } + + if shown := len(d.Comments.Comments); shown < d.Comments.TotalCount { + fmt.Fprintf(out, cs.Muted(" And %d more comments\n"), d.Comments.TotalCount-shown) + fmt.Fprintln(out) + } + } + fmt.Fprintf(out, cs.Muted("View this discussion on GitHub: %s\n"), d.URL) return nil } -func printRawView(out io.Writer, d *client.Discussion) error { +func printRawView(out io.Writer, d *client.Discussion, showComments bool) error { fmt.Fprintf(out, "title:\t%s\n", d.Title) state := "OPEN" if d.Closed { @@ -212,9 +251,81 @@ func printRawView(out io.Writer, d *client.Discussion) error { fmt.Fprintf(out, "url:\t%s\n", d.URL) fmt.Fprintln(out, "--") fmt.Fprintln(out, d.Body) + + if showComments { + for _, c := range d.Comments.Comments { + printRawComment(out, c, "") + } + } + return nil } +func printHumanComment(opts *ViewOptions, out io.Writer, c client.DiscussionComment, indent string) error { + cs := opts.IO.ColorScheme() + now := opts.Now() + + header := fmt.Sprintf("%s%s commented %s", + indent, + cs.Bold(c.Author.Login), + text.FuzzyAgo(now, c.CreatedAt), + ) + if c.IsAnswer { + header += " " + cs.Green("✓ Answer") + } + fmt.Fprintln(out, header) + + if c.Body != "" { + md, err := markdown.Render(c.Body, + markdown.WithTheme(opts.IO.TerminalTheme()), + markdown.WithWrap(opts.IO.TerminalWidth())) + if err != nil { + return err + } + if indent != "" { + md = text.Indent(md, indent) + } + fmt.Fprint(out, md) + } + + if reactions := shared.ReactionGroupList(c.ReactionGroups); reactions != "" { + fmt.Fprintf(out, "%s%s\n", indent, reactions) + } + + fmt.Fprintln(out) + + for _, reply := range c.Replies { + if err := printHumanComment(opts, out, reply, indent+" "); err != nil { + return err + } + } + + if shown := len(c.Replies); shown < c.TotalReplies { + fmt.Fprintf(out, "%s %s\n\n", indent, cs.Muted(fmt.Sprintf("And %d more replies", c.TotalReplies-shown))) + } + + return nil +} + +func printRawComment(out io.Writer, c client.DiscussionComment, indent string) { + answer := "" + if c.IsAnswer { + answer = "\tanswer" + } + fmt.Fprintf(out, "%scomment:\t%s\t%s\t%s%s\n", indent, c.Author.Login, c.CreatedAt.Format(time.RFC3339), c.URL, answer) + fmt.Fprintf(out, "%s--\n", indent) + if indent != "" { + fmt.Fprint(out, text.Indent(c.Body, indent)) + } else { + fmt.Fprint(out, c.Body) + } + fmt.Fprintln(out) + + for _, reply := range c.Replies { + printRawComment(out, reply, indent+" ") + } +} + func labelList(labels []client.DiscussionLabel, cs *iostreams.ColorScheme) string { if len(labels) == 0 { return "" diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go index 1299f2f64..6e90d131c 100644 --- a/pkg/cmd/discussion/view/view_test.go +++ b/pkg/cmd/discussion/view/view_test.go @@ -351,3 +351,211 @@ func TestViewRun_notAnswerable(t *testing.T) { assert.Contains(t, out, "Started by") assert.NotContains(t, out, "Asked by") } + +func testDiscussionWithComments() *client.Discussion { + d := testDiscussion() + d.Comments = client.DiscussionCommentList{ + TotalCount: 2, + Comments: []client.DiscussionComment{ + { + ID: "C_1", + URL: "https://github.com/OWNER/REPO/discussions/123#discussioncomment-1", + Author: client.DiscussionActor{Login: "octocat"}, + Body: "This is a comment", + CreatedAt: time.Date(2025, 3, 2, 0, 0, 0, 0, time.UTC), + IsAnswer: true, + ReactionGroups: []client.ReactionGroup{ + {Content: "THUMBS_UP", TotalCount: 3}, + }, + Replies: []client.DiscussionComment{ + { + ID: "C_1_R1", + URL: "https://github.com/OWNER/REPO/discussions/123#discussioncomment-2", + Author: client.DiscussionActor{Login: "hubot"}, + Body: "Thanks!", + CreatedAt: time.Date(2025, 3, 2, 1, 0, 0, 0, time.UTC), + }, + }, + TotalReplies: 5, + }, + { + ID: "C_2", + URL: "https://github.com/OWNER/REPO/discussions/123#discussioncomment-3", + Author: client.DiscussionActor{Login: "monalisa"}, + Body: "Another comment", + CreatedAt: time.Date(2025, 3, 3, 0, 0, 0, 0, time.UTC), + }, + }, + } + return d +} + +func TestViewRun_comments_tty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + d := testDiscussionWithComments() + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + assert.Equal(t, 30, commentLimit) + assert.Equal(t, "oldest", order) + return d, nil + }, + } + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Comments: true, + Order: "oldest", + Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "Comments") + assert.Contains(t, out, "octocat") + assert.Contains(t, out, "✓ Answer") + assert.Contains(t, out, "This is a comment") + assert.Contains(t, out, "hubot") + assert.Contains(t, out, "Thanks!") + assert.Contains(t, out, "And 4 more replies") + assert.Contains(t, out, "monalisa") + assert.Contains(t, out, "Another comment") +} + +func TestViewRun_comments_nontty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussionWithComments() + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + return d, nil + }, + } + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Comments: true, + Order: "oldest", + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "comment:\toctocat\t") + assert.Contains(t, out, "answer") + assert.Contains(t, out, "This is a comment") + assert.Contains(t, out, "comment:\thubot\t") + assert.Contains(t, out, "comment:\tmonalisa\t") +} + +func TestViewRun_comments_json(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussionWithComments() + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + return d, nil + }, + } + + exporter := cmdutil.NewJSONExporter() + exporter.SetFields(shared.DiscussionFields) + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Comments: true, + Order: "oldest", + Exporter: exporter, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, `"totalCount"`) + assert.Contains(t, out, `"isAnswer":true`) + assert.Contains(t, out, `"octocat"`) +} + +func TestNewCmdView_orderWithoutComments(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + } + f.Browser = &browser.Stub{} + + cmd := NewCmdView(f, func(opts *ViewOptions) error { + return nil + }) + + cmd.SetArgs([]string{"123", "--order", "newest"}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "--order requires --comments") +} + +func TestViewRun_noComments_usesGetByNumber(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussion() + mock := &client.DiscussionClientMock{ + GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { + return d, nil + }, + } + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Comments: false, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + assert.Equal(t, 1, len(mock.GetByNumberCalls())) + assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) +}