Add --suppress-header argument to list workspace/variables #69
Conversation
There was a problem hiding this comment.
@mpdokken thanks for doing this work! There's one small thing to address (whitespace formatting) and one question I asked @dalenewby. The workspace list isn't really CSV, so I don't believe that's what he had in mind, even though he mentioned the workspace list command. Might be good to have him weigh in on this.
cmd/root.go
Outdated
| cfgFile string | ||
| organization string | ||
| readOnlyMode bool | ||
| suppressCSVHeader bool |
There was a problem hiding this comment.
The indentation doesn't match here. Go has a built-in formatter that makes it easy to conform to standards on things like this. If you can, it's a good idea to configure your development environment to run go fmt (or go imports) on every save.
There was a problem hiding this comment.
Thanks for the feedback, I've setup the formatter and corrected the whitespace. I'll update the workspace list command if the user clarifies what they had in mind.
| @@ -44,7 +44,9 @@ var variablesListCmd = &cobra.Command{ | |||
| } | |||
|
|
|||
| if tabularCSV { | |||
| fmt.Println("workspace,key,value") | |||
| if !suppressCSVHeader { | |||
| fmt.Println("workspace,key,value") | |||
There was a problem hiding this comment.
@dalenewby is this what you had in mind when you wrote #55?
There was a problem hiding this comment.
Maybe? :-) As I recall, Steve had added a command that would nicely report information that I was obtaining via curl and an ugly URL followed by post-processing with an uglier jq query and a magical Perl statement that I only understand while reading the description in Programming Perl (page 221 in the first edition). I really wanted to use his new command but he was also printing out nice, human-readable headers for the data. The header made it hard for my Linux command pipeline to process the output because the header was a special case. I wanted a way to turn off headers for all commands that printed data. I also wanted error and informational messages to be directed to STDERR so I just got data on STDOUT.
There was a problem hiding this comment.
@dalenewby Since a lot of the command outputs aren't CSV, there's no standard format the users should expect. Would it be helpful to include the expected output format of the --suppress-header option in the README, for each command that prints data?
| content = wsNames[0] | ||
| } | ||
|
|
||
| if suppressCSVHeader { |
There was a problem hiding this comment.
I'm not sure this was the intent of issue #55. This data isn't really CSV in the usual sense.
|
These changes are not as helpful as they were once thought to be. |
Added the global argument to suppress headers in variablesListCmd and workspaceListToString, updated README.md and Test_workspaceListToString