Update check and inject output #2087
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
alpeb
reviewed
Jan 15, 2019
|
I know the Helm folks ran into issue with emoji output in their CLIs -- maybe @michelleN has an opinion? |
alpeb
reviewed
Jan 15, 2019
cli/cmd/check.go
Outdated
| if result.Retry { | ||
| fmt.Fprintf(w, "%s%s%s -- %s%s", checkLabel, filler, retryStatus, result.Err, lineBreak) | ||
| spin.Suffix = fmt.Sprintf(" %s -- %s", result.Description, result.Err) | ||
| spin.Color("bgBlack", "bold", "fgRed") |
There was a problem hiding this comment.
I have a light theme in my console (I know, weird, but I have my reasons :) ) so the spinner doesn't look right. Although I looked a bit into the spinner lib and couldn't find a way to cater for both dark and light themes at the same time :-(
There was a problem hiding this comment.
@alpeb can you try replacing that spin.Color(... line with spin.Start(...? maybe the default colors will play better in light and dark themes?
There was a problem hiding this comment.
Yes, that did the trick. I looks fine in both dark and light.
There was a problem hiding this comment.
@alpeb I'm trying another revision with spin.Color("bold"), let me know how that looks.
|
Really great improvement! The warnings are great.
|
|
klingerf
approved these changes
Jan 16, 2019
cli/cmd/check.go
Outdated
| underline := "" | ||
| for i := 0; i < len(result.Category); i++ { | ||
| underline = underline + "-" | ||
| } |
There was a problem hiding this comment.
You can use strings.Repeat here:
diff --git a/cli/cmd/check.go b/cli/cmd/check.go
index 8da69362..4d2c8fc9 100644
--- a/cli/cmd/check.go
+++ b/cli/cmd/check.go
@@ -145,13 +145,8 @@ func runChecks(w io.Writer, hc *healthcheck.HealthChecker) bool {
fmt.Fprintln(w)
}
- underline := ""
- for i := 0; i < len(result.Category); i++ {
- underline = underline + "-"
- }
-
fmt.Fprintln(w, result.Category)
- fmt.Fprintln(w, underline)
+ fmt.Fprintln(w, strings.Repeat("-", len(result.Category)))
lastCategory = result.Category
}|
|
||
| func getFiller(text string) string { | ||
| filler := "" | ||
| for i := 0; i < lineWidth-len(text)-len(okStatus)-len("\n"); i++ { |
The outputs of the `check` and `inject` commands did not vary much between successful and failed executions, and were a bit verbose and challenging to parse. Reorganize output of `check` and `inject` commands, to provide more output when errors occur, and less output when successful. Specific changes: `linkerd check` - visually group checks by category - introduce `hintURL`'s, to provide doc links when checks fail - add spinners when retrying, remove addtional retry lines - colored emojis to indicate success/warning/failure `linkerd inject` - modify default output to mirror `kubectl apply` - only output non-successful inject reports - support `--verbose` flag to output all inject reports Fixes #1471, #1653, #1656, #1739 Signed-off-by: Andrew Seigner <siggy@buoyant.io>
Signed-off-by: Andrew Seigner <siggy@buoyant.io>
Signed-off-by: Andrew Seigner <siggy@buoyant.io>
Signed-off-by: Andrew Seigner <siggy@buoyant.io>
siggy
force-pushed
the
siggy/i-dont-want-no-bools
branch
from
3e5ca3a
to
eb752ae
Compare
This was referenced Jan 16, 2019
siggy
added a commit
that referenced
this pull request
Jan 20, 2019
The default font in Windows console did not support the Unicode characters recently added to check and inject commands. Also the color library the linkerd cli depends on was not being used in a cross-platform way. Replace the existing Unicode characters used in `check` and `inject` with characters available in most fonts, including Windows console. Similarly replace the spinner used in `check` with one that uses characters available in most fonts. Modify `check` and `inject` to use `color.Output` and `color.Error`, which wrap `os.Stdout` and `os.Stderr`, and perform special tranformations when on Windows. Add a `--no-color` option to `linkerd logs`. While stern uses the same color library that `check`/`inject` use, it is not yet using the `color.Output` API for Windows support. That issue is tracked at: https://github.com/wercker/stern/issues/69 Relates to #2087 Signed-off-by: Andrew Seigner <siggy@buoyant.io>
siggy
added a commit
that referenced
this pull request
Jan 23, 2019
The default font in Windows console did not support the Unicode characters recently added to check and inject commands. Also the color library the linkerd cli depends on was not being used in a cross-platform way. Replace the existing Unicode characters used in `check` and `inject` with characters available in most fonts, including Windows console. Similarly replace the spinner used in `check` with one that uses characters available in most fonts. Modify `check` and `inject` to use `color.Output` and `color.Error`, which wrap `os.Stdout` and `os.Stderr`, and perform special tranformations when on Windows. Add a `--no-color` option to `linkerd logs`. While stern uses the same color library that `check`/`inject` use, it is not yet using the `color.Output` API for Windows support. That issue is tracked at: https://github.com/wercker/stern/issues/69 Relates to #2087 Signed-off-by: Andrew Seigner <siggy@buoyant.io>
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.
The outputs of the
checkandinjectcommands did not vary muchbetween successful and failed executions, and were a bit verbose and
challenging to parse.
Reorganize output of
checkandinjectcommands, to provide moreoutput when errors occur, and less output when successful.
Specific changes:
linkerd checkhintURL's, to provide doc links when checks faillinkerd injectkubectl apply--verboseflag to output all inject reportsFixes #1471, #1653, #1656, #1739
Signed-off-by: Andrew Seigner siggy@buoyant.io
Example outputs
Note the unicode symbols are rendered with color in the terminal:
linkerd checkwith retrieslinkerd check --prelinkerd checklinkerd check --prewith errorslinkerd check --proxylinkerd injectlinkerd inject --verbose