From c8925def4e6a97cbb3db09e24525aac4e7be34d8 Mon Sep 17 00:00:00 2001 From: dhernando Date: Thu, 9 Apr 2026 14:08:40 +0200 Subject: [PATCH 1/3] feat: use OutputTable with --no-headers support in all list commands --- internal/cmd/backup/list.go | 6 +++--- internal/cmd/backup/restore_list.go | 6 +++--- internal/cmd/backup/schedule_list.go | 6 +++--- internal/cmd/base/list.go | 30 ++++++++++++---------------- internal/cmd/base/list_test.go | 26 +++++------------------- internal/cmd/cloudprovider/list.go | 4 ++-- internal/cmd/cloudregion/list.go | 6 +++--- internal/cmd/cluster/list.go | 9 +++------ internal/cmd/cluster/list_test.go | 18 +---------------- internal/cmd/cluster/version.go | 6 +++--- internal/cmd/hybrid/list.go | 6 +++--- internal/cmd/iam/key_list.go | 6 +++--- internal/cmd/package/package.go | 6 +++--- 13 files changed, 48 insertions(+), 87 deletions(-) diff --git a/internal/cmd/backup/list.go b/internal/cmd/backup/list.go index c84b74f..de1e9b9 100644 --- a/internal/cmd/backup/list.go +++ b/internal/cmd/backup/list.go @@ -42,7 +42,7 @@ func newListCommand(s *state.State) *cobra.Command { } return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *backupv1.ListBackupsResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *backupv1.ListBackupsResponse) (output.TableRenderer, error) { t := output.NewTable[*backupv1.Backup](w) t.AddField("ID", func(v *backupv1.Backup) string { return v.GetId() @@ -62,8 +62,8 @@ func newListCommand(s *state.State) *cobra.Command { } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) diff --git a/internal/cmd/backup/restore_list.go b/internal/cmd/backup/restore_list.go index 37613c7..8c4550c 100644 --- a/internal/cmd/backup/restore_list.go +++ b/internal/cmd/backup/restore_list.go @@ -42,7 +42,7 @@ func newRestoreListCommand(s *state.State) *cobra.Command { } return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *backupv1.ListBackupRestoresResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *backupv1.ListBackupRestoresResponse) (output.TableRenderer, error) { t := output.NewTable[*backupv1.BackupRestore](w) t.AddField("ID", func(v *backupv1.BackupRestore) string { return v.GetId() @@ -62,8 +62,8 @@ func newRestoreListCommand(s *state.State) *cobra.Command { } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) diff --git a/internal/cmd/backup/schedule_list.go b/internal/cmd/backup/schedule_list.go index 11b541f..b89ab21 100644 --- a/internal/cmd/backup/schedule_list.go +++ b/internal/cmd/backup/schedule_list.go @@ -42,7 +42,7 @@ func newScheduleListCommand(s *state.State) *cobra.Command { } return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *backupv1.ListBackupSchedulesResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *backupv1.ListBackupSchedulesResponse) (output.TableRenderer, error) { t := output.NewTable[*backupv1.BackupSchedule](w) t.AddField("ID", func(v *backupv1.BackupSchedule) string { return v.GetId() @@ -68,8 +68,8 @@ func newScheduleListCommand(s *state.State) *cobra.Command { } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) diff --git a/internal/cmd/base/list.go b/internal/cmd/base/list.go index a305628..b161219 100644 --- a/internal/cmd/base/list.go +++ b/internal/cmd/base/list.go @@ -12,17 +12,15 @@ import ( // ListCmd defines a command for fetching and displaying a list response. // T is the full response proto message (e.g. *clusterv1.ListClustersResponse). // -// For table output, prefer OutputTable over PrintText. When OutputTable is set, -// the base automatically registers --no-headers and handles header suppression. -// PrintText is used as a fallback when OutputTable is not set. +// OutputTable must be set. The base automatically registers --no-headers and +// handles header suppression. type ListCmd[T any] struct { Use string Short string Long string Example string Fetch func(s *state.State, cmd *cobra.Command) (T, error) - OutputTable func(cmd *cobra.Command, out io.Writer, resp T) output.TableRenderer - PrintText func(cmd *cobra.Command, out io.Writer, resp T) error + OutputTable func(cmd *cobra.Command, out io.Writer, resp T) (output.TableRenderer, error) ValidArgsFunction func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) } @@ -42,22 +40,20 @@ func (lc ListCmd[T]) CobraCommand(s *state.State) *cobra.Command { if s.Config.JSONOutput() { return output.PrintJSON(cmd.OutOrStdout(), resp) } - if lc.OutputTable != nil { - r := lc.OutputTable(cmd, cmd.OutOrStdout(), resp) - noHeaders, _ := cmd.Flags().GetBool("no-headers") - r.SetNoHeaders(noHeaders) - r.Render() - return nil + if lc.OutputTable == nil { + panic("ListCmd: OutputTable must be set") } - if lc.PrintText == nil { - panic("ListCmd: either OutputTable or PrintText must be set") + r, err := lc.OutputTable(cmd, cmd.OutOrStdout(), resp) + if err != nil { + return err } - return lc.PrintText(cmd, cmd.OutOrStdout(), resp) + noHeaders, _ := cmd.Flags().GetBool("no-headers") + r.SetNoHeaders(noHeaders) + r.Render() + return nil }, } - if lc.OutputTable != nil { - cmd.Flags().Bool("no-headers", false, "Do not print column headers") - } + cmd.Flags().Bool("no-headers", false, "Do not print column headers") if lc.ValidArgsFunction != nil { cmd.ValidArgsFunction = lc.ValidArgsFunction } diff --git a/internal/cmd/base/list_test.go b/internal/cmd/base/list_test.go index 3724fe0..99caa17 100644 --- a/internal/cmd/base/list_test.go +++ b/internal/cmd/base/list_test.go @@ -2,7 +2,6 @@ package base_test import ( "bytes" - "fmt" "io" "testing" @@ -38,8 +37,8 @@ func TestListCmd_OutputTable(t *testing.T) { lc := base.ListCmd[string]{ Use: "test", Fetch: fetchHello, - OutputTable: func(_ *cobra.Command, out io.Writer, resp string) output.TableRenderer { - return stringTableRenderer(out, resp) + OutputTable: func(_ *cobra.Command, out io.Writer, resp string) (output.TableRenderer, error) { + return stringTableRenderer(out, resp), nil }, } @@ -53,8 +52,8 @@ func TestListCmd_OutputTable_NoHeaders(t *testing.T) { lc := base.ListCmd[string]{ Use: "test", Fetch: fetchHello, - OutputTable: func(_ *cobra.Command, out io.Writer, resp string) output.TableRenderer { - return stringTableRenderer(out, resp) + OutputTable: func(_ *cobra.Command, out io.Writer, resp string) (output.TableRenderer, error) { + return stringTableRenderer(out, resp), nil }, } @@ -64,22 +63,7 @@ func TestListCmd_OutputTable_NoHeaders(t *testing.T) { assert.Contains(t, stdout, "hello") } -func TestListCmd_PrintText(t *testing.T) { - lc := base.ListCmd[string]{ - Use: "test", - Fetch: fetchHello, - PrintText: func(_ *cobra.Command, out io.Writer, resp string) error { - _, err := fmt.Fprintln(out, resp) - return err - }, - } - - stdout, err := execListCmd(t, lc) - require.NoError(t, err) - assert.Contains(t, stdout, "hello") -} - -func TestListCmd_NeitherOutputTableNorPrintText_Panics(t *testing.T) { +func TestListCmd_NoOutputTable_Panics(t *testing.T) { lc := base.ListCmd[string]{ Use: "test", Fetch: fetchHello, diff --git a/internal/cmd/cloudprovider/list.go b/internal/cmd/cloudprovider/list.go index 8b0dc3d..6f18f4c 100644 --- a/internal/cmd/cloudprovider/list.go +++ b/internal/cmd/cloudprovider/list.go @@ -39,7 +39,7 @@ func newListCommand(s *state.State) *cobra.Command { return resp, nil }, - OutputTable: func(_ *cobra.Command, w io.Writer, resp *platformv1.ListCloudProvidersResponse) output.TableRenderer { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *platformv1.ListCloudProvidersResponse) (output.TableRenderer, error) { t := output.NewTable[*platformv1.CloudProvider](w) t.AddField("ID", func(v *platformv1.CloudProvider) string { return v.GetId() @@ -51,7 +51,7 @@ func newListCommand(s *state.State) *cobra.Command { return strconv.FormatBool(v.GetAvailable()) }) t.SetItems(resp.GetItems()) - return t + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/cloudregion/list.go b/internal/cmd/cloudregion/list.go index 49c9a42..91c8b71 100644 --- a/internal/cmd/cloudregion/list.go +++ b/internal/cmd/cloudregion/list.go @@ -43,7 +43,7 @@ func newListCommand(s *state.State) *cobra.Command { return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *platformv1.ListCloudProviderRegionsResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *platformv1.ListCloudProviderRegionsResponse) (output.TableRenderer, error) { t := output.NewTable[*platformv1.CloudProviderRegion](w) t.AddField("ID", func(v *platformv1.CloudProviderRegion) string { return v.GetId() @@ -57,8 +57,8 @@ func newListCommand(s *state.State) *cobra.Command { t.AddField("AVAILABLE", func(v *platformv1.CloudProviderRegion) string { return strconv.FormatBool(v.GetAvailable()) }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) diff --git a/internal/cmd/cluster/list.go b/internal/cmd/cluster/list.go index 12b2416..1112862 100644 --- a/internal/cmd/cluster/list.go +++ b/internal/cmd/cluster/list.go @@ -104,7 +104,7 @@ qcloud cluster list --page-size 10`, } return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *clusterv1.ListClustersResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *clusterv1.ListClustersResponse) (output.TableRenderer, error) { t := output.NewTable[*clusterv1.Cluster](w) t.AddField("ID", func(v *clusterv1.Cluster) string { return v.GetId() @@ -136,11 +136,8 @@ qcloud cluster list --page-size 10`, } return "" }) - t.Write(resp.Items) - if resp.NextPageToken != nil && *resp.NextPageToken != "" { - fmt.Fprintf(w, "Next page token: %s\n", *resp.NextPageToken) - } - return nil + t.SetItems(resp.Items) + return t, nil }, }.CobraCommand(s) diff --git a/internal/cmd/cluster/list_test.go b/internal/cmd/cluster/list_test.go index a3f0843..89d933f 100644 --- a/internal/cmd/cluster/list_test.go +++ b/internal/cmd/cluster/list_test.go @@ -167,7 +167,7 @@ func TestListClusters_PageSizeFlagSingleRequest(t *testing.T) { NextPageToken: &token, }, nil) - stdout, _, err := testutil.Exec(t, env, "cluster", "list", "--page-size", "1") + _, _, err := testutil.Exec(t, env, "cluster", "list", "--page-size", "1") require.NoError(t, err) assert.Equal(t, 1, env.Server.ListClustersCalls.Count()) @@ -175,7 +175,6 @@ func TestListClusters_PageSizeFlagSingleRequest(t *testing.T) { require.True(t, ok) require.NotNil(t, req.PageSize) assert.Equal(t, int32(1), *req.PageSize) - assert.Contains(t, stdout, "Next page token: next-token") } func TestListClusters_PageTokenFlagSingleRequest(t *testing.T) { @@ -225,18 +224,3 @@ func TestListClusters_CloudRegionFilter(t *testing.T) { require.NotNil(t, req.CloudProviderRegionId) assert.Equal(t, "us-east-1", *req.CloudProviderRegionId) } - -func TestListClusters_NextPageTokenPrintedAsFooter(t *testing.T) { - env := testutil.NewTestEnv(t) - - token := "footer-token" - env.Server.ListClustersCalls.Returns(&clusterv1.ListClustersResponse{ - Items: []*clusterv1.Cluster{{Id: "cluster-1"}}, - NextPageToken: &token, - }, nil) - - stdout, _, err := testutil.Exec(t, env, "cluster", "list", "--page-size", "1") - require.NoError(t, err) - - assert.Contains(t, stdout, "Next page token: footer-token") -} diff --git a/internal/cmd/cluster/version.go b/internal/cmd/cluster/version.go index 6997db7..1b713b7 100644 --- a/internal/cmd/cluster/version.go +++ b/internal/cmd/cluster/version.go @@ -47,15 +47,15 @@ qcloud cluster version list`, } return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *clusterv1.ListQdrantReleasesResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *clusterv1.ListQdrantReleasesResponse) (output.TableRenderer, error) { t := output.NewTable[*clusterv1.QdrantRelease](w) t.AddField("VERSION", func(r *clusterv1.QdrantRelease) string { return r.GetVersion() }) t.AddField("DEFAULT", func(r *clusterv1.QdrantRelease) string { return output.BoolYesNo(r.GetDefault()) }) t.AddField("END OF LIFE", func(r *clusterv1.QdrantRelease) string { return output.BoolMark(r.GetEndOfLife()) }) t.AddField("UNAVAILABLE", func(r *clusterv1.QdrantRelease) string { return output.BoolMark(r.GetUnavailable()) }) t.AddField("REMARKS", func(r *clusterv1.QdrantRelease) string { return r.GetRemarks() }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/hybrid/list.go b/internal/cmd/hybrid/list.go index f86af1d..b55c7de 100644 --- a/internal/cmd/hybrid/list.go +++ b/internal/cmd/hybrid/list.go @@ -37,7 +37,7 @@ func newListCommand(s *state.State) *cobra.Command { } return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *hybridv1.ListHybridCloudEnvironmentsResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *hybridv1.ListHybridCloudEnvironmentsResponse) (output.TableRenderer, error) { t := output.NewTable[*hybridv1.HybridCloudEnvironment](w) t.AddField("ID", func(v *hybridv1.HybridCloudEnvironment) string { return v.GetId() @@ -63,8 +63,8 @@ func newListCommand(s *state.State) *cobra.Command { } return "" }) - t.Write(resp.Items) - return nil + t.SetItems(resp.Items) + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/iam/key_list.go b/internal/cmd/iam/key_list.go index 4969264..2ef051f 100644 --- a/internal/cmd/iam/key_list.go +++ b/internal/cmd/iam/key_list.go @@ -42,7 +42,7 @@ qcloud iam key list --json`, AccountId: accountID, }) }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *authv1.ListManagementKeysResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *authv1.ListManagementKeysResponse) (output.TableRenderer, error) { t := output.NewTable[*authv1.ManagementKey](w) t.AddField("ID", func(v *authv1.ManagementKey) string { return v.GetId() @@ -56,8 +56,8 @@ qcloud iam key list --json`, } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/package/package.go b/internal/cmd/package/package.go index 4968167..2dae9eb 100644 --- a/internal/cmd/package/package.go +++ b/internal/cmd/package/package.go @@ -72,7 +72,7 @@ qcloud package list --cloud-provider hybrid`, return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *bookingv1.ListPackagesResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *bookingv1.ListPackagesResponse) (output.TableRenderer, error) { t := output.NewTable[*bookingv1.Package](w) t.AddField("NAME", func(p *bookingv1.Package) string { return p.GetName() @@ -115,8 +115,8 @@ qcloud package list --cloud-provider hybrid`, t.AddField("PRICE/HR", func(p *bookingv1.Package) string { return output.FormatMillicents(p.GetUnitIntPricePerHour(), p.GetCurrency()) }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) From f3a81a3dc8d9f2c19c82eb9db240b619f6b472e2 Mon Sep 17 00:00:00 2001 From: dhernando Date: Fri, 10 Apr 2026 09:41:02 +0200 Subject: [PATCH 2/3] fix: bring back PrintText so that parallel work doesn't fail --- internal/cmd/base/list.go | 31 +++++++++++++++++++------------ internal/cmd/base/list_test.go | 18 +++++++++++++++++- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/internal/cmd/base/list.go b/internal/cmd/base/list.go index b161219..53598c5 100644 --- a/internal/cmd/base/list.go +++ b/internal/cmd/base/list.go @@ -12,8 +12,9 @@ import ( // ListCmd defines a command for fetching and displaying a list response. // T is the full response proto message (e.g. *clusterv1.ListClustersResponse). // -// OutputTable must be set. The base automatically registers --no-headers and -// handles header suppression. +// At least one of OutputTable or PrintText must be set. OutputTable is +// preferred; when set, --no-headers is automatically registered and handled. +// PrintText is the legacy fallback for commands that have not yet migrated. type ListCmd[T any] struct { Use string Short string @@ -21,6 +22,7 @@ type ListCmd[T any] struct { Example string Fetch func(s *state.State, cmd *cobra.Command) (T, error) OutputTable func(cmd *cobra.Command, out io.Writer, resp T) (output.TableRenderer, error) + PrintText func(cmd *cobra.Command, out io.Writer, resp T) error ValidArgsFunction func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) } @@ -40,20 +42,25 @@ func (lc ListCmd[T]) CobraCommand(s *state.State) *cobra.Command { if s.Config.JSONOutput() { return output.PrintJSON(cmd.OutOrStdout(), resp) } - if lc.OutputTable == nil { - panic("ListCmd: OutputTable must be set") + if lc.OutputTable != nil { + r, err := lc.OutputTable(cmd, cmd.OutOrStdout(), resp) + if err != nil { + return err + } + noHeaders, _ := cmd.Flags().GetBool("no-headers") + r.SetNoHeaders(noHeaders) + r.Render() + return nil } - r, err := lc.OutputTable(cmd, cmd.OutOrStdout(), resp) - if err != nil { - return err + if lc.PrintText != nil { + return lc.PrintText(cmd, cmd.OutOrStdout(), resp) } - noHeaders, _ := cmd.Flags().GetBool("no-headers") - r.SetNoHeaders(noHeaders) - r.Render() - return nil + panic("ListCmd: OutputTable or PrintText must be set") }, } - cmd.Flags().Bool("no-headers", false, "Do not print column headers") + if lc.OutputTable != nil { + cmd.Flags().Bool("no-headers", false, "Do not print column headers") + } if lc.ValidArgsFunction != nil { cmd.ValidArgsFunction = lc.ValidArgsFunction } diff --git a/internal/cmd/base/list_test.go b/internal/cmd/base/list_test.go index 99caa17..63db03e 100644 --- a/internal/cmd/base/list_test.go +++ b/internal/cmd/base/list_test.go @@ -2,6 +2,7 @@ package base_test import ( "bytes" + "fmt" "io" "testing" @@ -63,7 +64,22 @@ func TestListCmd_OutputTable_NoHeaders(t *testing.T) { assert.Contains(t, stdout, "hello") } -func TestListCmd_NoOutputTable_Panics(t *testing.T) { +func TestListCmd_PrintText(t *testing.T) { + lc := base.ListCmd[string]{ + Use: "test", + Fetch: fetchHello, + PrintText: func(_ *cobra.Command, out io.Writer, resp string) error { + _, err := fmt.Fprint(out, resp) + return err + }, + } + + stdout, err := execListCmd(t, lc) + require.NoError(t, err) + assert.Equal(t, "hello", stdout) +} + +func TestListCmd_NeitherOutputTableNorPrintText_Panics(t *testing.T) { lc := base.ListCmd[string]{ Use: "test", Fetch: fetchHello, From 79a20226da3a81a93e43db436f33d0c355f08448 Mon Sep 17 00:00:00 2001 From: dhernando Date: Fri, 10 Apr 2026 16:31:20 +0200 Subject: [PATCH 3/3] feat: implement the rest of lists using OutputTable Furthermore, removed the PrintText option in ListCmd to avoid inconsistencies. Also updated AGENTS.md with up to date examples. --- AGENTS.md | 10 +++---- internal/cmd/account/list.go | 6 ++-- internal/cmd/account/list_test.go | 16 ++++++++++ internal/cmd/account/member_list.go | 6 ++-- internal/cmd/account/member_list_test.go | 16 ++++++++++ internal/cmd/base/list.go | 37 ++++++++++------------- internal/cmd/base/list_test.go | 38 +++++++++--------------- internal/cmd/cluster/key_list.go | 12 ++++---- internal/cmd/iam/permission_list.go | 6 ++-- internal/cmd/iam/permission_list_test.go | 14 +++++++++ internal/cmd/iam/role_list.go | 6 ++-- internal/cmd/iam/role_list_test.go | 14 +++++++++ internal/cmd/iam/user_list.go | 6 ++-- internal/cmd/iam/user_list_test.go | 14 +++++++++ 14 files changed, 129 insertions(+), 72 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ebfeecd..14316f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,9 +94,7 @@ Each subcommand group lives in `internal/cmd//`: All leaf commands are built using one of five generic base types. Always prefer these over raw `cobra.Command`. #### `base.ListCmd[T]` -For listing resources. No args, no flags needed (extend via `BaseCobraCommand` if flags are required). - -Prefer `OutputTable` over `PrintText` for table output. When `OutputTable` is set, the base automatically registers `--no-headers` and handles header suppression. `PrintText` is used as a fallback when `OutputTable` is not set. +For listing resources. `OutputTable` must be set. The base automatically registers `--no-headers` and handles header suppression. By default the command takes no positional args; set `Args` to accept them. ```go base.ListCmd[*foov1.ListFoosResponse]{ @@ -105,11 +103,11 @@ base.ListCmd[*foov1.ListFoosResponse]{ Fetch: func(s *state.State, cmd *cobra.Command) (*foov1.ListFoosResponse, error) { // call gRPC, return response }, - OutputTable: func(_ *cobra.Command, w io.Writer, resp *foov1.ListFoosResponse) output.Renderable { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *foov1.ListFoosResponse) (output.TableRenderer, error) { t := output.NewTable[*foov1.Foo](w) t.AddField("ID", func(v *foov1.Foo) string { return v.GetId() }) - t.SetItems(resp.Items) - return t + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) ``` diff --git a/internal/cmd/account/list.go b/internal/cmd/account/list.go index 2640c13..0e6f329 100644 --- a/internal/cmd/account/list.go +++ b/internal/cmd/account/list.go @@ -34,7 +34,7 @@ qcloud account list --json`, return client.Account().ListAccounts(ctx, &accountv1.ListAccountsRequest{}) }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *accountv1.ListAccountsResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *accountv1.ListAccountsResponse) (output.TableRenderer, error) { t := output.NewTable[*accountv1.Account](w) t.AddField("ID", func(v *accountv1.Account) string { return v.GetId() }) t.AddField("NAME", func(v *accountv1.Account) string { return v.GetName() }) @@ -45,8 +45,8 @@ qcloud account list --json`, } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/account/list_test.go b/internal/cmd/account/list_test.go index 3a20c87..46aa8e9 100644 --- a/internal/cmd/account/list_test.go +++ b/internal/cmd/account/list_test.go @@ -84,3 +84,19 @@ func TestAccountList_Empty(t *testing.T) { assert.Contains(t, stdout, "ID") assert.Contains(t, stdout, "NAME") } + +func TestAccountList_NoHeaders(t *testing.T) { + env := testutil.NewTestEnv(t) + + env.AccountServer.ListAccountsCalls.Returns(&accountv1.ListAccountsResponse{ + Items: []*accountv1.Account{ + {Id: "acct-001", Name: "Production"}, + }, + }, nil) + + stdout, _, err := testutil.Exec(t, env, "account", "list", "--no-headers") + require.NoError(t, err) + assert.NotContains(t, stdout, "ID") + assert.NotContains(t, stdout, "NAME") + assert.Contains(t, stdout, "acct-001") +} diff --git a/internal/cmd/account/member_list.go b/internal/cmd/account/member_list.go index 96d507b..bc25a8d 100644 --- a/internal/cmd/account/member_list.go +++ b/internal/cmd/account/member_list.go @@ -48,7 +48,7 @@ qcloud account member list --json`, return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *accountv1.ListAccountMembersResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *accountv1.ListAccountMembersResponse) (output.TableRenderer, error) { t := output.NewTable[*accountv1.AccountMember](w) t.AddField("ID", func(v *accountv1.AccountMember) string { return v.GetAccountMember().GetId() @@ -65,8 +65,8 @@ qcloud account member list --json`, } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/account/member_list_test.go b/internal/cmd/account/member_list_test.go index 827986d..4adb25a 100644 --- a/internal/cmd/account/member_list_test.go +++ b/internal/cmd/account/member_list_test.go @@ -106,3 +106,19 @@ func TestMemberList_Empty(t *testing.T) { assert.Contains(t, stdout, "ID") assert.Contains(t, stdout, "EMAIL") } + +func TestMemberList_NoHeaders(t *testing.T) { + env := testutil.NewTestEnv(t) + + env.AccountServer.ListAccountMembersCalls.Returns(&accountv1.ListAccountMembersResponse{ + Items: []*accountv1.AccountMember{ + {AccountMember: &iamv1.User{Id: "user-001", Email: "owner@example.com"}}, + }, + }, nil) + + stdout, _, err := testutil.Exec(t, env, "account", "member", "list", "--no-headers") + require.NoError(t, err) + assert.NotContains(t, stdout, "ID") + assert.NotContains(t, stdout, "EMAIL") + assert.Contains(t, stdout, "user-001") +} diff --git a/internal/cmd/base/list.go b/internal/cmd/base/list.go index 53598c5..31f8364 100644 --- a/internal/cmd/base/list.go +++ b/internal/cmd/base/list.go @@ -12,28 +12,31 @@ import ( // ListCmd defines a command for fetching and displaying a list response. // T is the full response proto message (e.g. *clusterv1.ListClustersResponse). // -// At least one of OutputTable or PrintText must be set. OutputTable is -// preferred; when set, --no-headers is automatically registered and handled. -// PrintText is the legacy fallback for commands that have not yet migrated. +// OutputTable must be set. When set, --no-headers is automatically registered +// and handled. type ListCmd[T any] struct { Use string Short string Long string Example string + Args cobra.PositionalArgs // optional; defaults to cobra.NoArgs Fetch func(s *state.State, cmd *cobra.Command) (T, error) OutputTable func(cmd *cobra.Command, out io.Writer, resp T) (output.TableRenderer, error) - PrintText func(cmd *cobra.Command, out io.Writer, resp T) error ValidArgsFunction func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) } // CobraCommand builds a cobra.Command from this ListCmd. func (lc ListCmd[T]) CobraCommand(s *state.State) *cobra.Command { + posArgs := lc.Args + if posArgs == nil { + posArgs = cobra.NoArgs + } cmd := &cobra.Command{ Use: lc.Use, Short: lc.Short, Long: lc.Long, Example: lc.Example, - Args: cobra.NoArgs, + Args: posArgs, RunE: func(cmd *cobra.Command, args []string) error { resp, err := lc.Fetch(s, cmd) if err != nil { @@ -42,25 +45,17 @@ func (lc ListCmd[T]) CobraCommand(s *state.State) *cobra.Command { if s.Config.JSONOutput() { return output.PrintJSON(cmd.OutOrStdout(), resp) } - if lc.OutputTable != nil { - r, err := lc.OutputTable(cmd, cmd.OutOrStdout(), resp) - if err != nil { - return err - } - noHeaders, _ := cmd.Flags().GetBool("no-headers") - r.SetNoHeaders(noHeaders) - r.Render() - return nil - } - if lc.PrintText != nil { - return lc.PrintText(cmd, cmd.OutOrStdout(), resp) + r, err := lc.OutputTable(cmd, cmd.OutOrStdout(), resp) + if err != nil { + return err } - panic("ListCmd: OutputTable or PrintText must be set") + noHeaders, _ := cmd.Flags().GetBool("no-headers") + r.SetNoHeaders(noHeaders) + r.Render() + return nil }, } - if lc.OutputTable != nil { - cmd.Flags().Bool("no-headers", false, "Do not print column headers") - } + cmd.Flags().Bool("no-headers", false, "Do not print column headers") if lc.ValidArgsFunction != nil { cmd.ValidArgsFunction = lc.ValidArgsFunction } diff --git a/internal/cmd/base/list_test.go b/internal/cmd/base/list_test.go index 63db03e..04a12ca 100644 --- a/internal/cmd/base/list_test.go +++ b/internal/cmd/base/list_test.go @@ -2,7 +2,6 @@ package base_test import ( "bytes" - "fmt" "io" "testing" @@ -49,43 +48,34 @@ func TestListCmd_OutputTable(t *testing.T) { assert.Contains(t, stdout, "hello") } -func TestListCmd_OutputTable_NoHeaders(t *testing.T) { +func TestListCmd_WithArgs(t *testing.T) { lc := base.ListCmd[string]{ - Use: "test", - Fetch: fetchHello, + Use: "test ", + Args: cobra.ExactArgs(1), + Fetch: func(_ *state.State, cmd *cobra.Command) (string, error) { + return cmd.Flags().Arg(0), nil + }, OutputTable: func(_ *cobra.Command, out io.Writer, resp string) (output.TableRenderer, error) { return stringTableRenderer(out, resp), nil }, } - stdout, err := execListCmd(t, lc, "--no-headers") + stdout, err := execListCmd(t, lc, "world") require.NoError(t, err) - assert.NotContains(t, stdout, "VALUE") - assert.Contains(t, stdout, "hello") + assert.Contains(t, stdout, "world") } -func TestListCmd_PrintText(t *testing.T) { +func TestListCmd_OutputTable_NoHeaders(t *testing.T) { lc := base.ListCmd[string]{ Use: "test", Fetch: fetchHello, - PrintText: func(_ *cobra.Command, out io.Writer, resp string) error { - _, err := fmt.Fprint(out, resp) - return err + OutputTable: func(_ *cobra.Command, out io.Writer, resp string) (output.TableRenderer, error) { + return stringTableRenderer(out, resp), nil }, } - stdout, err := execListCmd(t, lc) + stdout, err := execListCmd(t, lc, "--no-headers") require.NoError(t, err) - assert.Equal(t, "hello", stdout) -} - -func TestListCmd_NeitherOutputTableNorPrintText_Panics(t *testing.T) { - lc := base.ListCmd[string]{ - Use: "test", - Fetch: fetchHello, - } - - assert.Panics(t, func() { - _, _ = execListCmd(t, lc) - }) + assert.NotContains(t, stdout, "VALUE") + assert.Contains(t, stdout, "hello") } diff --git a/internal/cmd/cluster/key_list.go b/internal/cmd/cluster/key_list.go index 06d825c..b0d60c7 100644 --- a/internal/cmd/cluster/key_list.go +++ b/internal/cmd/cluster/key_list.go @@ -16,14 +16,14 @@ import ( ) func newKeyListCommand(s *state.State) *cobra.Command { - return base.DescribeCmd[*clusterauthv2.ListDatabaseApiKeysResponse]{ + return base.ListCmd[*clusterauthv2.ListDatabaseApiKeysResponse]{ Use: "list ", Short: "List API keys for a cluster", Example: `# List API keys for a cluster qcloud cluster key list 7b2ea926-724b-4de2-b73a-8675c42a6ebe`, Args: util.ExactArgs(1, "a cluster ID"), - Fetch: func(s *state.State, cmd *cobra.Command, args []string) (*clusterauthv2.ListDatabaseApiKeysResponse, error) { - clusterID := args[0] + Fetch: func(s *state.State, cmd *cobra.Command) (*clusterauthv2.ListDatabaseApiKeysResponse, error) { + clusterID := cmd.Flags().Arg(0) ctx := cmd.Context() client, err := s.Client(ctx) @@ -46,7 +46,7 @@ qcloud cluster key list 7b2ea926-724b-4de2-b73a-8675c42a6ebe`, return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *clusterauthv2.ListDatabaseApiKeysResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *clusterauthv2.ListDatabaseApiKeysResponse) (output.TableRenderer, error) { t := output.NewTable[*clusterauthv2.DatabaseApiKey](w) t.AddField("ID", func(v *clusterauthv2.DatabaseApiKey) string { return v.GetId() @@ -69,8 +69,8 @@ qcloud cluster key list 7b2ea926-724b-4de2-b73a-8675c42a6ebe`, } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, ValidArgsFunction: completion.ClusterIDCompletion(s), }.CobraCommand(s) diff --git a/internal/cmd/iam/permission_list.go b/internal/cmd/iam/permission_list.go index b2f5bdb..be3a86a 100644 --- a/internal/cmd/iam/permission_list.go +++ b/internal/cmd/iam/permission_list.go @@ -42,7 +42,7 @@ qcloud iam permission list --json`, AccountId: accountID, }) }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *iamv1.ListPermissionsResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *iamv1.ListPermissionsResponse) (output.TableRenderer, error) { t := output.NewTable[*iamv1.Permission](w) t.AddField("PERMISSION", func(v *iamv1.Permission) string { return v.GetValue() @@ -50,8 +50,8 @@ qcloud iam permission list --json`, t.AddField("CATEGORY", func(v *iamv1.Permission) string { return v.GetCategory() }) - t.Write(resp.GetPermissions()) - return nil + t.SetItems(resp.GetPermissions()) + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/iam/permission_list_test.go b/internal/cmd/iam/permission_list_test.go index 4b0e447..b970337 100644 --- a/internal/cmd/iam/permission_list_test.go +++ b/internal/cmd/iam/permission_list_test.go @@ -69,3 +69,17 @@ func TestPermissionList_BackendError(t *testing.T) { _, _, err := testutil.Exec(t, env, "iam", "permission", "list") require.Error(t, err) } + +func TestPermissionList_NoHeaders(t *testing.T) { + env := testutil.NewTestEnv(t) + + env.IAMServer.ListPermissionsCalls.Returns(&iamv1.ListPermissionsResponse{ + Permissions: []*iamv1.Permission{{Value: "read:clusters", Category: new("Cluster")}}, + }, nil) + + stdout, _, err := testutil.Exec(t, env, "iam", "permission", "list", "--no-headers") + require.NoError(t, err) + assert.NotContains(t, stdout, "PERMISSION") + assert.NotContains(t, stdout, "CATEGORY") + assert.Contains(t, stdout, "read:clusters") +} diff --git a/internal/cmd/iam/role_list.go b/internal/cmd/iam/role_list.go index 41bb287..ac0cd4a 100644 --- a/internal/cmd/iam/role_list.go +++ b/internal/cmd/iam/role_list.go @@ -42,7 +42,7 @@ qcloud iam role list --json`, AccountId: accountID, }) }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *iamv1.ListRolesResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *iamv1.ListRolesResponse) (output.TableRenderer, error) { t := output.NewTable[*iamv1.Role](w) t.AddField("ID", func(v *iamv1.Role) string { return v.GetId() @@ -62,8 +62,8 @@ qcloud iam role list --json`, } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/iam/role_list_test.go b/internal/cmd/iam/role_list_test.go index e46adf9..2ca73ee 100644 --- a/internal/cmd/iam/role_list_test.go +++ b/internal/cmd/iam/role_list_test.go @@ -105,3 +105,17 @@ func TestRoleList_BackendError(t *testing.T) { _, _, err := testutil.Exec(t, env, "iam", "role", "list") require.Error(t, err) } + +func TestRoleList_NoHeaders(t *testing.T) { + env := testutil.NewTestEnv(t) + + env.IAMServer.ListRolesCalls.Returns(&iamv1.ListRolesResponse{ + Items: []*iamv1.Role{{Id: "role-abc", Name: "Admin"}}, + }, nil) + + stdout, _, err := testutil.Exec(t, env, "iam", "role", "list", "--no-headers") + require.NoError(t, err) + assert.NotContains(t, stdout, "ID") + assert.NotContains(t, stdout, "NAME") + assert.Contains(t, stdout, "role-abc") +} diff --git a/internal/cmd/iam/user_list.go b/internal/cmd/iam/user_list.go index 4f03b4b..e17d04c 100644 --- a/internal/cmd/iam/user_list.go +++ b/internal/cmd/iam/user_list.go @@ -44,7 +44,7 @@ qcloud iam user list --json`, } return resp, nil }, - PrintText: func(_ *cobra.Command, w io.Writer, resp *iamv1.ListUsersResponse) error { + OutputTable: func(_ *cobra.Command, w io.Writer, resp *iamv1.ListUsersResponse) (output.TableRenderer, error) { t := output.NewTable[*iamv1.User](w) t.AddField("ID", func(v *iamv1.User) string { return v.GetId() }) t.AddField("EMAIL", func(v *iamv1.User) string { return v.GetEmail() }) @@ -55,8 +55,8 @@ qcloud iam user list --json`, } return "" }) - t.Write(resp.GetItems()) - return nil + t.SetItems(resp.GetItems()) + return t, nil }, }.CobraCommand(s) } diff --git a/internal/cmd/iam/user_list_test.go b/internal/cmd/iam/user_list_test.go index 3042f6e..2915ba4 100644 --- a/internal/cmd/iam/user_list_test.go +++ b/internal/cmd/iam/user_list_test.go @@ -68,3 +68,17 @@ func TestUserList_Error(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "permission denied") } + +func TestUserList_NoHeaders(t *testing.T) { + env := testutil.NewTestEnv(t) + + env.IAMServer.ListUsersCalls.Returns(&iamv1.ListUsersResponse{ + Items: []*iamv1.User{{Id: "user-1", Email: "alice@example.com"}}, + }, nil) + + stdout, _, err := testutil.Exec(t, env, "iam", "user", "list", "--no-headers") + require.NoError(t, err) + assert.NotContains(t, stdout, "ID") + assert.NotContains(t, stdout, "EMAIL") + assert.Contains(t, stdout, "user-1") +}