feat: add discussion command set#13541
Conversation
Implement a GraphQL client for GitHub Discussions supporting: - List and search discussions with pagination - Get discussion by number with comments - Get paginated comment replies - List repository categories and labels - Create discussions with optional labels - Update discussion title, body, category, and labels - Add/remove labels via separate mutations Includes comprehensive table-driven tests with HTTP mocking. Co-authored-by: Max Beizer <max.beizer@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add shared package with: - ParseDiscussionArg: parses discussion number or URL from CLI args - MatchCategory: case-insensitive category matching by name or slug - ResolveLabels: case-insensitive label name to ID resolution - DiscussionClientFunc: factory helper for lazy client initialization Includes tests for all utilities. Co-authored-by: Max Beizer <max.beizer@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add `gh discussion list` with support for: - Filtering by category, state, author, and labels - Search keyword matching - Table and JSON output formats - Web mode (opens browser with filters) - Pagination via --limit flag Also adds the discussion command group scaffolding and root wiring. Co-authored-by: Max Beizer <max.beizer@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add `gh discussion view` with support for: - Viewing discussion details with metadata and body - Threaded comment display with --comments flag - Paginated reply fetching with --replies flag - JSON output with --json/--jq/--template flags - Web mode (opens browser) - Accepts discussion number or URL as argument Co-authored-by: Max Beizer <max.beizer@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add `gh discussion create` with support for: - Non-interactive creation with --title, --body, --category, --label flags - Interactive mode with prompts for title, body, and category - Body input from file via --body-file (including stdin) - Label resolution (name to ID) before creation - Category validation by name or slug Co-authored-by: Max Beizer <max.beizer@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add `gh discussion edit` with support for: - Non-interactive editing with --title, --body, --body-file, --category flags - Label management with --add-label and --remove-label flags - Interactive mode with multi-select prompts for fields to edit - Flag presence detection (allows setting body to empty string) - Label name to ID resolution via shared.ResolveLabels - Body input from stdin via --body-file - Co-authored-by: Max Beizer <max.beizer@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b6d5f84 to
797effe
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new preview gh discussion command group to the GitHub CLI, backed by a dedicated Discussions GraphQL client and shared helpers, with support for human-readable and JSON output modes.
Changes:
- Adds the
gh discussioncommand group and wires it into the CLI root command. - Implements
discussion list,discussion view,discussion create, anddiscussion editcommands (including interactive flows and JSON output). - Adds a Discussions GraphQL client, shared argument/category/label utilities, and accompanying unit tests.
Show a summary per file
| File | Description |
|---|---|
| pkg/cmd/root/root.go | Registers the new discussion command group at the root level. |
| pkg/cmd/discussion/discussion.go | Defines the top-level gh discussion command and groups subcommands. |
| pkg/cmd/discussion/list/list.go | Implements listing discussions with filters, web mode, and JSON output. |
| pkg/cmd/discussion/list/list_test.go | Tests list behavior across tty/non-tty/json/web and filters. |
| pkg/cmd/discussion/view/view.go | Implements viewing discussions, including comments and comment-reply pagination. |
| pkg/cmd/discussion/view/view_test.go | Tests view output (tty/raw/json) and pagination for comments/replies. |
| pkg/cmd/discussion/create/create.go | Implements discussion creation (interactive or via flags) with label resolution. |
| pkg/cmd/discussion/create/create_test.go | Tests create flows, validation, prompting, and error handling. |
| pkg/cmd/discussion/edit/edit.go | Implements discussion editing (interactive or via flags), including label add/remove. |
| pkg/cmd/discussion/edit/edit_test.go | Tests edit flows, validation, prompting, and mutation inputs. |
| pkg/cmd/discussion/shared/client.go | Adds a factory helper for creating a Discussions client from Factory. |
| pkg/cmd/discussion/shared/lookup.go | Parses discussion arguments (number / URL) into number + repo override. |
| pkg/cmd/discussion/shared/lookup_test.go | Tests discussion argument parsing. |
| pkg/cmd/discussion/shared/categories.go | Adds category name/slug matching helper. |
| pkg/cmd/discussion/shared/labels.go | Adds label-name-to-ID resolution helper. |
| pkg/cmd/discussion/shared/labels_test.go | Tests label resolution behavior. |
| pkg/cmd/discussion/client/client.go | Defines the Discussions client interface. |
| pkg/cmd/discussion/client/client_impl.go | Implements Discussions GraphQL operations (list/search/get/create/update, pagination). |
| pkg/cmd/discussion/client/types.go | Defines domain types + ExportData mappings for --json output. |
| pkg/cmd/discussion/client/client_mock.go | Adds generated mock for testing (moq). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Files not reviewed (1)
- pkg/cmd/discussion/client/client_mock.go: Language not supported
- Files reviewed: 19/21 changed files
- Comments generated: 10
BagToad
left a comment
There was a problem hiding this comment.
There's some code scanning alerts that should be fixed.
Looks like code scanning doesn't block right now, so I want to just formally block on those code scanning alerts just in case.
maxbeizer
left a comment
There was a problem hiding this comment.
One thing I noticed with discussion list --json: I know we previously discussed adding pagination metadata like next (e.g. #13084 (comment)), but the current command advertises discussion fields like number,title,url while returning a top-level envelope:
{ "totalCount": 2, "discussions": [
{ "number": 42, "title": "..." } ], "next": "..."
}That makes sense for pagination, but it seems a little inconsistent with the usual gh <resource> list --json field1,field2 contract, where selected resource fields are emitted directly as an array. It also means totalCount, discussions, and next are always emitted even though they are not selectable JSON fields.
Should we either align this with issue list / pr list and emit just the discussion array, or explicitly make the envelope fields part of the supported/documented JSON shape?
- Fix double default in list --limit help text - Wire --body-file flag in create command (symmetric with edit) - Cap comment/reply pagination to maxPageSize (100) - Use int32 for discussion number params (fixes CodeQL alerts) - Use strconv.ParseInt instead of Atoi for int32 targets - Fix IsStdoutTTY -> IsStderrTTY for web mode message - Use CancelError instead of error string for no-op edit - Trim whitespace from label names in resolution - Fix category error formatting (comma-joined instead of %q slice) - Fix typo in lookup.go comment - Remove dead len==0 check in view replies path - Inline exporterNeedsComments into needsComments - Validate comment ownership in GetCommentReplies - Remove unimplemented interface methods and CloseReason type Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@maxbeizer, that's actually intentional because the top-level array JSON in |
Signed-off-by: Babak K. Shandiz <babakks@github.com>
… command Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
oh that's right. I remember now. Thank you! |
Return the discussion alongside the error when the primary mutation succeeds but a secondary mutation (e.g., labels) fails. Callers print the URL on stdout and the error on stderr, then exit with code 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rewrite the help text to be more conversational and less mechanical. Clarify that inline reply previews are not affected by --order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…stResult Track the input pagination cursor on DiscussionListResult and add an ExportData method that conditionally includes cursor/next fields. Use ExportData in the list command instead of manually building the JSON envelope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rewrite list command tests as a single table-driven TestListRun with exact output matching, inline exporter setup, and browser.Verify. Add TestListJSONFields and TestNewCmdList for field coverage and flag parsing. Add test cases for JSON cursor/next fields, all no-results states, and search path with cursor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@BagToad, I just pushed a batch of changes addressing review feedback: Fixes:
Improvements:
Tests:
Deferred:
|
Signed-off-by: Babak K. Shandiz <babakks@github.com>
| @@ -0,0 +1,1077 @@ | |||
| package client | |||
There was a problem hiding this comment.
client_impl.go is the only *_impl.go file in cli/cli. Other client implementations (e.g. api/client.go, pkg/cmd/attestation/api/client.go) put the interface and concrete type together in client.go, or name the file by what it is (graphql_client.go) rather than by role.
Since this package has one implementation, merging into client.go reads most naturally.
There was a problem hiding this comment.
Yeah, this is a left over from the start. I'll fix this one at the end.
| cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Filter by label") | ||
| cmdutil.StringEnumFlag(cmd, &opts.State, "state", "s", stateOpen, []string{stateOpen, stateClosed, stateAll}, "Filter by state") | ||
| cmd.Flags().IntVarP(&opts.Limit, "limit", "L", defaultLimit, "Maximum number of discussions to fetch") | ||
| cmdutil.NilBoolFlag(cmd, &opts.Answered, "answered", "", "Filter by answered state") |
There was a problem hiding this comment.
thought: might be worth documenting that --answered=false only matches Q&A discussions (it maps to is:unanswered, which is Q&A-only). Non-Q&A discussions silently disappear from the result set when this is set.
There was a problem hiding this comment.
It's already covered in examples:
# List unanswered discussions as JSON
$ gh discussion list --answered=false --json number,title,url
There was a problem hiding this comment.
As we talked sync, let's rephrase the example titles:
# List answered Q&A discussions as JSON
$ gh discussion list --answered --json number,title,url
# List unanswered Q&A discussions as JSON
$ gh discussion list --answered=false --json number,title,url
The original TestNewCmdList was removed during test consolidation, so the temporary suffix is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stop wrapping ListCategories errors with "fetching categories:" in create and edit commands. The underlying error (e.g., "discussions disabled") is already descriptive and now surfaces consistently across all commands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The field was set but never read; Client is the only HTTP path used. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
| if !strings.EqualFold(src.Discussion.Repository.Owner.Login, repo.RepoOwner()) || | ||
| !strings.EqualFold(src.Discussion.Repository.Name, repo.RepoName()) { | ||
| return nil, fmt.Errorf("comment %s does not belong to %s/%s", commentID, repo.RepoOwner(), repo.RepoName()) | ||
| } |
There was a problem hiding this comment.
src.Discussion.Number is queried at line 630 but never compared against the number argument. The repo check above guards cross-repo confusion; a parallel number check would round out the validation. Passing a --replies <commentID> whose comment lives on a different discussion in the same repo currently renders the requested discussion's header with the foreign comment underneath, and JSON output attributes the comment to the requested discussion number. Cheap to add next to the existing check.
| return 0, nil, fmt.Errorf("invalid discussion URL: %q", arg) | ||
| } | ||
|
|
||
| num, _ := strconv.ParseInt(m[3], 10, 32) |
There was a problem hiding this comment.
Two of the three paths above check the ParseInt error; this one discards it. On overflow ParseInt returns math.MaxInt32 with ErrRange, so a URL ending in /discussions/9999999999 silently becomes discussion 2147483647 and produces a confusing not-found error. Worth handling the error for consistency with lines 17 and 22.
| if showComments { | ||
| for _, c := range d.Comments.Comments { | ||
| printRawComment(out, c, "") | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Thanks to @BagToad for calling this out, this is actually not consistent with other commands like issue view --comments (in non-tty mode). We tend to print only the comments in non-tty mode, and this one should be fixed.
The same for replies mode. When we're printing the replies, we should just print the replies (not the parent comment or the discussion itself).
Also, the output should be a flat list of comments (or replies), each with their own header fields. Currently we print a long tab-separated line per each comment/reply, which is not the consistent way to do this.
Add client methods for managing discussion comments and replies: - AddComment: creates a top-level comment or reply via addDiscussionComment - UpdateComment: edits a comment body via updateDiscussionComment - DeleteComment: removes a comment via deleteDiscussionComment - GetComment: fetches a comment by node ID with typename validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement gh discussion comment for adding, editing, and deleting comments and replies on discussions. Supports --body, --body-file, --editor for input, --reply-to for threading, --edit and --delete for modifying existing comments, with confirmation prompts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…PI calls Replace gh api graphql addDiscussionComment mutations in view and list acceptance tests with gh discussion comment. Extract comment IDs via a single view call with jq2env instead of multiple calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover add, reply, edit, delete flows including body-file input, non-interactive delete requiring --yes, and deletion of both parent comment and reply to avoid ghost entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add test for --delete without --yes in non-TTY mode and fix existing delete tests to set isTTY: true for prompter-based confirmation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add ParseDiscussionOrCommentArg to the shared package for parsing discussion numbers, URLs, comment node IDs, and comment URLs (with #discussioncomment-NNNNN fragments). Add ResolveCommentNodeID to the client for constructing node IDs from repository database ID and comment database ID via msgpack encoding. Update the comment command to accept comment URLs as positional args for reply, edit, and delete operations. Update the view command to accept comment URLs in the --replies flag. Add DiscussionID field to DiscussionComment for reply flow (fetches the parent discussion ID from a comment). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ests Update comment acceptance test to use positional comment IDs instead of --reply-to, and add URL-based add/edit/delete test cases. Update view acceptance test to cover --replies with a comment URL. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
…ment argument Drop the --replies flag from gh discussion view and instead accept a comment URL or comment node ID (DC_*) as the positional argument. A discussion number, #number, or discussion URL behaves as before, while a comment reference shows that comment and its replies. Rework GetCommentReplies to be node-centric: it resolves the parent discussion directly from the comment node, so it no longer needs the discussion number or a repository ownership check. The method now takes a host string instead of a full repository. Also add a comment mode for --web that resolves and opens the comment URL, and add godoc to the DiscussionClient interface methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetComment resolves a comment by its node ID, so it only needs the host to select the GraphQL endpoint. Change its signature from a full ghrepo.Interface to a host string and update all callers and tests accordingly, mirroring GetCommentReplies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…test Replace --replies usage with a positional comment node ID or URL to match the redesigned gh discussion view command. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…gument In replies mode the --comments flag is meaningless, so reject the combination with a flag error instead of silently ignoring it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
feat(discussion): add comment command
This PR adds the
gh discussioncommand group (list, view, create, edit) as a preview feature.Huge thanks to @maxbeizer for doing the heavy lifting on this.
was also instrumental in implementation, review, and polish throughout.
I rewrote the commit history (81 commits squashed into 6) for easier review and a cleaner git log. Note that the commits are structured to split the work logically; intermediate commits may not compile independently.
Commands
discussion listList discussions in a repository with optional filters. (8b73951)
discussion viewView a discussion's details, comments, or replies to a specific comment. (57008e7)
discussion createCreate a new discussion, interactively or via flags. (7bd67a8)
discussion editEdit a discussion's title, body, category, or labels, interactively or via flags. (797effe)
Reusable components
pkg/cmd/discussion/client: GraphQL client for Discussions covering list, search, get, create, update, and label mutations with full pagination support. (51b7653)pkg/cmd/discussion/shared: Shared utilities including argument parsing, category matching, label resolution, and client factory helpers. (d3a1538)Screenshots
gh discussion listgh discussion view 9094gh discussion view 9094 --comments --limit 1gh discussion view 9094 --replies DC_kwDODKw3uc4AkHLS --limit 1gh discussion create(non-interactive)gh discussion create(interactive, category selection)gh discussion create(interactive)gh discussion edit(interactive)Select fields to update

Populate new values

Submit

gh discussion edit(non-interactive)