Skip to content

feat: coder ls should show possible columns to filter by #4240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 28, 2022
Merged

feat: coder ls should show possible columns to filter by #4240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1383551
added showing columns in help call, need to format to make pretty
Sep 6, 2022
93df69a
finished formatting column strings for print of list /ls command
Sep 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions cli/cliui/table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
}

// Get the list of table column headers.
headersRaw, err := typeToTableHeaders(v.Type().Elem())
headersRaw, err := TypeToTableHeaders(v.Type().Elem())
if err != nil {
return "", xerrors.Errorf("get table headers recursively for type %q: %w", v.Type().Elem().String(), err)
}
Expand Down Expand Up @@ -207,10 +207,10 @@ func isStructOrStructPointer(t reflect.Type) bool {
return t.Kind() == reflect.Struct || (t.Kind() == reflect.Pointer && t.Elem().Kind() == reflect.Struct)
}

// typeToTableHeaders converts a type to a slice of column names. If the given
// TypeToTableHeaders converts a type to a slice of column names. If the given
// type is invalid (not a struct or a pointer to a struct, has invalid table
// tags, etc.), an error is returned.
func typeToTableHeaders(t reflect.Type) ([]string, error) {
func TypeToTableHeaders(t reflect.Type) ([]string, error) {
if !isStructOrStructPointer(t) {
return nil, xerrors.Errorf("typeToTableHeaders called with a non-struct or a non-pointer-to-a-struct type")
}
Expand All @@ -235,7 +235,7 @@ func typeToTableHeaders(t reflect.Type) ([]string, error) {
return nil, xerrors.Errorf("field %q in type %q is marked as recursive but does not contain a struct or a pointer to a struct", field.Name, t.String())
}

childNames, err := typeToTableHeaders(fieldType)
childNames, err := TypeToTableHeaders(fieldType)
if err != nil {
return nil, xerrors.Errorf("get child field header names for field %q in type %q: %w", field.Name, fieldType.String(), err)
}
Expand Down
37 changes: 30 additions & 7 deletions cli/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package cli

import (
"fmt"
"reflect"
"strings"
"time"

"github.com/google/uuid"
Expand Down Expand Up @@ -58,10 +60,12 @@ func workspaceListRowFromWorkspace(now time.Time, usersByID map[uuid.UUID]coders

func list() *cobra.Command {
var (
all bool
columns []string
defaultQuery = "owner:me"
searchQuery string
all bool
columns []string
defaultQuery = "owner:me"
searchQuery string
me bool
displayWorkspaces []workspaceListRow
)
cmd := &cobra.Command{
Annotations: workspaceCommand,
Expand All @@ -80,6 +84,14 @@ func list() *cobra.Command {
if all && searchQuery == defaultQuery {
filter.FilterQuery = ""
}

if me {
myUser, err := client.User(cmd.Context(), codersdk.Me)
if err != nil {
return err
}
filter.Owner = myUser.Username
}
Comment on lines +88 to +94
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: this might be a left-over piece of a merge conflict

workspaces, err := client.Workspaces(cmd.Context(), filter)
if err != nil {
return err
Expand All @@ -101,7 +113,7 @@ func list() *cobra.Command {
}

now := time.Now()
displayWorkspaces := make([]workspaceListRow, len(workspaces))
displayWorkspaces = make([]workspaceListRow, len(workspaces))
for i, workspace := range workspaces {
displayWorkspaces[i] = workspaceListRowFromWorkspace(now, usersByID, workspace)
}
Expand All @@ -115,10 +127,21 @@ func list() *cobra.Command {
return err
},
}

v := reflect.Indirect(reflect.ValueOf(displayWorkspaces))
availColumns, err := cliui.TypeToTableHeaders(v.Type().Elem())
if err != nil {
panic(err)
}
for i, s := range availColumns {
availColumns[i] = strings.Replace(s, " ", "_", -1)
}
columnString := strings.Join(availColumns[:], ", ")
Comment on lines +131 to +139
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good, but since we'd probably want to use this in multiple commands it would be better served as a function in the cliui package that accepts any:

func TableHeaders(t any) ([]string, error) {
	v := reflect.Indirect(reflect.ValueOf(t))
	return cliui.TypeToTableHeaders(v.Type())
}

func FormatColumnNames(columns []string) string {
	out := strings.Join(columns[:], ", ")
	out = strings.ReplaceAll(out, " ", "_")
	return out
}

which could be called like

columns, err := cliui.TableHeaders(workspaceListRow{})
if err != nil {
	panic(err)
}

columnString := cliui.FormatColumnNames(columns)

This would also remove the need for having a global(ish) var for the output

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

future fix ideaaa :D


cmd.Flags().BoolVarP(&all, "all", "a", false,
"Specifies whether all workspaces will be listed or not.")
cmd.Flags().StringArrayVarP(&columns, "column", "c", nil,
"Specify a column to filter in the table.")
cmd.Flags().StringVar(&searchQuery, "search", defaultQuery, "Search for a workspace with a query.")
fmt.Sprintf("Specify a column to filter in the table. Available columns are: %v", columnString))
cmd.Flags().StringVar(&searchQuery, "search", "", "Search for a workspace with a query.")
return cmd
}