- Notifications
You must be signed in to change notification settings - Fork928
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Paired with Ali on this. TheReflect() piece is just to get the struct fields on the workspace.
Requesting other eyes but we both tested this together and it works fine from what I can tell.
| if me { | ||
| myUser, err := client.User(cmd.Context(), codersdk.Me) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| filter.Owner = myUser.Username | ||
| } |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM but it'd be better if it were a set of reusable functions instead :)
| 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[:], ", ") |
There was a problem hiding this comment.
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 acceptsany:
funcTableHeaders(tany) ([]string,error) {v:=reflect.Indirect(reflect.ValueOf(t))returncliui.TypeToTableHeaders(v.Type())}funcFormatColumnNames(columns []string)string {out:=strings.Join(columns[:],", ")out=strings.ReplaceAll(out," ","_")returnout}
which could be called like
columns,err:=cliui.TableHeaders(workspaceListRow{})iferr!=nil {panic(err)}columnString:=cliui.FormatColumnNames(columns)
This would also remove the need for having a global(ish) var for the output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
future fix ideaaa :D
Solves feature request for#1432