From d9e97518231adeb82b36e3188d51440519651160 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Fri, 3 Apr 2026 13:16:12 -0500 Subject: [PATCH 01/17] Add `discussion view` command Implement `gh discussion view` for viewing a single discussion with: - Number or URL argument via shared.ParseDiscussionArg - TTY output: title, metadata (state, category, author, age, comment count), labels, markdown-rendered body, reactions - Context-aware author attribution: "Asked by" for answerable categories (Q&A), "Started by" for others - Non-TTY output: key-value pairs matching `gh issue view` format - JSON output via Exporter (Discussion.ExportData) - --web flag to open in browser - Pager support for TTY output Also adds: - GetByNumber client method with not-found detection - shared.ParseDiscussionArg for number/URL/#number parsing - shared.ReactionGroupList for emoji reaction display Comment threading (--comments) is deferred to the next PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 49 +++- pkg/cmd/discussion/discussion.go | 5 + pkg/cmd/discussion/shared/display.go | 35 +++ pkg/cmd/discussion/shared/lookup.go | 40 +++ pkg/cmd/discussion/view/view.go | 232 +++++++++++++++ pkg/cmd/discussion/view/view_test.go | 353 +++++++++++++++++++++++ 6 files changed, 712 insertions(+), 2 deletions(-) create mode 100644 pkg/cmd/discussion/shared/display.go create mode 100644 pkg/cmd/discussion/shared/lookup.go create mode 100644 pkg/cmd/discussion/view/view.go create mode 100644 pkg/cmd/discussion/view/view_test.go diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index d3f8e817b..ed5536fac 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -351,8 +351,53 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, return &result, nil } -func (c *discussionClient) GetByNumber(_ ghrepo.Interface, _ int) (*Discussion, error) { - return nil, fmt.Errorf("not implemented") +func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Discussion, error) { + query := fmt.Sprintf(`query DiscussionByNumber($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + hasDiscussionsEnabled + discussion(number: $number) { + %s + body + comments { totalCount } + } + } + }`, discussionFields) + + variables := map[string]interface{}{ + "owner": repo.RepoOwner(), + "name": repo.RepoName(), + "number": number, + } + + type response struct { + Repository struct { + HasDiscussionsEnabled bool `json:"hasDiscussionsEnabled"` + Discussion *struct { + discussionNode + Body string `json:"body"` + Comments struct { + TotalCount int `json:"totalCount"` + } `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()) + } + + d := mapDiscussion(data.Repository.Discussion.discussionNode) + d.Body = data.Repository.Discussion.Body + d.Comments = DiscussionCommentList{TotalCount: data.Repository.Discussion.Comments.TotalCount} + return &d, nil } func (c *discussionClient) GetWithComments(_ ghrepo.Interface, _ int, _ int, _ string) (*Discussion, error) { diff --git a/pkg/cmd/discussion/discussion.go b/pkg/cmd/discussion/discussion.go index a9bf9b981..a54763895 100644 --- a/pkg/cmd/discussion/discussion.go +++ b/pkg/cmd/discussion/discussion.go @@ -3,6 +3,7 @@ package discussion import ( "github.com/MakeNowJust/heredoc" cmdList "github.com/cli/cli/v2/pkg/cmd/discussion/list" + cmdView "github.com/cli/cli/v2/pkg/cmd/discussion/view" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -36,5 +37,9 @@ func NewCmdDiscussion(f *cmdutil.Factory) *cobra.Command { cmdList.NewCmdList(f, nil), ) + cmdutil.AddGroup(cmd, "Targeted commands", + cmdView.NewCmdView(f, nil), + ) + return cmd } diff --git a/pkg/cmd/discussion/shared/display.go b/pkg/cmd/discussion/shared/display.go new file mode 100644 index 000000000..ea866fcb2 --- /dev/null +++ b/pkg/cmd/discussion/shared/display.go @@ -0,0 +1,35 @@ +package shared + +import ( + "fmt" + "strings" + + "github.com/cli/cli/v2/pkg/cmd/discussion/client" +) + +var reactionEmoji = map[string]string{ + "THUMBS_UP": "\U0001f44d", + "THUMBS_DOWN": "\U0001f44e", + "LAUGH": "\U0001f604", + "HOORAY": "\U0001f389", + "CONFUSED": "\U0001f615", + "HEART": "\u2764\ufe0f", + "ROCKET": "\U0001f680", + "EYES": "\U0001f440", +} + +// ReactionGroupList formats reaction groups for display. +func ReactionGroupList(groups []client.ReactionGroup) string { + var parts []string + for _, g := range groups { + if g.TotalCount == 0 { + continue + } + emoji := reactionEmoji[g.Content] + if emoji == "" { + emoji = g.Content + } + parts = append(parts, fmt.Sprintf("%s %d", emoji, g.TotalCount)) + } + return strings.Join(parts, " • ") +} diff --git a/pkg/cmd/discussion/shared/lookup.go b/pkg/cmd/discussion/shared/lookup.go new file mode 100644 index 000000000..1196dd477 --- /dev/null +++ b/pkg/cmd/discussion/shared/lookup.go @@ -0,0 +1,40 @@ +package shared + +import ( + "fmt" + "net/url" + "regexp" + "strconv" + + "github.com/cli/cli/v2/internal/ghrepo" +) + +var discussionURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/discussions/(\d+)`) + +// ParseDiscussionArg parses a discussion number or URL from a command argument. +// It returns the discussion number and, if the argument was a URL, a repo override. +func ParseDiscussionArg(arg string) (int, ghrepo.Interface, error) { + if num, err := strconv.Atoi(arg); err == nil { + return num, nil, nil + } + + if len(arg) > 1 && arg[0] == '#' { + if num, err := strconv.Atoi(arg[1:]); err == nil { + return num, nil, nil + } + } + + u, err := url.Parse(arg) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") { + return 0, nil, fmt.Errorf("invalid discussion argument: %s", arg) + } + + m := discussionURLRE.FindStringSubmatch(u.Path) + if m == nil { + return 0, nil, fmt.Errorf("invalid discussion URL: %s", arg) + } + + num, _ := strconv.Atoi(m[3]) + repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname()) + return num, repo, nil +} diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go new file mode 100644 index 000000000..788b7d766 --- /dev/null +++ b/pkg/cmd/discussion/view/view.go @@ -0,0 +1,232 @@ +package view + +import ( + "fmt" + "io" + "sort" + "strings" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmd/discussion/client" + "github.com/cli/cli/v2/pkg/cmd/discussion/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/pkg/markdown" + "github.com/spf13/cobra" +) + +// ViewOptions holds the configuration for the view command. +type ViewOptions struct { + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Browser browser.Browser + Client func() (client.DiscussionClient, error) + + DiscussionNumber int + WebMode bool + Exporter cmdutil.Exporter + Now func() time.Time +} + +// NewCmdView creates the "discussion view" command. +func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { + opts := &ViewOptions{ + IO: f.IOStreams, + Browser: f.Browser, + Now: time.Now, + } + + cmd := &cobra.Command{ + Use: "view { | }", + Short: "View a discussion", + Long: heredoc.Docf(` + Display the title, body, and other information about a discussion. + + With %[1]s--web%[1]s flag, open the discussion in a web browser instead. + `, "`"), + Example: heredoc.Doc(` + # View a discussion by number + $ gh discussion view 123 + + # View a discussion by URL + $ gh discussion view https://github.com/OWNER/REPO/discussions/123 + + # Open in browser + $ gh discussion view 123 --web + `), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + number, repo, err := shared.ParseDiscussionArg(args[0]) + if err != nil { + return err + } + + if repo != nil { + opts.BaseRepo = func() (ghrepo.Interface, error) { + return repo, nil + } + } else { + opts.BaseRepo = f.BaseRepo + } + + opts.DiscussionNumber = number + opts.Client = shared.DiscussionClientFunc(f) + + if runF != nil { + return runF(opts) + } + return viewRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open a discussion in the browser") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.DiscussionFields) + + return cmd +} + +func viewRun(opts *ViewOptions) error { + repo, err := opts.BaseRepo() + if err != nil { + return err + } + + if opts.WebMode { + openURL := ghrepo.GenerateRepoURL(repo, "discussions/%d", opts.DiscussionNumber) + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(openURL)) + } + return opts.Browser.Browse(openURL) + } + + c, err := opts.Client() + if err != nil { + return err + } + + opts.IO.DetectTerminalTheme() + opts.IO.StartProgressIndicator() + + discussion, err := c.GetByNumber(repo, opts.DiscussionNumber) + + opts.IO.StopProgressIndicator() + + if err != nil { + return err + } + + if opts.Exporter != nil { + return opts.Exporter.Write(opts.IO, discussion) + } + + if err := opts.IO.StartPager(); err != nil { + fmt.Fprintf(opts.IO.ErrOut, "error starting pager: %v\n", err) + } + defer opts.IO.StopPager() + + if opts.IO.IsStdoutTTY() { + return printHumanView(opts, discussion) + } + + return printRawView(opts.IO.Out, discussion) +} + +func printHumanView(opts *ViewOptions, d *client.Discussion) error { + out := opts.IO.Out + cs := opts.IO.ColorScheme() + + numberStr := fmt.Sprintf("#%d", d.Number) + if d.State == "OPEN" { + numberStr = cs.Green(numberStr) + } else { + numberStr = cs.Muted(numberStr) + } + fmt.Fprintf(out, "%s %s\n", cs.Bold(d.Title), numberStr) + + state := "Open" + stateColor := cs.Green + if d.State != "OPEN" { + state = "Closed" + stateColor = cs.Muted + } + + verb := "Started by" + if d.Category.IsAnswerable { + verb = "Asked by" + } + + fmt.Fprintf(out, "%s · %s · %s %s · %s · %s\n", + stateColor(state), + d.Category.Name, + verb, + d.Author.Login, + text.FuzzyAgo(opts.Now(), d.CreatedAt), + text.Pluralize(d.Comments.TotalCount, "comment"), + ) + + if labels := labelList(d.Labels, cs); labels != "" { + fmt.Fprint(out, cs.Bold("Labels: ")) + fmt.Fprintln(out, labels) + } + + var md string + if d.Body == "" { + md = fmt.Sprintf("\n %s\n\n", cs.Muted("No description provided")) + } else { + var err error + md, err = markdown.Render(d.Body, + markdown.WithTheme(opts.IO.TerminalTheme()), + markdown.WithWrap(opts.IO.TerminalWidth())) + if err != nil { + return err + } + } + fmt.Fprintf(out, "\n%s\n", md) + + if reactions := shared.ReactionGroupList(d.ReactionGroups); reactions != "" { + fmt.Fprintln(out, reactions) + 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 { + fmt.Fprintf(out, "title:\t%s\n", d.Title) + fmt.Fprintf(out, "state:\t%s\n", d.State) + fmt.Fprintf(out, "category:\t%s\n", d.Category.Name) + fmt.Fprintf(out, "author:\t%s\n", d.Author.Login) + fmt.Fprintf(out, "labels:\t%s\n", labelList(d.Labels, nil)) + fmt.Fprintf(out, "comments:\t%d\n", d.Comments.TotalCount) + fmt.Fprintf(out, "number:\t%d\n", d.Number) + fmt.Fprintf(out, "url:\t%s\n", d.URL) + fmt.Fprintln(out, "--") + fmt.Fprintln(out, d.Body) + return nil +} + +func labelList(labels []client.DiscussionLabel, cs *iostreams.ColorScheme) string { + if len(labels) == 0 { + return "" + } + + sort.SliceStable(labels, func(i, j int) bool { + return strings.ToLower(labels[i].Name) < strings.ToLower(labels[j].Name) + }) + + names := make([]string, len(labels)) + for i, l := range labels { + if cs == nil { + names[i] = l.Name + } else { + names[i] = cs.Label(l.Color, l.Name) + } + } + return strings.Join(names, ", ") +} diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go new file mode 100644 index 000000000..9694fcaa8 --- /dev/null +++ b/pkg/cmd/discussion/view/view_test.go @@ -0,0 +1,353 @@ +package view + +import ( + "bytes" + "testing" + "time" + + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/discussion/client" + "github.com/cli/cli/v2/pkg/cmd/discussion/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func testDiscussion() *client.Discussion { + return &client.Discussion{ + ID: "D_123", + Number: 123, + Title: "How to authenticate with SSO?", + Body: "I need help with SSO authentication.", + URL: "https://github.com/OWNER/REPO/discussions/123", + State: "OPEN", + Author: client.DiscussionAuthor{Login: "monalisa"}, + Category: client.DiscussionCategory{ + Name: "Q&A", Slug: "q-a", IsAnswerable: true, + }, + Labels: []client.DiscussionLabel{{Name: "help-wanted", Color: "0075ca"}}, + Answered: false, + Comments: client.DiscussionCommentList{TotalCount: 3}, + ReactionGroups: []client.ReactionGroup{ + {Content: "THUMBS_UP", TotalCount: 5}, + {Content: "ROCKET", TotalCount: 2}, + }, + CreatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), + } +} + +func TestNewCmdView(t *testing.T) { + tests := []struct { + name string + args []string + wantNum int + wantErr string + }{ + { + name: "number argument", + args: []string{"123"}, + wantNum: 123, + }, + { + name: "hash number argument", + args: []string{"#456"}, + wantNum: 456, + }, + { + name: "URL argument", + args: []string{"https://github.com/OWNER/REPO/discussions/789"}, + wantNum: 789, + }, + { + name: "invalid argument", + args: []string{"not-a-number"}, + wantErr: "invalid discussion argument", + }, + { + name: "no arguments", + args: []string{}, + wantErr: "accepts 1 arg(s), received 0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(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{} + + var gotOpts *ViewOptions + cmd := NewCmdView(f, func(opts *ViewOptions) error { + gotOpts = opts + return nil + }) + + cmd.SetArgs(tt.args) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantNum, gotOpts.DiscussionNumber) + }) + } +} + +func TestViewRun_tty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + 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, + Now: func() time.Time { return time.Date(2025, 3, 1, 1, 0, 0, 0, time.UTC) }, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "How to authenticate with SSO?") + assert.Contains(t, out, "#123") + assert.Contains(t, out, "Q&A") + assert.Contains(t, out, "Asked by") + assert.Contains(t, out, "monalisa") + assert.Contains(t, out, "3 comments") + assert.Contains(t, out, "help-wanted") + assert.Contains(t, out, "View this discussion on GitHub") +} + +func TestViewRun_nontty(t *testing.T) { + ios, _, stdout, _ := 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, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "title:\tHow to authenticate with SSO?") + assert.Contains(t, out, "state:\tOPEN") + assert.Contains(t, out, "category:\tQ&A") + assert.Contains(t, out, "author:\tmonalisa") + assert.Contains(t, out, "labels:\thelp-wanted") + assert.Contains(t, out, "number:\t123") + assert.Contains(t, out, "--") + assert.Contains(t, out, "I need help with SSO authentication.") +} + +func TestViewRun_json(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussion() + mock := &client.DiscussionClientMock{ + GetByNumberFunc: func(repo ghrepo.Interface, number int) (*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, + Exporter: exporter, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, `"title"`) + assert.Contains(t, out, `"number"`) + assert.Contains(t, out, "How to authenticate with SSO?") +} + +func TestViewRun_web(t *testing.T) { + ios, _, _, stderr := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + b := &browser.Stub{} + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Browser: b, + DiscussionNumber: 123, + WebMode: true, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + b.Verify(t, "https://github.com/OWNER/REPO/discussions/123") + assert.Contains(t, stderr.String(), "Opening") +} + +func TestViewRun_urlArg(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussion() + d.URL = "https://github.com/OTHER/REPO/discussions/42" + d.Number = 42 + + mock := &client.DiscussionClientMock{ + GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { + assert.Equal(t, "OTHER", repo.RepoOwner()) + assert.Equal(t, "REPO", repo.RepoName()) + assert.Equal(t, 42, number) + return d, nil + }, + } + + f := &cmdutil.Factory{} + f.IOStreams = ios + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + } + f.Browser = &browser.Stub{} + + var gotOpts *ViewOptions + cmd := NewCmdView(f, func(opts *ViewOptions) error { + gotOpts = opts + opts.Client = func() (client.DiscussionClient, error) { + return mock, nil + } + return viewRun(opts) + }) + + cmd.SetArgs([]string{"https://github.com/OTHER/REPO/discussions/42"}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + require.NoError(t, err) + assert.Equal(t, 42, gotOpts.DiscussionNumber) + + out := stdout.String() + assert.Contains(t, out, "number:\t42") +} + +func TestViewRun_answerable(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + d := testDiscussion() + d.Category.IsAnswerable = true + + 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, + Now: func() time.Time { return time.Date(2025, 3, 1, 1, 0, 0, 0, time.UTC) }, + } + + err := viewRun(opts) + require.NoError(t, err) + assert.Contains(t, stdout.String(), "Asked by") +} + +func TestViewRun_notAnswerable(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + d := testDiscussion() + d.Category.Name = "General" + d.Category.IsAnswerable = false + + 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, + Now: func() time.Time { return time.Date(2025, 3, 1, 1, 0, 0, 0, time.UTC) }, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "Started by") + assert.NotContains(t, out, "Asked by") +} From d6e63f63d3da6f96f65b2ed3887d54595f0d649c Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Thu, 16 Apr 2026 17:03:16 -0500 Subject: [PATCH 02/17] fix(discussion view): align with post-review list changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rewrite GetByNumber to use strongly-typed GraphQL query instead of removed raw string interpolation (discussionFields/discussionNode) - Fix State string references to use Closed bool throughout view.go - Fix DiscussionAuthor → DiscussionActor type rename in tests - Add ReactionGroups field to Discussion domain type and ExportData - Add computed "state" field to ExportData for JSON output - Add shared.DiscussionFields for view command's --json flag - Regenerate client mock Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 62 +++++++++++------------- pkg/cmd/discussion/client/types.go | 13 +++++ pkg/cmd/discussion/shared/client.go | 25 ++++++++++ pkg/cmd/discussion/view/view.go | 10 ++-- pkg/cmd/discussion/view/view_test.go | 4 +- 5 files changed, 76 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index ed5536fac..9cba1f803 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -352,51 +352,47 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, } func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Discussion, error) { - query := fmt.Sprintf(`query DiscussionByNumber($owner: String!, $name: String!, $number: Int!) { - repository(owner: $owner, name: $name) { - hasDiscussionsEnabled - discussion(number: $number) { - %s - body - comments { totalCount } - } - } - }`, discussionFields) + var query struct { + Repository struct { + HasDiscussionsEnabled bool + Discussion *struct { + discussionListNode + Body string + Comments struct { + TotalCount int + } + } `graphql:"discussion(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } variables := map[string]interface{}{ - "owner": repo.RepoOwner(), - "name": repo.RepoName(), - "number": number, + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + "number": githubv4.Int(number), } - type response struct { - Repository struct { - HasDiscussionsEnabled bool `json:"hasDiscussionsEnabled"` - Discussion *struct { - discussionNode - Body string `json:"body"` - Comments struct { - TotalCount int `json:"totalCount"` - } `json:"comments"` - } `json:"discussion"` - } `json:"repository"` - } - - var data response - err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data) + err := c.gql.Query(repo.RepoHost(), "DiscussionByNumber", &query, variables) if err != nil { return nil, err } - if !data.Repository.HasDiscussionsEnabled { + if !query.Repository.HasDiscussionsEnabled { return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) } - if data.Repository.Discussion == nil { + if query.Repository.Discussion == nil { return nil, fmt.Errorf("discussion #%d not found in '%s/%s'", number, repo.RepoOwner(), repo.RepoName()) } - d := mapDiscussion(data.Repository.Discussion.discussionNode) - d.Body = data.Repository.Discussion.Body - d.Comments = DiscussionCommentList{TotalCount: data.Repository.Discussion.Comments.TotalCount} + d := mapDiscussionFromListNode(query.Repository.Discussion.discussionListNode) + d.Body = query.Repository.Discussion.Body + d.Comments = DiscussionCommentList{TotalCount: query.Repository.Discussion.Comments.TotalCount} + + for _, rg := range query.Repository.Discussion.ReactionGroups { + d.ReactionGroups = append(d.ReactionGroups, ReactionGroup{ + Content: rg.Content, + TotalCount: rg.Users.TotalCount, + }) + } + return &d, nil } diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index f3c58456c..0fffa9a8d 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -19,6 +19,7 @@ type Discussion struct { AnswerChosenAt time.Time AnswerChosenBy *DiscussionActor Comments DiscussionCommentList + ReactionGroups []ReactionGroup CreatedAt time.Time UpdatedAt time.Time ClosedAt time.Time @@ -44,6 +45,12 @@ func (d Discussion) ExportData(fields []string) map[string]interface{} { data[f] = d.URL case "closed": data[f] = d.Closed + case "state": + if d.Closed { + data[f] = "CLOSED" + } else { + data[f] = "OPEN" + } case "stateReason": data[f] = d.StateReason case "author": @@ -79,6 +86,12 @@ func (d Discussion) ExportData(fields []string) map[string]interface{} { "totalCount": d.Comments.TotalCount, "nodes": comments, } + case "reactionGroups": + reactions := make([]interface{}, len(d.ReactionGroups)) + for i, rg := range d.ReactionGroups { + reactions[i] = rg.Export() + } + data[f] = reactions case "createdAt": data[f] = d.CreatedAt case "updatedAt": diff --git a/pkg/cmd/discussion/shared/client.go b/pkg/cmd/discussion/shared/client.go index d0f34e04b..96c3345e0 100644 --- a/pkg/cmd/discussion/shared/client.go +++ b/pkg/cmd/discussion/shared/client.go @@ -7,6 +7,31 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" ) +// DiscussionFields lists all field names available for --json output +// on discussion commands that return a full discussion (e.g. view). +var DiscussionFields = []string{ + "id", + "number", + "title", + "body", + "url", + "closed", + "state", + "stateReason", + "author", + "category", + "labels", + "answered", + "answerChosenAt", + "answerChosenBy", + "comments", + "reactionGroups", + "createdAt", + "updatedAt", + "closedAt", + "locked", +} + // DiscussionClientFunc returns a factory function that creates a DiscussionClient // from the given Factory. The returned function is intended to be stored in // command Options structs and called lazily inside RunE. diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index 788b7d766..c7942d432 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -140,7 +140,7 @@ func printHumanView(opts *ViewOptions, d *client.Discussion) error { cs := opts.IO.ColorScheme() numberStr := fmt.Sprintf("#%d", d.Number) - if d.State == "OPEN" { + if !d.Closed { numberStr = cs.Green(numberStr) } else { numberStr = cs.Muted(numberStr) @@ -149,7 +149,7 @@ func printHumanView(opts *ViewOptions, d *client.Discussion) error { state := "Open" stateColor := cs.Green - if d.State != "OPEN" { + if d.Closed { state = "Closed" stateColor = cs.Muted } @@ -199,7 +199,11 @@ func printHumanView(opts *ViewOptions, d *client.Discussion) error { func printRawView(out io.Writer, d *client.Discussion) error { fmt.Fprintf(out, "title:\t%s\n", d.Title) - fmt.Fprintf(out, "state:\t%s\n", d.State) + state := "OPEN" + if d.Closed { + state = "CLOSED" + } + fmt.Fprintf(out, "state:\t%s\n", state) fmt.Fprintf(out, "category:\t%s\n", d.Category.Name) fmt.Fprintf(out, "author:\t%s\n", d.Author.Login) fmt.Fprintf(out, "labels:\t%s\n", labelList(d.Labels, nil)) diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go index 9694fcaa8..1299f2f64 100644 --- a/pkg/cmd/discussion/view/view_test.go +++ b/pkg/cmd/discussion/view/view_test.go @@ -22,8 +22,8 @@ func testDiscussion() *client.Discussion { Title: "How to authenticate with SSO?", Body: "I need help with SSO authentication.", URL: "https://github.com/OWNER/REPO/discussions/123", - State: "OPEN", - Author: client.DiscussionAuthor{Login: "monalisa"}, + Closed: false, + Author: client.DiscussionActor{Login: "monalisa"}, Category: client.DiscussionCategory{ Name: "Q&A", Slug: "q-a", IsAnswerable: true, }, From ca84d4c6a3f80ef29fc253b914441b1315dd7000 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Fri, 17 Apr 2026 14:51:44 -0500 Subject: [PATCH 03/17] 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())) +} From dea54ab3ababe2d8b8712e2a72cc2079b9c114bf Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Mon, 20 Apr 2026 15:41:55 -0500 Subject: [PATCH 04/17] fix: gofmt alignment in GetWithComments response struct Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index e50e85cad..7b4ad3ce9 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -494,14 +494,14 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co 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"` + 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"` From cd3b23bf36c008a49ebf8791d46ea5b2c89f4518 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 21:37:58 +0100 Subject: [PATCH 05/17] docs(discussion view): add preview remark Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/view/view.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index 782311e59..7bfb379ca 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -44,7 +44,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "view { | }", - Short: "View a discussion", + Short: "View a discussion (preview)", Long: heredoc.Docf(` Display the title, body, and other information about a discussion. From 57b3e9091cd5d622ad86468b73e099204cba07f4 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 21:53:10 +0100 Subject: [PATCH 06/17] fix(discussion/shared): anchor discussion url regexp Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/shared/lookup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/shared/lookup.go b/pkg/cmd/discussion/shared/lookup.go index 1196dd477..c1b78771a 100644 --- a/pkg/cmd/discussion/shared/lookup.go +++ b/pkg/cmd/discussion/shared/lookup.go @@ -9,7 +9,7 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" ) -var discussionURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/discussions/(\d+)`) +var discussionURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/discussions/(\d+)$`) // ParseDiscussionArg parses a discussion number or URL from a command argument. // It returns the discussion number and, if the argument was a URL, a repo override. From 7603a0c8ce0bb0dc588a3f93bc384b6442f550cf Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 21:53:56 +0100 Subject: [PATCH 07/17] fix(discussion/shared): quote arg in errors Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/shared/lookup.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/discussion/shared/lookup.go b/pkg/cmd/discussion/shared/lookup.go index c1b78771a..98fdacd13 100644 --- a/pkg/cmd/discussion/shared/lookup.go +++ b/pkg/cmd/discussion/shared/lookup.go @@ -26,12 +26,12 @@ func ParseDiscussionArg(arg string) (int, ghrepo.Interface, error) { u, err := url.Parse(arg) if err != nil || (u.Scheme != "http" && u.Scheme != "https") { - return 0, nil, fmt.Errorf("invalid discussion argument: %s", arg) + return 0, nil, fmt.Errorf("invalid discussion argument: %q", arg) } m := discussionURLRE.FindStringSubmatch(u.Path) if m == nil { - return 0, nil, fmt.Errorf("invalid discussion URL: %s", arg) + return 0, nil, fmt.Errorf("invalid discussion URL: %q", arg) } num, _ := strconv.Atoi(m[3]) From ca8d878d80f44ec991968d91021f95c87bf1458b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 21:54:35 +0100 Subject: [PATCH 08/17] docs(discussion/shared): explain why we accept http scheme Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/shared/lookup.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/discussion/shared/lookup.go b/pkg/cmd/discussion/shared/lookup.go index 98fdacd13..e754568f0 100644 --- a/pkg/cmd/discussion/shared/lookup.go +++ b/pkg/cmd/discussion/shared/lookup.go @@ -29,6 +29,9 @@ func ParseDiscussionArg(arg string) (int, ghrepo.Interface, error) { return 0, nil, fmt.Errorf("invalid discussion argument: %q", arg) } + // Note that an HTTP URL is also okay, because we're just using the URL to find + // the discussion number, repo and host, and we wont be unsecure HTTP API calls. + m := discussionURLRE.FindStringSubmatch(u.Path) if m == nil { return 0, nil, fmt.Errorf("invalid discussion URL: %q", arg) From f7a79683c0372b6eaff2973c9cc25d6521ab705d Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 21:55:14 +0100 Subject: [PATCH 09/17] test(discussion/shared): add test for ParseDiscussionArg Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/shared/lookup_test.go | 119 +++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 pkg/cmd/discussion/shared/lookup_test.go diff --git a/pkg/cmd/discussion/shared/lookup_test.go b/pkg/cmd/discussion/shared/lookup_test.go new file mode 100644 index 000000000..0670efdd9 --- /dev/null +++ b/pkg/cmd/discussion/shared/lookup_test.go @@ -0,0 +1,119 @@ +package shared + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseDiscussionArg(t *testing.T) { + tests := []struct { + name string + arg string + wantNum int + wantOwner string + wantRepo string + wantHost string + wantErr string + }{ + { + name: "empty", + arg: "", + wantErr: `invalid discussion argument: ""`, + }, + { + name: "whitespaces", + arg: " ", + wantErr: `invalid discussion argument: " "`, + }, + { + name: "invalid string", + arg: "not-a-number", + wantErr: `invalid discussion argument: "not-a-number"`, + }, + { + name: "hash only", + arg: "#", + wantErr: `invalid discussion argument: "#"`, + }, + { + name: "hash non-numeric", + arg: "#abc", + wantErr: `invalid discussion argument: "#abc"`, + }, + { + name: "URL with wrong path", + arg: "https://github.com/owner/repo/issues/10", + wantErr: `invalid discussion URL: "https://github.com/owner/repo/issues/10"`, + }, + { + name: "URL missing number", + arg: "https://github.com/owner/repo/discussions/", + wantErr: `invalid discussion URL: "https://github.com/owner/repo/discussions/"`, + }, + { + name: "zero", + arg: "0", + wantNum: 0, + }, + { + name: "plain number", + arg: "42", + wantNum: 42, + }, + { + name: "hash number", + arg: "#99", + wantNum: 99, + }, + { + name: "HTTPS URL", + arg: "https://github.com/cli/cli/discussions/123", + wantNum: 123, + wantOwner: "cli", + wantRepo: "cli", + wantHost: "github.com", + }, + { + name: "HTTP URL", + arg: "http://github.com/owner/repo/discussions/7", + wantNum: 7, + wantOwner: "owner", + wantRepo: "repo", + wantHost: "github.com", + }, + { + name: "GHES URL", + arg: "https://git.example.com/org/project/discussions/55", + wantNum: 55, + wantOwner: "org", + wantRepo: "project", + wantHost: "git.example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + num, repo, err := ParseDiscussionArg(tt.arg) + + if tt.wantErr != "" { + require.Error(t, err) + assert.EqualError(t, err, tt.wantErr) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.wantNum, num) + + if tt.wantOwner != "" || tt.wantRepo != "" || tt.wantHost != "" { + require.NotNil(t, repo) + assert.Equal(t, tt.wantOwner, repo.RepoOwner()) + assert.Equal(t, tt.wantRepo, repo.RepoName()) + assert.Equal(t, tt.wantHost, repo.RepoHost()) + } else { + assert.Nil(t, repo) + } + }) + } +} From 6bd96abd6a76f2df6a300c094817f271e4f037f3 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 22:01:51 +0100 Subject: [PATCH 10/17] fix(gh discussion view): wrap arg parse error as flag error Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/view/view.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index 7bfb379ca..8cb027b44 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -77,7 +77,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman number, repo, err := shared.ParseDiscussionArg(args[0]) if err != nil { - return err + return cmdutil.FlagErrorf("%s", err) } if repo != nil { From 75b71505c882e9707cf81eb618bad170aae42c82 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 22:19:59 +0100 Subject: [PATCH 11/17] refactor(discussion view): use slices package for label sorting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/view/view.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index 8cb027b44..ca94b238e 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -3,7 +3,7 @@ package view import ( "fmt" "io" - "sort" + "slices" "strings" "time" @@ -331,12 +331,13 @@ func labelList(labels []client.DiscussionLabel, cs *iostreams.ColorScheme) strin return "" } - sort.SliceStable(labels, func(i, j int) bool { - return strings.ToLower(labels[i].Name) < strings.ToLower(labels[j].Name) + sortedLabels := slices.Clone(labels) + slices.SortStableFunc(sortedLabels, func(i, j client.DiscussionLabel) int { + return strings.Compare(i.Name, j.Name) }) - names := make([]string, len(labels)) - for i, l := range labels { + names := make([]string, len(sortedLabels)) + for i, l := range sortedLabels { if cs == nil { names[i] = l.Name } else { From d2e081bce13341b64c875510eb26393cc4a7d76f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 23:50:04 +0100 Subject: [PATCH 12/17] feat(discussion view): add cursor-based pagination to comments Add --limit and --after flags for paginating through discussion comments. Cursor output is shown in TTY (hint message), raw (next: field), and JSON. Change GetWithComments to accept 'after' cursor and 'newest' bool instead of order string. Implement forward/backward cursor-based pagination in GraphQL queries depending on comment order. Change Replies from []DiscussionComment to DiscussionCommentList with Direction field. Display direction-aware messages (newer/older) for both comments and replies. Move DiscussionFields and reactionGroupList from shared to view package. Delete shared/display.go. Add 7 new pagination tests and update existing test fixtures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client.go | 2 +- pkg/cmd/discussion/client/client_impl.go | 81 ++++++-- pkg/cmd/discussion/client/client_mock.go | 26 ++- pkg/cmd/discussion/client/types.go | 32 ++- pkg/cmd/discussion/shared/client.go | 25 --- pkg/cmd/discussion/shared/display.go | 35 ---- pkg/cmd/discussion/view/view.go | 112 +++++++++- pkg/cmd/discussion/view/view_test.go | 251 +++++++++++++++++++++-- 8 files changed, 438 insertions(+), 126 deletions(-) delete mode 100644 pkg/cmd/discussion/shared/display.go diff --git a/pkg/cmd/discussion/client/client.go b/pkg/cmd/discussion/client/client.go index a794d13e1..5c7f1bdc4 100644 --- a/pkg/cmd/discussion/client/client.go +++ b/pkg/cmd/discussion/client/client.go @@ -12,7 +12,7 @@ type DiscussionClient interface { List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (*DiscussionListResult, error) GetByNumber(repo ghrepo.Interface, number int) (*Discussion, error) - GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) + GetWithComments(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*Discussion, error) ListCategories(repo ghrepo.Interface) ([]DiscussionCategory, error) Create(repo ghrepo.Interface, input CreateDiscussionInput) (*Discussion, error) Update(repo ghrepo.Interface, input UpdateDiscussionInput) (*Discussion, error) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 7b4ad3ce9..d83c9d35d 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -3,6 +3,7 @@ package client import ( "fmt" "net/http" + "slices" "strings" "time" @@ -355,9 +356,8 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc var query struct { Repository struct { HasDiscussionsEnabled bool - Discussion *struct { + Discussion struct { discussionListNode - Body string Comments struct { TotalCount int } @@ -371,19 +371,15 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc "number": githubv4.Int(number), } - err := c.gql.Query(repo.RepoHost(), "DiscussionByNumber", &query, variables) + err := c.gql.Query(repo.RepoHost(), "DiscussionMinimal", &query, variables) if err != nil { return nil, err } if !query.Repository.HasDiscussionsEnabled { return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) } - if query.Repository.Discussion == nil { - return nil, fmt.Errorf("discussion #%d not found in '%s/%s'", number, repo.RepoOwner(), repo.RepoName()) - } d := mapDiscussionFromListNode(query.Repository.Discussion.discussionListNode) - d.Body = query.Repository.Discussion.Body d.Comments = DiscussionCommentList{TotalCount: query.Repository.Discussion.Comments.TotalCount} for _, rg := range query.Repository.Discussion.ReactionGroups { @@ -396,12 +392,19 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc return &d, nil } -func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { +func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, limit int, after string, newest bool) (*Discussion, error) { // Build the comments field with first/last based on order. - // "oldest" uses first (chronological), "newest" uses last (reverse chronological). + // oldest uses first+after (chronological), newest uses last+before (reverse). commentDirection := "first" - if order == "newest" { + cursorDirection := "after" + if newest { commentDirection = "last" + cursorDirection = "before" + } + + cursorArg := "" + if after != "" { + cursorArg = fmt.Sprintf(`, %s: "%s"`, cursorDirection, after) } query := fmt.Sprintf(`query DiscussionWithComments($owner: String!, $name: String!, $number: Int!) { @@ -426,8 +429,14 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co updatedAt closedAt locked - comments(%s: %d) { + comments(%s: %d%s) { totalCount + pageInfo { + endCursor + hasNextPage + startCursor + hasPreviousPage + } nodes { id url @@ -437,7 +446,7 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co isAnswer upvoteCount reactionGroups { content users { totalCount } } - replies(first: 4) { + replies(last: 4) { totalCount nodes { id @@ -454,7 +463,7 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co } } } - }`, commentDirection, commentLimit) + }`, commentDirection, limit, cursorArg) variables := map[string]interface{}{ "owner": repo.RepoOwner(), @@ -525,8 +534,14 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co ClosedAt time.Time `json:"closedAt"` Locked bool `json:"locked"` Comments struct { - TotalCount int `json:"totalCount"` - Nodes []commentJSON `json:"nodes"` + TotalCount int `json:"totalCount"` + PageInfo struct { + EndCursor string `json:"endCursor"` + HasNextPage bool `json:"hasNextPage"` + StartCursor string `json:"startCursor"` + HasPreviousPage bool `json:"hasPreviousPage"` + } `json:"pageInfo"` + Nodes []commentJSON `json:"nodes"` } `json:"comments"` } `json:"discussion"` } `json:"repository"` @@ -570,9 +585,9 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co 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{ + replyComments := make([]DiscussionComment, len(c.Replies.Nodes)) + for i, r := range c.Replies.Nodes { + replyComments[i] = DiscussionComment{ ID: r.ID, URL: r.URL, Author: mapActor(r.Author), @@ -581,7 +596,12 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co IsAnswer: r.IsAnswer, UpvoteCount: r.UpvoteCount, ReactionGroups: mapReactions(r.ReactionGroups), - }) + } + } + dc.Replies = DiscussionCommentList{ + Comments: replyComments, + TotalCount: c.Replies.TotalCount, + Direction: DiscussionCommentListDirectionBackward, // Since we always fetch the last 4 replies } } return dc @@ -629,15 +649,32 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co // 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] + if newest { + slices.Reverse(comments) + } + + nextCursor := "" + if newest { + if src.Comments.PageInfo.HasPreviousPage { + nextCursor = src.Comments.PageInfo.StartCursor } + } else { + if src.Comments.PageInfo.HasNextPage { + nextCursor = src.Comments.PageInfo.EndCursor + } + } + + direction := DiscussionCommentListDirectionForward + if newest { + direction = DiscussionCommentListDirectionBackward } d.Comments = DiscussionCommentList{ Comments: comments, TotalCount: src.Comments.TotalCount, + Cursor: after, + NextCursor: nextCursor, + Direction: direction, } return &d, nil diff --git a/pkg/cmd/discussion/client/client_mock.go b/pkg/cmd/discussion/client/client_mock.go index a690f84b1..a3cec3c63 100644 --- a/pkg/cmd/discussion/client/client_mock.go +++ b/pkg/cmd/discussion/client/client_mock.go @@ -30,7 +30,7 @@ var _ DiscussionClient = &DiscussionClientMock{} // GetByNumberFunc: func(repo ghrepo.Interface, number int) (*Discussion, error) { // panic("mock out the GetByNumber method") // }, -// GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { +// GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*Discussion, error) { // panic("mock out the GetWithComments method") // }, // ListFunc: func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) { @@ -80,7 +80,7 @@ type DiscussionClientMock struct { GetByNumberFunc func(repo ghrepo.Interface, number int) (*Discussion, error) // GetWithCommentsFunc mocks the GetWithComments method. - GetWithCommentsFunc func(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) + GetWithCommentsFunc func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*Discussion, error) // ListFunc mocks the List method. ListFunc func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) @@ -153,8 +153,10 @@ type DiscussionClientMock struct { Number int // CommentLimit is the commentLimit argument value. CommentLimit int - // Order is the order argument value. - Order string + // After is the after argument value. + After string + // Newest is the newest argument value. + Newest bool } // List holds details about calls to the List method. List []struct { @@ -401,7 +403,7 @@ func (mock *DiscussionClientMock) GetByNumberCalls() []struct { } // GetWithComments calls GetWithCommentsFunc. -func (mock *DiscussionClientMock) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { +func (mock *DiscussionClientMock) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*Discussion, error) { if mock.GetWithCommentsFunc == nil { panic("DiscussionClientMock.GetWithCommentsFunc: method is nil but DiscussionClient.GetWithComments was just called") } @@ -409,17 +411,19 @@ func (mock *DiscussionClientMock) GetWithComments(repo ghrepo.Interface, number Repo ghrepo.Interface Number int CommentLimit int - Order string + After string + Newest bool }{ Repo: repo, Number: number, CommentLimit: commentLimit, - Order: order, + After: after, + Newest: newest, } mock.lockGetWithComments.Lock() mock.calls.GetWithComments = append(mock.calls.GetWithComments, callInfo) mock.lockGetWithComments.Unlock() - return mock.GetWithCommentsFunc(repo, number, commentLimit, order) + return mock.GetWithCommentsFunc(repo, number, commentLimit, after, newest) } // GetWithCommentsCalls gets all the calls that were made to GetWithComments. @@ -430,13 +434,15 @@ func (mock *DiscussionClientMock) GetWithCommentsCalls() []struct { Repo ghrepo.Interface Number int CommentLimit int - Order string + After string + Newest bool } { var calls []struct { Repo ghrepo.Interface Number int CommentLimit int - Order string + After string + Newest bool } mock.lockGetWithComments.RLock() calls = mock.calls.GetWithComments diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index 0fffa9a8d..a5affcb3a 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -82,10 +82,17 @@ func (d Discussion) ExportData(fields []string) map[string]interface{} { for i, c := range d.Comments.Comments { comments[i] = c.Export() } - data[f] = map[string]interface{}{ + m := map[string]interface{}{ "totalCount": d.Comments.TotalCount, "nodes": comments, } + if d.Comments.Cursor != "" { + m["cursor"] = d.Comments.Cursor + } + if d.Comments.NextCursor != "" { + m["next"] = d.Comments.NextCursor + } + data[f] = m case "reactionGroups": reactions := make([]interface{}, len(d.ReactionGroups)) for i, rg := range d.ReactionGroups { @@ -171,14 +178,13 @@ type DiscussionComment struct { IsAnswer bool UpvoteCount int ReactionGroups []ReactionGroup - Replies []DiscussionComment - TotalReplies int + Replies DiscussionCommentList } // Export returns the comment as a map for JSON output. func (c DiscussionComment) Export() map[string]interface{} { - replies := make([]interface{}, len(c.Replies)) - for i, r := range c.Replies { + replies := make([]interface{}, len(c.Replies.Comments)) + for i, r := range c.Replies.Comments { replies[i] = r.Export() } reactions := make([]interface{}, len(c.ReactionGroups)) @@ -194,15 +200,27 @@ func (c DiscussionComment) Export() map[string]interface{} { "isAnswer": c.IsAnswer, "upvoteCount": c.UpvoteCount, "reactionGroups": reactions, - "replies": replies, - "totalReplies": c.TotalReplies, + "replies": map[string]interface{}{ + "totalCount": c.Replies.TotalCount, + "nodes": replies, + }, } } +type DiscussionCommentListDirection string + +const ( + DiscussionCommentListDirectionForward DiscussionCommentListDirection = "forward" + DiscussionCommentListDirectionBackward DiscussionCommentListDirection = "backward" +) + // DiscussionCommentList represents a paginated list of comments on a discussion. type DiscussionCommentList struct { Comments []DiscussionComment TotalCount int + Cursor string + NextCursor string + Direction DiscussionCommentListDirection } // ReactionGroup represents a set of reactions of the same type. diff --git a/pkg/cmd/discussion/shared/client.go b/pkg/cmd/discussion/shared/client.go index 96c3345e0..d0f34e04b 100644 --- a/pkg/cmd/discussion/shared/client.go +++ b/pkg/cmd/discussion/shared/client.go @@ -7,31 +7,6 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" ) -// DiscussionFields lists all field names available for --json output -// on discussion commands that return a full discussion (e.g. view). -var DiscussionFields = []string{ - "id", - "number", - "title", - "body", - "url", - "closed", - "state", - "stateReason", - "author", - "category", - "labels", - "answered", - "answerChosenAt", - "answerChosenBy", - "comments", - "reactionGroups", - "createdAt", - "updatedAt", - "closedAt", - "locked", -} - // DiscussionClientFunc returns a factory function that creates a DiscussionClient // from the given Factory. The returned function is intended to be stored in // command Options structs and called lazily inside RunE. diff --git a/pkg/cmd/discussion/shared/display.go b/pkg/cmd/discussion/shared/display.go deleted file mode 100644 index ea866fcb2..000000000 --- a/pkg/cmd/discussion/shared/display.go +++ /dev/null @@ -1,35 +0,0 @@ -package shared - -import ( - "fmt" - "strings" - - "github.com/cli/cli/v2/pkg/cmd/discussion/client" -) - -var reactionEmoji = map[string]string{ - "THUMBS_UP": "\U0001f44d", - "THUMBS_DOWN": "\U0001f44e", - "LAUGH": "\U0001f604", - "HOORAY": "\U0001f389", - "CONFUSED": "\U0001f615", - "HEART": "\u2764\ufe0f", - "ROCKET": "\U0001f680", - "EYES": "\U0001f440", -} - -// ReactionGroupList formats reaction groups for display. -func ReactionGroupList(groups []client.ReactionGroup) string { - var parts []string - for _, g := range groups { - if g.TotalCount == 0 { - continue - } - emoji := reactionEmoji[g.Content] - if emoji == "" { - emoji = g.Content - } - parts = append(parts, fmt.Sprintf("%s %d", emoji, g.TotalCount)) - } - return strings.Join(parts, " • ") -} diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index ca94b238e..b114aaaa2 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -19,6 +19,55 @@ import ( "github.com/spf13/cobra" ) +var discussionFields = []string{ + "id", + "number", + "title", + "body", + "url", + "closed", + "state", + "stateReason", + "author", + "category", + "labels", + "answered", + "answerChosenAt", + "answerChosenBy", + "comments", + "reactionGroups", + "createdAt", + "updatedAt", + "closedAt", + "locked", +} + +var reactionEmoji = map[string]string{ + "THUMBS_UP": "\U0001f44d", + "THUMBS_DOWN": "\U0001f44e", + "LAUGH": "\U0001f604", + "HOORAY": "\U0001f389", + "CONFUSED": "\U0001f615", + "HEART": "\u2764\ufe0f", + "ROCKET": "\U0001f680", + "EYES": "\U0001f440", +} + +func reactionGroupList(groups []client.ReactionGroup) string { + var parts []string + for _, g := range groups { + if g.TotalCount == 0 { + continue + } + emoji := reactionEmoji[g.Content] + if emoji == "" { + emoji = g.Content + } + parts = append(parts, fmt.Sprintf("%s %d", emoji, g.TotalCount)) + } + return strings.Join(parts, " • ") +} + // ViewOptions holds the configuration for the view command. type ViewOptions struct { IO *iostreams.IOStreams @@ -29,6 +78,8 @@ type ViewOptions struct { DiscussionNumber int WebMode bool Comments bool + Limit int + After string Order string Exporter cmdutil.Exporter Now func() time.Time @@ -50,6 +101,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman 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). + Use %[1]s--limit%[1]s and %[1]s--after%[1]s for paginating through comments. With %[1]s--web%[1]s flag, open the discussion in a web browser instead. `, "`"), @@ -64,7 +116,13 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman $ gh discussion view 123 --comments # View with newest comments first - $ gh discussion view 123 --comments --order newest + $ gh discussion view 123 --comments --order oldest + + # Limit to 10 comments + $ gh discussion view 123 --comments --limit 10 + + # Fetch the next page of comments + $ gh discussion view 123 --comments --after CURSOR # Open in browser $ gh discussion view 123 --web @@ -74,6 +132,15 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman if cmd.Flags().Changed("order") && !opts.Comments { return cmdutil.FlagErrorf("--order requires --comments") } + if cmd.Flags().Changed("limit") && !opts.Comments { + return cmdutil.FlagErrorf("--limit requires --comments") + } + if cmd.Flags().Changed("after") && !opts.Comments { + return cmdutil.FlagErrorf("--after requires --comments") + } + if opts.Limit < 1 { + return cmdutil.FlagErrorf("invalid limit: %d", opts.Limit) + } number, repo, err := shared.ParseDiscussionArg(args[0]) if err != nil { @@ -100,8 +167,10 @@ 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) + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of comments to fetch") + cmd.Flags().StringVar(&opts.After, "after", "", "Cursor for the next page of comments") + cmdutil.StringEnumFlag(cmd, &opts.Order, "order", "", "newest", []string{"oldest", "newest"}, "Order of comments") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, discussionFields) return cmd } @@ -130,7 +199,7 @@ func viewRun(opts *ViewOptions) error { var discussion *client.Discussion if opts.Comments { - discussion, err = c.GetWithComments(repo, opts.DiscussionNumber, 30, opts.Order) + discussion, err = c.GetWithComments(repo, opts.DiscussionNumber, opts.Limit, opts.After, opts.Order == "newest") } else { discussion, err = c.GetByNumber(repo, opts.DiscussionNumber) } @@ -209,7 +278,7 @@ func printHumanView(opts *ViewOptions, d *client.Discussion) error { } fmt.Fprintf(out, "\n%s\n", md) - if reactions := shared.ReactionGroupList(d.ReactionGroups); reactions != "" { + if reactions := reactionGroupList(d.ReactionGroups); reactions != "" { fmt.Fprintln(out, reactions) fmt.Fprintln(out) } @@ -226,7 +295,19 @@ func printHumanView(opts *ViewOptions, d *client.Discussion) error { } if shown := len(d.Comments.Comments); shown < d.Comments.TotalCount { - fmt.Fprintf(out, cs.Muted(" And %d more comments\n"), d.Comments.TotalCount-shown) + remaining := d.Comments.TotalCount - shown + age := "more" + if d.Comments.Direction == client.DiscussionCommentListDirectionForward { + age = "newer" + } else if d.Comments.Direction == client.DiscussionCommentListDirectionBackward { + age = "older" + } + fmt.Fprintf(out, cs.Muted(" And %d %s comments\n"), remaining, age) + fmt.Fprintln(out) + } + + if d.Comments.NextCursor != "" { + fmt.Fprintf(out, cs.Muted("To see more comments, pass: --after %s\n"), d.Comments.NextCursor) fmt.Fprintln(out) } } @@ -247,6 +328,9 @@ func printRawView(out io.Writer, d *client.Discussion, showComments bool) error fmt.Fprintf(out, "author:\t%s\n", d.Author.Login) fmt.Fprintf(out, "labels:\t%s\n", labelList(d.Labels, nil)) fmt.Fprintf(out, "comments:\t%d\n", d.Comments.TotalCount) + if showComments && d.Comments.NextCursor != "" { + fmt.Fprintf(out, "next:\t%s\n", d.Comments.NextCursor) + } fmt.Fprintf(out, "number:\t%d\n", d.Number) fmt.Fprintf(out, "url:\t%s\n", d.URL) fmt.Fprintln(out, "--") @@ -288,20 +372,26 @@ func printHumanComment(opts *ViewOptions, out io.Writer, c client.DiscussionComm fmt.Fprint(out, md) } - if reactions := shared.ReactionGroupList(c.ReactionGroups); reactions != "" { + if reactions := reactionGroupList(c.ReactionGroups); reactions != "" { fmt.Fprintf(out, "%s%s\n", indent, reactions) } fmt.Fprintln(out) - for _, reply := range c.Replies { + for _, reply := range c.Replies.Comments { 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))) + if shown := len(c.Replies.Comments); shown < c.Replies.TotalCount { + directionLabel := "more" + if c.Replies.Direction == client.DiscussionCommentListDirectionForward { + directionLabel = "newer" + } else if c.Replies.Direction == client.DiscussionCommentListDirectionBackward { + directionLabel = "older" + } + fmt.Fprintf(out, "%s %s\n\n", indent, cs.Muted(fmt.Sprintf("And %d %s replies", c.Replies.TotalCount-shown, directionLabel))) } return nil @@ -321,7 +411,7 @@ func printRawComment(out io.Writer, c client.DiscussionComment, indent string) { } fmt.Fprintln(out) - for _, reply := range c.Replies { + for _, reply := range c.Replies.Comments { printRawComment(out, reply, indent+" ") } } diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go index 6e90d131c..92a3846a6 100644 --- a/pkg/cmd/discussion/view/view_test.go +++ b/pkg/cmd/discussion/view/view_test.go @@ -8,7 +8,6 @@ import ( "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/discussion/client" - "github.com/cli/cli/v2/pkg/cmd/discussion/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/stretchr/testify/assert" @@ -192,7 +191,7 @@ func TestViewRun_json(t *testing.T) { } exporter := cmdutil.NewJSONExporter() - exporter.SetFields(shared.DiscussionFields) + exporter.SetFields(discussionFields) opts := &ViewOptions{ IO: ios, @@ -367,16 +366,18 @@ func testDiscussionWithComments() *client.Discussion { 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), + Replies: client.DiscussionCommentList{ + TotalCount: 5, + Comments: []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", @@ -397,9 +398,9 @@ func TestViewRun_comments_tty(t *testing.T) { d := testDiscussionWithComments() mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { assert.Equal(t, 30, commentLimit) - assert.Equal(t, "oldest", order) + assert.Equal(t, false, newest) return d, nil }, } @@ -414,6 +415,7 @@ func TestViewRun_comments_tty(t *testing.T) { }, DiscussionNumber: 123, Comments: true, + Limit: 30, Order: "oldest", Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, } @@ -439,7 +441,7 @@ func TestViewRun_comments_nontty(t *testing.T) { d := testDiscussionWithComments() mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { return d, nil }, } @@ -454,6 +456,7 @@ func TestViewRun_comments_nontty(t *testing.T) { }, DiscussionNumber: 123, Comments: true, + Limit: 30, Order: "oldest", Now: time.Now, } @@ -475,13 +478,13 @@ func TestViewRun_comments_json(t *testing.T) { d := testDiscussionWithComments() mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { return d, nil }, } exporter := cmdutil.NewJSONExporter() - exporter.SetFields(shared.DiscussionFields) + exporter.SetFields(discussionFields) opts := &ViewOptions{ IO: ios, @@ -493,6 +496,7 @@ func TestViewRun_comments_json(t *testing.T) { }, DiscussionNumber: 123, Comments: true, + Limit: 30, Order: "oldest", Exporter: exporter, Now: time.Now, @@ -559,3 +563,220 @@ func TestViewRun_noComments_usesGetByNumber(t *testing.T) { assert.Equal(t, 1, len(mock.GetByNumberCalls())) assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) } + +func TestNewCmdView_limitWithoutComments(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", "--limit", "10"}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "--limit requires --comments") +} + +func TestNewCmdView_afterWithoutComments(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", "--after", "CURSOR_ABC"}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "--after requires --comments") +} + +func TestNewCmdView_invalidLimit(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", "--comments", "--limit", "0"}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid limit") +} + +func TestViewRun_commentsWithPagination_tty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + d := testDiscussionWithComments() + d.Comments.NextCursor = "NEXT_CURSOR_123" + + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, 10, commentLimit) + assert.Equal(t, "CURSOR_ABC", after) + assert.Equal(t, false, newest) + 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, + Limit: 10, + After: "CURSOR_ABC", + 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, "To see more comments, pass: --after NEXT_CURSOR_123") +} + +func TestViewRun_commentsWithPagination_nontty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussionWithComments() + d.Comments.NextCursor = "NEXT_CURSOR_456" + + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*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, + Limit: 30, + Order: "oldest", + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "next:\tNEXT_CURSOR_456") +} + +func TestViewRun_commentsWithPagination_json(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussionWithComments() + d.Comments.Cursor = "PREV_CURSOR" + d.Comments.NextCursor = "NEXT_CURSOR_789" + + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + return d, nil + }, + } + + exporter := cmdutil.NewJSONExporter() + exporter.SetFields(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, + Limit: 30, + Order: "oldest", + Exporter: exporter, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, `"cursor":"PREV_CURSOR"`) + assert.Contains(t, out, `"next":"NEXT_CURSOR_789"`) +} + +func TestViewRun_noPaginationCursor_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, after string, newest bool) (*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, + Limit: 30, + 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.NotContains(t, out, "--after") +} From 9f3a31ddb36c5dfcce5d7f6159bb2e8f5c0c6302 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Sat, 25 Apr 2026 12:19:04 -0500 Subject: [PATCH 13/17] fix(discussion view): use GraphQL variables for cursor, fix --json comments Fix two issues in the discussion view command: 1. GraphQL injection via cursor interpolation: The --after cursor value was interpolated directly into the raw GraphQL query string using fmt.Sprintf, which is unsafe since cursor values come from user input. Now uses GraphQL variables ($cursor: String) instead, matching the pattern used by issue list, pr list, and other commands. 2. Incomplete --json comments output: Running `gh discussion view N --json comments` silently returned only totalCount with no comment nodes, because the data fetch was gated solely on the --comments flag. Now checks if the JSON exporter requests the comments field and fetches full comment data accordingly, matching how issue view and pr view drive data loading from exporter fields. Also fixes example text that said "newest" but showed --order oldest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 27 ++++---- pkg/cmd/discussion/view/view.go | 33 ++++++++-- pkg/cmd/discussion/view/view_test.go | 84 +++++++++++++++++++++++- 3 files changed, 123 insertions(+), 21 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index d83c9d35d..83a0efc7c 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -393,21 +393,16 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc } func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, limit int, after string, newest bool) (*Discussion, error) { - // Build the comments field with first/last based on order. - // oldest uses first+after (chronological), newest uses last+before (reverse). - commentDirection := "first" - cursorDirection := "after" + // Use two static query shapes to avoid interpolating user input into the + // GraphQL document. The cursor is always passed as a variable. + var commentsArg string if newest { - commentDirection = "last" - cursorDirection = "before" + commentsArg = "last: $limit, before: $cursor" + } else { + commentsArg = "first: $limit, after: $cursor" } - cursorArg := "" - if after != "" { - cursorArg = fmt.Sprintf(`, %s: "%s"`, cursorDirection, after) - } - - query := fmt.Sprintf(`query DiscussionWithComments($owner: String!, $name: String!, $number: Int!) { + query := fmt.Sprintf(`query DiscussionWithComments($owner: String!, $name: String!, $number: Int!, $limit: Int!, $cursor: String) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled discussion(number: $number) { @@ -429,7 +424,7 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, li updatedAt closedAt locked - comments(%s: %d%s) { + comments(%s) { totalCount pageInfo { endCursor @@ -463,12 +458,16 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, li } } } - }`, commentDirection, limit, cursorArg) + }`, commentsArg) variables := map[string]interface{}{ "owner": repo.RepoOwner(), "name": repo.RepoName(), "number": number, + "limit": limit, + } + if after != "" { + variables["cursor"] = after } type actorJSON struct { diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index b114aaaa2..97220e0f2 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -116,7 +116,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman $ gh discussion view 123 --comments # View with newest comments first - $ gh discussion view 123 --comments --order oldest + $ gh discussion view 123 --comments --order newest # Limit to 10 comments $ gh discussion view 123 --comments --limit 10 @@ -129,13 +129,14 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if cmd.Flags().Changed("order") && !opts.Comments { + commentsMode := opts.Comments || (opts.Exporter != nil && exporterNeedsComments(opts.Exporter)) + if cmd.Flags().Changed("order") && !commentsMode { return cmdutil.FlagErrorf("--order requires --comments") } - if cmd.Flags().Changed("limit") && !opts.Comments { + if cmd.Flags().Changed("limit") && !commentsMode { return cmdutil.FlagErrorf("--limit requires --comments") } - if cmd.Flags().Changed("after") && !opts.Comments { + if cmd.Flags().Changed("after") && !commentsMode { return cmdutil.FlagErrorf("--after requires --comments") } if opts.Limit < 1 { @@ -175,6 +176,28 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman return cmd } +// exporterNeedsComments returns true when the JSON exporter requests the comments field. +func exporterNeedsComments(exporter cmdutil.Exporter) bool { + for _, f := range exporter.Fields() { + if f == "comments" { + return true + } + } + return false +} + +// needsComments returns true when the command should fetch full comment data, +// either because --comments was set or because --json requested the comments field. +func needsComments(opts *ViewOptions) bool { + if opts.Comments { + return true + } + if opts.Exporter != nil { + return exporterNeedsComments(opts.Exporter) + } + return false +} + func viewRun(opts *ViewOptions) error { repo, err := opts.BaseRepo() if err != nil { @@ -198,7 +221,7 @@ func viewRun(opts *ViewOptions) error { opts.IO.StartProgressIndicator() var discussion *client.Discussion - if opts.Comments { + if needsComments(opts) { discussion, err = c.GetWithComments(repo, opts.DiscussionNumber, opts.Limit, opts.After, opts.Order == "newest") } else { discussion, err = c.GetByNumber(repo, opts.DiscussionNumber) diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go index 92a3846a6..1ae739168 100644 --- a/pkg/cmd/discussion/view/view_test.go +++ b/pkg/cmd/discussion/view/view_test.go @@ -183,9 +183,9 @@ func TestViewRun_json(t *testing.T) { ios, _, stdout, _ := iostreams.Test() ios.SetStdoutTTY(false) - d := testDiscussion() + d := testDiscussionWithComments() mock := &client.DiscussionClientMock{ - GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { return d, nil }, } @@ -202,6 +202,8 @@ func TestViewRun_json(t *testing.T) { return mock, nil }, DiscussionNumber: 123, + Limit: 30, + Order: "newest", Exporter: exporter, Now: time.Now, } @@ -780,3 +782,81 @@ func TestViewRun_noPaginationCursor_tty(t *testing.T) { out := stdout.String() assert.NotContains(t, out, "--after") } + +func TestViewRun_jsonComments_usesGetWithComments(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussionWithComments() + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + return d, nil + }, + } + + exporter := cmdutil.NewJSONExporter() + exporter.SetFields([]string{"comments"}) + + 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, + Limit: 30, + Order: "newest", + Exporter: exporter, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + // --json comments should use GetWithComments even without --comments flag + assert.Equal(t, 0, len(mock.GetByNumberCalls())) + assert.Equal(t, 1, len(mock.GetWithCommentsCalls())) + + out := stdout.String() + assert.Contains(t, out, `"totalCount"`) + assert.Contains(t, out, `"octocat"`) +} + +func TestViewRun_jsonWithoutComments_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 + }, + } + + exporter := cmdutil.NewJSONExporter() + exporter.SetFields([]string{"title", "number"}) + + 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, + Exporter: exporter, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + // --json title,number should NOT fetch comments + assert.Equal(t, 1, len(mock.GetByNumberCalls())) + assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) +} From 49a846aa1ac75d936dc913d72e12b3e768218a75 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 27 Apr 2026 08:45:43 +0100 Subject: [PATCH 14/17] docs(discussion view): use non-default behaviour in example Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/view/view.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index 97220e0f2..27da3f1e7 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -115,8 +115,8 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman # View with comments $ gh discussion view 123 --comments - # View with newest comments first - $ gh discussion view 123 --comments --order newest + # View with oldest comments first + $ gh discussion view 123 --comments --order oldest # Limit to 10 comments $ gh discussion view 123 --comments --limit 10 From 72a6c98d3763f49009dc8f7c3c1ef7c5a020fdf7 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 27 Apr 2026 08:46:56 +0100 Subject: [PATCH 15/17] refactor(discussion view): simplify mode check Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/view/view.go | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index 27da3f1e7..d20a666d2 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -129,7 +129,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - commentsMode := opts.Comments || (opts.Exporter != nil && exporterNeedsComments(opts.Exporter)) + commentsMode := needsComments(opts) if cmd.Flags().Changed("order") && !commentsMode { return cmdutil.FlagErrorf("--order requires --comments") } @@ -178,24 +178,13 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman // exporterNeedsComments returns true when the JSON exporter requests the comments field. func exporterNeedsComments(exporter cmdutil.Exporter) bool { - for _, f := range exporter.Fields() { - if f == "comments" { - return true - } - } - return false + return slices.Contains(exporter.Fields(), "comments") } // needsComments returns true when the command should fetch full comment data, // either because --comments was set or because --json requested the comments field. func needsComments(opts *ViewOptions) bool { - if opts.Comments { - return true - } - if opts.Exporter != nil { - return exporterNeedsComments(opts.Exporter) - } - return false + return opts.Comments || opts.Exporter != nil && exporterNeedsComments(opts.Exporter) } func viewRun(opts *ViewOptions) error { From 524a503e8650e309c9bd9df104e290659cf10034 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 27 Apr 2026 09:09:21 +0100 Subject: [PATCH 16/17] refactor(discussion/client): use strongly-typed query for fetching comments Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 355 ++++++++--------------- 1 file changed, 125 insertions(+), 230 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 83a0efc7c..71890682f 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -392,258 +392,153 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc return &d, nil } -func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, limit int, after string, newest bool) (*Discussion, error) { - // Use two static query shapes to avoid interpolating user input into the - // GraphQL document. The cursor is always passed as a variable. - var commentsArg string - if newest { - commentsArg = "last: $limit, before: $cursor" - } else { - commentsArg = "first: $limit, after: $cursor" +// discussionCommentNode is the GraphQL response shape for a discussion comment +// including nested replies. +type discussionCommentNode struct { + ID string + URL string `graphql:"url"` + Author actorNode + Body string + CreatedAt time.Time + IsAnswer bool + UpvoteCount int + ReactionGroups []struct { + Content string + Users struct { + TotalCount int + } } - - query := fmt.Sprintf(`query DiscussionWithComments($owner: String!, $name: String!, $number: Int!, $limit: Int!, $cursor: String) { - 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) { - totalCount - pageInfo { - endCursor - hasNextPage - startCursor - hasPreviousPage - } - nodes { - id - url - author { login ... on User { id name } ... on Bot { id } } - body - createdAt - isAnswer - upvoteCount - reactionGroups { content users { totalCount } } - replies(last: 4) { - totalCount - nodes { - id - url - author { login ... on User { id name } ... on Bot { id } } - body - createdAt - isAnswer - upvoteCount - reactionGroups { content users { totalCount } } - } - } - } + Replies struct { + TotalCount int + Nodes []struct { + ID string + URL string `graphql:"url"` + Author actorNode + Body string + CreatedAt time.Time + IsAnswer bool + UpvoteCount int + ReactionGroups []struct { + Content string + Users struct { + TotalCount int } } } - }`, commentsArg) + } `graphql:"replies(last: 4)"` +} + +// mapCommentFromNode converts a discussionCommentNode into the domain DiscussionComment type. +func mapCommentFromNode(n discussionCommentNode) DiscussionComment { + dc := DiscussionComment{ + ID: n.ID, + URL: n.URL, + Author: mapActorFromListNode(n.Author), + Body: n.Body, + CreatedAt: n.CreatedAt, + IsAnswer: n.IsAnswer, + UpvoteCount: n.UpvoteCount, + } + + for _, rg := range n.ReactionGroups { + dc.ReactionGroups = append(dc.ReactionGroups, ReactionGroup{ + Content: rg.Content, + TotalCount: rg.Users.TotalCount, + }) + } + + replyComments := make([]DiscussionComment, len(n.Replies.Nodes)) + for i, r := range n.Replies.Nodes { + rc := DiscussionComment{ + ID: r.ID, + URL: r.URL, + Author: mapActorFromListNode(r.Author), + Body: r.Body, + CreatedAt: r.CreatedAt, + IsAnswer: r.IsAnswer, + UpvoteCount: r.UpvoteCount, + } + for _, rg := range r.ReactionGroups { + rc.ReactionGroups = append(rc.ReactionGroups, ReactionGroup{ + Content: rg.Content, + TotalCount: rg.Users.TotalCount, + }) + } + replyComments[i] = rc + } + dc.Replies = DiscussionCommentList{ + Comments: replyComments, + TotalCount: n.Replies.TotalCount, + Direction: DiscussionCommentListDirectionBackward, + } + + return dc +} + +func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, limit int, after string, newest bool) (*Discussion, error) { + var query struct { + Repository struct { + HasDiscussionsEnabled bool + Discussion struct { + discussionListNode + Comments struct { + TotalCount int + PageInfo struct { + EndCursor string + HasNextPage bool + StartCursor string + HasPreviousPage bool + } + Nodes []discussionCommentNode + } `graphql:"comments(first: $first, last: $last, after: $after, before: $before)"` + } `graphql:"discussion(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } variables := map[string]interface{}{ - "owner": repo.RepoOwner(), - "name": repo.RepoName(), - "number": number, - "limit": limit, - } - if after != "" { - variables["cursor"] = after + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + "number": githubv4.Int(number), + "first": (*githubv4.Int)(nil), + "last": (*githubv4.Int)(nil), + "after": (*githubv4.String)(nil), + "before": (*githubv4.String)(nil), } - type actorJSON struct { - Login string `json:"login"` - ID string `json:"id"` - Name string `json:"name"` + if newest { + variables["last"] = githubv4.Int(limit) + if after != "" { + variables["before"] = githubv4.String(after) + } + } else { + variables["first"] = githubv4.Int(limit) + if after != "" { + variables["after"] = githubv4.String(after) + } } - 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"` - PageInfo struct { - EndCursor string `json:"endCursor"` - HasNextPage bool `json:"hasNextPage"` - StartCursor string `json:"startCursor"` - HasPreviousPage bool `json:"hasPreviousPage"` - } `json:"pageInfo"` - Nodes []commentJSON `json:"nodes"` - } `json:"comments"` - } `json:"discussion"` - } `json:"repository"` - } - - var data response - err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data) + err := c.gql.Query(repo.RepoHost(), "DiscussionWithComments", &query, variables) if err != nil { return nil, err } - if !data.Repository.HasDiscussionsEnabled { + if !query.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 + src := query.Repository.Discussion - mapActor := func(a actorJSON) DiscussionActor { - return DiscussionActor{ID: a.ID, Login: a.Login, Name: a.Name} - } + d := mapDiscussionFromListNode(src.discussionListNode) - 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 { - replyComments := make([]DiscussionComment, len(c.Replies.Nodes)) - for i, r := range c.Replies.Nodes { - replyComments[i] = 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), - } - } - dc.Replies = DiscussionCommentList{ - Comments: replyComments, - TotalCount: c.Replies.TotalCount, - Direction: DiscussionCommentListDirectionBackward, // Since we always fetch the last 4 replies - } - } - 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} + for _, rg := range src.ReactionGroups { + d.ReactionGroups = append(d.ReactionGroups, ReactionGroup{ + Content: rg.Content, + TotalCount: rg.Users.TotalCount, + }) } comments := make([]DiscussionComment, len(src.Comments.Nodes)) for i, c := range src.Comments.Nodes { - comments[i] = mapComment(c) + comments[i] = mapCommentFromNode(c) } // When using "last" (newest order), the API returns items in chronological From 5946d1a2981a6dcb865d408c20dbcc0d4ec75198 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 27 Apr 2026 09:13:36 +0100 Subject: [PATCH 17/17] chore(discussion/client): apply formatting Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 71890682f..b7fdf36b2 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -396,7 +396,7 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc // including nested replies. type discussionCommentNode struct { ID string - URL string `graphql:"url"` + URL string `graphql:"url"` Author actorNode Body string CreatedAt time.Time @@ -412,7 +412,7 @@ type discussionCommentNode struct { TotalCount int Nodes []struct { ID string - URL string `graphql:"url"` + URL string `graphql:"url"` Author actorNode Body string CreatedAt time.Time