-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support the NO_COLOR env variable to suppres any colored output. #1594
Conversation
|
||
opts := backend.DisplayOptions{ | ||
Color: cmdutil.GetGlobalColorization(), | ||
} |
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.
For command that do not support a --color flag, this is the pattern to use. The color picked here will be "Never" if NO_COLOR is set in the environment. Or 'Always' otherwise. In the future we can consider if these command shoudl support --color as well. But i didn't want to have to add that here.
@@ -69,7 +75,7 @@ func newCancelCmd() *cobra.Command { | |||
|
|||
msg := fmt.Sprintf("%sThe currently running update for '%s' has been canceled!%s", colors.SpecAttention, s.Name(), | |||
colors.Reset) | |||
fmt.Println(colors.ColorizeText(msg)) |
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.
no one should call colors.ColorizeText now. Only the very lowest level (that is calling into 'lorely') now does this.
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.
Is it possible to stop exporting it? A brief look at the code makes me think we may be able to do but I am not sure.
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.
Yup. I've stopped exporting it.
Color: cmdutil.GetGlobalColorization(), | ||
} | ||
|
||
s, err := requireStack(stack, false, opts) |
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.
Many interactive functions now take in opts so they can decide if they should colorize or not.
Related issues this may close |
Would it be possible as part of this to make the --color flag global across all commands? |
Yes. I can look into that. |
Making this a global flag worked, and def cleaned things up. |
Note: at this point... passing the variable around serves almost no purpose. It's effectively just a process-wide global that is set right at startup... I'm going to keep as is. but we could literally just remove all usages of passing things around... |
|
||
// GetGlobalColorization gets the global setting for how things should be colored. | ||
// This is helpful for the parts of our stack that do not take a DisplayOptions struct. | ||
func GetGlobalColorization() colors.Colorization { |
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.
I would have assumed this would have to check the global c value you in the root pulumi command? Is that done some other place?
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.
it's somewhat the reverse. the pulumi command just goes and sets this, so that it's available for anyone to go find if they need.
That's what i meant in my comment about us not needing to really pass along any 'color' option anymore. There is always just a well known place it can be found.
No description provided.