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
✨ Add --no-color
option to disable colored output on console
#4402
Conversation
Closes OctoPrint#4396 Also removed the setting of INFO logs to white, instead it will be left as default. If there is a light-colored background (such as white) then it becomes unreadable. Can then just use whatever user has configured for their default and not worry about contrast issues.
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.
Looks okay to me, although weirdly different from existing code in a tiny detail.
src/octoprint/cli/server.py
Outdated
@@ -307,6 +309,7 @@ def get_value(key): | |||
safe_mode = "flag" if get_value("safe_mode") else None | |||
ignore_blacklist = get_value("ignore_blacklist") | |||
overlays = get_value("overlays") | |||
no_color = bool(get_value("no_color")) |
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.
Why are we changing the style of boolean? The safe mode uses None-not-None, and we have Python bool for no-color. Seems perhaps unnecessarily inconsistent, if I may bikeshed a little.
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.
safe_mode
is a special case, as it doesn't set a boolean value but rather a source of the safe mode activation. If it is anything but None
, OctoPrint will run in safe mode, but it will log the reason for that based on the value of this flag.
However, the explicit cast to bool
should indeed not be needed here, as this should already be a boolean thanks to being marked with is_flag=True
in the click def.
That way we get support for NO_COLOR as well: https://no-color.org/
What does this PR do and why is it necessary?
Add
--no-color
option to disable colored output on console.Closes #4396
Also removed the setting of INFO logs to white, instead it will be left as default. If there is a light-colored background (such as white) then it becomes unreadable. Can then just use whatever user has configured for their default and not worry about contrast issues.
How was it tested? How can it be tested by the reviewer?
Manually by starting with and without the flag.