- Notifications
You must be signed in to change notification settings - Fork928
Use new table formatter everywhere#3544
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
Uh oh!
There was an error while loading.Please reload this page.
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.
👍
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.
Looks great!
One thing I started thinking about. When someone filters columns or what not, can they pass in any value as long as it's close? Like say we havetable:"scope id", as a user I might want to pass in-c scope_id (no quotes) or-c 'SCOPE ID' (i.e. what I see).
| err := cliui.ValidateColumns(featureColumns, columns) | ||
| if err != nil { | ||
| return err | ||
| } |
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.
One downside of this change is that we have to round-trip to the server for a response instead of exiting immediately on bad input.
Would it make sense to expose acliui.ValidateTableColumns([]codersdk.Derp{}, columns) function?
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.
I'm hoping to automate this whole process of validating columns and adding the columns/sort flags in a future PR. I just removed this because it didn't match any of the other commands, but your comment is valid. I can add it back if you want or we could wait until we have better output utilities.
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.
Sounds good. My opinion is that we’re not in a hurry to restore the early check. It’s a small QoL improvement but it can wait for the refactor just fine.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Yes both of those will work. |
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.
frontend ✅
Uses the new table formatter introduced in#3415 everywhere across the CLI except for two places:
schedule showandusers showwhich don't use a traditional table format.