- Notifications
You must be signed in to change notification settings - Fork928
fix: use ANSI colors codes instead of RGB#14665
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.
Changes from1 commit
5e308df86175bf098d35d3d130605b8fa3b4ce84a4a75dbb2390a7ca7e6db79ce913a522a2d0b5e4211840eb24eccf174a1fbef1d44c3726d16f6251babe96ff3c3926d93c56506aa28997dc4324a838dFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2,7 +2,9 @@ package cliui | ||
| import ( | ||
| "flag" | ||
| "fmt" | ||
| "os" | ||
| "slices" | ||
| "sync" | ||
| "time" | ||
| @@ -12,6 +14,10 @@ import ( | ||
| "github.com/coder/pretty" | ||
| ) | ||
| const NoColorFlag = "no-color" | ||
| var NoColor = false | ||
| var Canceled = xerrors.New("canceled") | ||
| // DefaultStyles compose visual elements of the UI. | ||
| @@ -37,21 +43,30 @@ var ( | ||
| ) | ||
| var ( | ||
| Green = Color("10") | ||
| Red = Color("9") | ||
| Yellow = Color("11") | ||
| Blue = Color("12") | ||
| ) | ||
| // Color returns a color for the given string. | ||
| func Color(s string) termenv.Color { | ||
| colorOnce.Do(func() { | ||
| color = termenv.NewOutput(os.Stdout).ColorProfile() | ||
| if flag.Lookup("test.v") != nil { | ||
| // Use a consistent colorless profile in tests so that results | ||
| // are deterministic. | ||
| color = termenv.Ascii | ||
| } | ||
| // Currently it appears there is no way to use the flag from | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Have you tried setting the option on the root command? It should then evaluate on every command. | ||
| // serpent as it isn't possible to create a root middleware that | ||
| // runs for every command. For now we just check if `os.Args` | ||
| // has the flag. | ||
| if slices.Contains(os.Args, fmt.Sprintf("--%s", NoColorFlag)) { | ||
| color = termenv.Ascii | ||
| } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Is this needed? I see that the serpent options modify the global var in this package, so checking That said, I really wish we could do it differently as modifying globals is a bit icky 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Unfortunately not as | ||
| }) | ||
| return color.Color(s) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -461,6 +461,12 @@ func (r *RootCmd) Command(subcommands []*serpent.Command) (*serpent.Command, err | ||
| Value: serpent.StringOf(&r.globalConfig), | ||
| Group: globalGroup, | ||
| }, | ||
| { | ||
| Flag: cliui.NoColorFlag, | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Wonder if it would make sense to also allow setting it as an env ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Yes that should be a good idea. There is a 'standard' around using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Sounds good to me! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Suggestion: I think we should add the env as well here. | ||
| Description: "Disable use of color in CLI output.", | ||
| Value: serpent.BoolOf(&cliui.NoColor), | ||
| Group: globalGroup, | ||
| }, | ||
| { | ||
| Flag: "version", | ||
| // This was requested by a customer to assist with their migration. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -38,6 +38,14 @@ func main() { | ||
| }, | ||
| } | ||
| root.Options = []serpent.Option{ | ||
| { | ||
| Default: "false", | ||
| Flag: cliui.NoColorFlag, | ||
| Value: serpent.BoolOf(&cliui.NoColor), | ||
| }, | ||
| } | ||
| root.Children = append(root.Children, &serpent.Command{ | ||
| Use: "colors", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others.Learn more. This should be hidden There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Done! | ||
| Handler: func(inv *serpent.Invocation) error { | ||