- Notifications
You must be signed in to change notification settings - Fork928
fix: Match kubectl table style for simpler scripting#1363
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
codecovbot commentedMay 10, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #1363 +/- ##==========================================+ Coverage 66.69% 67.01% +0.32%========================================== Files 284 287 +3 Lines 18614 18762 +148 Branches 235 235 ==========================================+ Hits 12414 12573 +159+ Misses 4931 4902 -29- Partials 1269 1287 +18
Continue to review full report at Codecov.
|
| // Table creates a new table with standardized styles. | ||
| func Table() table.Writer { | ||
| tableWriter := table.NewWriter() |
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.
Not to bikeshed on this, but Go had a stdlib solution for this:https://pkg.go.dev/text/tabwriter. Haven't taken a look at this package though
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.
Oh actually nvm, I thought u were totally switching out the table packages
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.
Oh, nah. This package handles hiding columns and sorting which is kinda nice.
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.
ye i agree, should've looked through more b4 commenting
| cmd.Flags().StringArrayVarP(&columns, "column", "c", nil, | ||
| "Specify a column to filter in the table.") |
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.
To filter by multiple columns, do you need to specify the flag multiple times? Or can it accept a comma delimited string? I think we should document how to specify multiple.
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.
You have to specify multiple, but the flag does specify the type as doing so.
Uh oh!
There was an error while loading.Please reload this page.
Fixes#1322.