Add `discussion list` command by maxbeizer · Pull Request #13084 · cli/cli

maxbeizer · 2026-04-02T18:00:23Z

Summary

Implements gh discussion list — the first user-facing command in the discussion group (PR 2 of 11).

Features

Listing: repository.discussions GraphQL query with state, category, answered, sort, and order filters
Search: Falls back to search(type: DISCUSSION) when --author, --label, or --search are provided (same dual-path pattern as gh issue list)
Pagination: --after cursor flag for paginating through results; next cursor in JSON output envelope
Category resolution: Matches --category by slug first, then name (case-insensitive). Errors with sorted available slugs on mismatch. Lives in shared/ for reuse by other commands.
TTY output: Colored table with ID, TITLE, CATEGORY, LABELS, ANSWERED (✓), UPDATED columns
Non-TTY output: Tab-separated with STATE as second field
JSON output: Envelope {"totalCount": N, "discussions": [...], "next": "<cursor>"} with --jq and --template support
Web mode: --web opens discussions in browser, populates search box when filters are present
Preview annotations: Top-level discussion and list commands marked as (preview)

Flags

  -A, --author string     Filter by author
  -c, --category string   Filter by category name or slug
  -l, --label strings     Filter by label
  -L, --limit int         Maximum number of discussions to fetch (default 30)
  -s, --state string      Filter by state: {open|closed|all} (default "open")
  -S, --search string     Search discussions with query
      --answered           Filter to answered discussions only
      --sort string        Sort by field: {created|updated} (default "updated")
      --order string       Order of results: {asc|desc} (default "desc")
      --after string       Cursor for the next page of results
  -w, --web               Open in browser
      --json fields        Output JSON with specified fields
  -q, --jq expression     Filter JSON output
  -t, --template string   Format JSON output

Client implementation

Implements List, Search, and ListCategories on the DiscussionClient:

List and Search accept after cursor param and return DiscussionListResult (discussions + total count + next cursor)
Guard clauses for limit <= 0
Domain-level consts for state (FilterStateOpen/FilterStateClosed), order-by (OrderByCreated/OrderByUpdated), and direction (OrderDirectionAsc/OrderDirectionDesc)
switch statements with default error clauses for all enum mappings (no strings.ToUpper)
Search uses qualifier/keyword terminology with %q quoting for whitespace safety
ListCategories checks hasDiscussionsEnabled and returns a clear error for repos with discussions disabled
Private API response types with json tags, mapped field-by-field to domain types

Tests

17 test cases covering TTY/non-TTY/JSON output (including next cursor), web mode, empty results, category resolution, category not found, author filter, label filter, search filter, after cursor, closed state, toFilterState helper, and flag parsing (including --sort, --order, --search, --after, and invalid values).

What's next

PR 3: discussion view (without comments)

Refs: #12810,

Add the discussion list command with full support for: - Listing discussions with state, category, answered, and order filters - Searching discussions by author and labels (uses Search API) - Category resolution by slug or name (case-insensitive) - TTY and non-TTY table output with colored IDs and labels - JSON output with totalCount envelope - Web mode (--web) to open discussions in browser - Pagination for large result sets Implement List, Search, and ListCategories methods on the DiscussionClient. List and Search use plain text GraphQL for flexible variable handling. ListCategories uses safely typed GraphQL via shurcooL/githubv4. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

babakks

Thanks for the PR, @maxbeizer! 🍻 It's great.

Note that some of these comments are best to be resolved in a follow up PR, as they'd break the base for the discussion view PR.

Summary of my comments (you can find the details in their corresponding comments):

Add tests for discussion client methods.
Add --search option.
Add --sort and --order options.
Add --after option for further pagination calls, and the next field in JSON output. We should basically do what we planned for pagination in discussion view (in #12810 (comment)).
Some domain-vs-API and domain-vs-CLI adaptions.
Question around having hasDiscussionsEnabled next to discussions; does that even work on repos where discussions are disabled.
A bit around --help docs; (preview) note, examples, and command usage format.

Further questions, but not ready to comment/share for now

Shouldn't ClosedAt and AnswerChosenAt fields be null-able?
What happens to ghost users in author.login query?
See if we can replace the plain text GQL query with a strongly-typed one.

babakks · 2026-04-13T09:26:59Z

@@ -19,12 +22,314 @@ func NewDiscussionClient(httpClient *http.Client) DiscussionClient {
 	}
 }


TBF in a follow-up.

Constructor functions, like NewDiscussionClient here, are expected to return a pointer to a struct/type and not an interface. I know here we can't just capitalise and export discussionClient struct as it would collide with DiscussionClient interface. So, my suggestion is to rename the interface to Discussions and then export the implementation as DiscussionClient. It's not perfect and could be seen as misleading, but I think I'm okay with it (TBF it's better than what we did in agent-task where the interface and the struct names are just different in casing).

Actually, the problem is we're defining both the interface and the implementation in the same package and that's why our hands are tied here. Interfaces make more sense to be defined at the consumer side (especially, with duck-typing, as in Go), but I prefer to keep them in the same package for consistency with other commands.

babakks · 2026-04-13T09:46:08Z

+	orderField := "UPDATED_AT"
+	orderDir := "DESC"
+	if filters.OrderBy != "" {
+		orderField = strings.ToUpper(filters.OrderBy) + "_AT"


Let's not rely on such string manipulations to transfer types/values from domain to API types. What we should do is to:

Define acceptable values for the domain OrderBY type as consts in types.go (no need to define a secondary string type, though).

Here, add a switch-statement to assign the orderField temp (no strings.* usage). The switch should have a default clause that returns an error. This is to prevent half-baked code changes in the future, if any.

babakks · 2026-04-13T09:49:57Z

+	if filters.Direction != "" {
+		orderDir = strings.ToUpper(filters.Direction)
+	}


The same comment here.

babakks · 2026-04-13T14:11:02Z

+	isTerminal := opts.IO.IsStdoutTTY()
+	if isTerminal {
+		title := listHeader(ghrepo.FullName(repo), len(discussions), totalCount, opts.State)
+		fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title)
+	}
+
+	printDiscussions(opts, discussions, totalCount)


I think we should move all this into printDiscussion, as it's the main decision maker on printing data.

babakks · 2026-04-13T14:22:36Z

+	remaining := totalCount - len(discussions)
+	if remaining > 0 {
+		fmt.Fprintf(opts.IO.Out, cs.Muted("And %d more\n"), remaining)
+	}


I know issue list has the same logic to print the remaining lines, but I think it never because both the caller always pass the length of the issues slice as the totalCount to the table renderer function. That's just a fortunate bug, IMO, because we never print that line, especially in non-TTY experience (where we should never print anything that machines cannot read).

Here, it's not a bug because we have a valid totalCount passed in, but we still shouldn't print it in non-TTY mode. So:

Suggested change

remaining := totalCount - len(discussions)

if remaining > 0 {

fmt.Fprintf(opts.IO.Out, cs.Muted("And %d more\n"), remaining)

}

if remaining := totalCount - len(discussions); isTerminal && remaining > 0 {

fmt.Fprintf(opts.IO.Out, cs.Muted("And %d more\n"), remaining)

}

babakks · 2026-04-13T14:26:22Z

+
+	var queryParts []string
+	if opts.State != "" && opts.State != "all" {
+		queryParts = append(queryParts, "is:"+opts.State)


The same interface-to-domain adaption should be done here for state.

babakks · 2026-04-13T14:27:32Z

+		queryParts = append(queryParts, "is:"+opts.State)
+	}
+	if opts.Author != "" {
+		queryParts = append(queryParts, "author:"+opts.Author)


Let's quote author value.

Suggested change

queryParts = append(queryParts, "author:"+opts.Author)

queryParts = append(queryParts, fmt.Sprintf("author:%q", opts.Author))

babakks · 2026-04-13T14:28:16Z

+	}
+
+	if len(queryParts) > 0 {
+		discussionsURL += "?" + url.Values{"q": {strings.Join(queryParts, " ")}}.Encode()


We should also include the --search value here, if any.

maxbeizer

Thanks for the thorough review, @babakks! Really appreciate the detail here. 🙏

I agree with the feedback across the board. Here's my plan for tackling it:

In this PR:

Domain/API type safety: *string for State, exported consts in types.go, switch statements with default error clauses, toFilterState() helper
Pagination: --after flag, DiscussionListResult return type with cursor, next in JSON envelope
Search: --search/-S flag, single keyword string, qualifier/keyword terminology, %q quoting
CLI flags: split --order-by into --sort + --order, (preview) annotations, Use: "list [flags]", defaultLimit const, additional examples
Code organization: category lookup → shared/, single-line errors, pageLimit → remaining, hoist hasDiscussionsEnabled out of loop
Guard clauses: limit <= 0 validation in client methods

In follow-up PRs:

Interface rename (DiscussionClient → Discussions) + export struct
Client method tests (following agent-task/capi/sessions_test.go pattern)

Will investigate:

hasDiscussionsEnabled + discussions query behavior on repos with discussions disabled

maxbeizer · 2026-04-13T15:19:33Z

Investigation result for the hasDiscussionsEnabled question (comment):

Querying a repo with discussions disabled (cli/go-gh) returns hasDiscussionsEnabled: false with discussions.totalCount: 0 — no GQL error. So our current approach of checking the boolean works correctly. The guard clause is valid as-is.

- Export domain consts (FilterStateOpen/Closed, OrderByCreated/Updated, OrderDirectionAsc/Desc) in types.go - Change State fields to *string (nil = all states) - Add DiscussionListResult type with NextCursor for pagination - Update List/Search signatures: add after param, return result type - Add limit <= 0 guard clauses in client methods - Replace strings.ToUpper/ToLower with switch statements + default errors - Rename pageLimit to remaining, hoist hasDiscussionsEnabled check - Use qualifier/keyword terminology in search query building - Quote author values with %q for whitespace safety - Add Keywords string field (single string, not []string) - Split --order into --sort {created|updated} and --order {asc|desc} - Add --search/-S and --after flags - Add next field in JSON output envelope - Extract defaultLimit const - Add toFilterState helper for CLI-to-domain mapping - Move matchCategory to shared/categories.go with godoc - Use single-line error messages with sorted slugs - Add (preview) annotations to Short docs - Update Use to "list [flags]" - Add examples for --answered=false, --state all, multi-label - Update tests for new signatures and new flags Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

babakks · 2026-04-13T15:35:57Z

Querying a repo with discussions disabled (cli/go-gh) returns hasDiscussionsEnabled: false with discussions.totalCount: 0 — no GQL error. So our current approach of checking the boolean works correctly. The guard clause is valid as-is.

There's one more case when we fetch the categories, too, and that happens before the actual list query.

maxbeizer · 2026-04-13T15:38:10Z

Good call! I checked — ListCategories on a repo with discussions disabled returns an empty list (no GQL error), so the user would get unknown category: "foo"; must be one of [] which is confusing.

I think the cleanest fix is to add hasDiscussionsEnabled to the ListCategories query and return the "discussions disabled" error from there too. That way any code path that calls ListCategories gets the right error. I'll add that in the next push.

When --category is used, ListCategories runs before the List query. On repos with discussions disabled, it silently returns empty categories, leading to a confusing "must be one of []" error. Now it checks hasDiscussionsEnabled and returns the standard "discussions disabled" error, matching the behavior of List and Search. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The GraphQL Discussion type has a `closed` boolean field, not a `state` string. Updated the API response struct and GQL fragment to query `closed` instead of `state`, and derive the domain-level State string ("OPEN"/"CLOSED") from the boolean in mapDiscussion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Signed-off-by: Babak K. Shandiz <babakks@github.com>

…tion Signed-off-by: Babak K. Shandiz <babakks@github.com>

Signed-off-by: Babak K. Shandiz <babakks@github.com>

…ons` Signed-off-by: Babak K. Shandiz <babakks@github.com>

Signed-off-by: Babak K. Shandiz <babakks@github.com>

babakks · 2026-04-14T14:09:48Z

@maxbeizer, I went through the code and applied the below changes. I think it's good to be merged now. Please let me know what do you think! 🍻

Replaced State string with Closed bool in the domain Discussion type
Renamed DiscussionAuthor to DiscussionActor (covers both author and answerChosenBy)
Updated GQL fragments to fetch author/answerChosenBy actor fields (id, name via ...on User/...on Bot)
Created discussionListNode (with graphql tags) separate from discussionNode, scoped to list/search fields only
Converted both List and Search from raw GQL strings (GraphQL()) to strongly-typed struct queries (Query())
Removed discussionNode, mapDiscussion, and discussionFields (now dead code)
Simplified List query to declare all optional parameters upfront (no dynamic query building)
Changed List/Search return types to *DiscussionListResult
Created local discussionListFields in the list package (excludes comments)
Removed reactionGroups from list/search types and fields
Replaced cs.Gray with cs.Muted for closed discussion IDs
Moved header printing into printDiscussions
Inlined messages in noResults and listHeader
Quoted author qualifier in web URL
Included --search keywords in web URL

github-actions Bot added external pull request originating outside of the CLI core team needs-triage needs to be reviewed labels Apr 2, 2026

maxbeizer force-pushed the discussion-list branch from b23bc28 to 0f22710 Compare April 2, 2026 18:30

maxbeizer force-pushed the discussion-list branch from 0f22710 to 90449b5 Compare April 3, 2026 17:46

maxbeizer marked this pull request as ready for review April 3, 2026 17:47

maxbeizer requested a review from a team as a code owner April 3, 2026 17:47

maxbeizer requested review from babakks and removed request for a team April 3, 2026 17:47

maxbeizer mentioned this pull request Apr 3, 2026

Add discussion view command #13093

Closed

babakks requested changes Apr 13, 2026

View reviewed changes

maxbeizer commented Apr 13, 2026

View reviewed changes

maxbeizer and others added 2 commits April 13, 2026 10:20

Fix gofmt alignment in test file

a08f5f2

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

maxbeizer and others added 2 commits April 13, 2026 10:38

maxbeizer mentioned this pull request Apr 13, 2026

Add acceptance tests for discussion list #13151

Closed

babakks added 10 commits April 14, 2026 11:45

fix(discussion list): use separate list of fields

3f52503

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion list): replace state with closed in domain types

236224d

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion/client): fetch body in list/search

15d3eea

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion/client): fetch author/answerChosenBy fields

034e38f

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion/client): simplify list query

e6befd5

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion/client): change list return type to pointer

2a46a9d

Signed-off-by: Babak K. Shandiz <babakks@github.com>

refactor(discussion/client): use strongly-typed query for search func…

9afbd61

…tion Signed-off-by: Babak K. Shandiz <babakks@github.com>

refactor(discussion/client): use strongly-typed query for list function

e403e82

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion/shared): print quoted category slugs

afb1b7d

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion/shared): deleted unused fields slice

9a42e90

Signed-off-by: Babak K. Shandiz <babakks@github.com>

babakks added 6 commits April 14, 2026 14:50

fix(discussion/client): remove reaction groups from list/search types

835d7f3

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion list): only print remaining items in tty mode

2d1b1a7

Signed-off-by: Babak K. Shandiz <babakks@github.com>

refactor(discussion list): inline printed messages

d45ce94

Signed-off-by: Babak K. Shandiz <babakks@github.com>

refactor(discussion list): encapsulate printing within `printDiscussi…

8d70cc9

…ons` Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion list): quote author qualifier in web mode

f62875f

Signed-off-by: Babak K. Shandiz <babakks@github.com>

fix(discussion list): include search keywords in web mode

2a4a982

Signed-off-by: Babak K. Shandiz <babakks@github.com>

babakks merged commit 2bb24f9 into cli:feature/discussion Apr 15, 2026
7 of 8 checks passed

maxbeizer mentioned this pull request Apr 21, 2026

test(discussion): add httpmock unit tests for DiscussionClient #13252

Merged

maxbeizer mentioned this pull request Jun 3, 2026

feat: add discussion command set #13541

Open

		@@ -19,12 +22,314 @@ func NewDiscussionClient(httpClient *http.Client) DiscussionClient {
		}
		}

	queryParts = append(queryParts, "author:"+opts.Author)
	queryParts = append(queryParts, fmt.Sprintf("author:%q", opts.Author))

Conversation

maxbeizer commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Features

Flags

Client implementation

Tests

What's next

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Further questions, but not ready to comment/share for now

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxbeizer left a comment

Choose a reason for hiding this comment

Uh oh!

maxbeizer commented Apr 13, 2026

Uh oh!

babakks commented Apr 13, 2026

Uh oh!

maxbeizer commented Apr 13, 2026

Uh oh!

babakks commented Apr 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maxbeizer commented Apr 2, 2026 •

edited

Loading