Option to display results as ASCII text #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Nice work! One iteration and that should be it.
Also formatting is perfect. I usually use goimports instead of go fmt.
I do like your idea of a more generic CLI option. Initially I was also thinking in the direction of --display-mode, but as you said: it will be much more in-line with other k8s tools, if we use a -o option. That will leave the door open for json/yaml output in the future. Do you think -output icons and -output ascii is descriptive enough?
|
Even if #9 fixes color output on PowerShell, I think a monochrome ASCII mode makes more sense than with colors. |
|
Thanks for the feedback. I think this covers everything - please take a look. |
|
Thanks for your contribution! LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a new
--display-modecommand line argument, with possible valuesicons(displays the tick and cross icons, coloured if outputting to a terminal) andascii(displays the wordsyesandno, never coloured). Theasciimode is intended to be easier to consume programmatically e.g. from scripts.I'd very much welcome feedback on the whether the CLI option feels right - I wondered whether a
-o/--outputoption might be more aligned with other tools, and would be more reusable if you later wanted to add e.g. JSON? It should be easy for me to change, and better to get it right now than to have to make a breaking change later. Let me know your thoughts!Finally, I believe this is formatted correctly etc., but I'm still a bit of a Go newbie especially as regards style, so please, be kind to my mistakes!
Fixes #5.