-
Notifications
You must be signed in to change notification settings - Fork 89
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
new: Add colored log levels #1171
Conversation
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.
Thanks a lot for the PR! :)
This is great, but I have a few visual and technical requests:
Visual requests:
- I think we should only color the level (e.g.,
WARN
in[WARN]
) and not the whole line. - For color choice I would prefer if we would follow what tracing-subscriber is doing: tracing-subscriber/src/fmt/format/mod.rs#L1504-L1510
- We should respect the
NO_COLOR
environment variable at build time to disable any colors.
Technical requests:
I think it would be good, if we introduced a newtype ColorLevel(Level)
that implements a colored 'Display' via anstyle
.
anstyle comes with the stdlib, so do you want me to re-create what they do? |
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.
anstyle comes with the stdlib, so do you want me to re-create what they do?
If you disable the default std
feature, it should work:
anstyle = { version = "1", default-features = false }
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.
Thanks, this is looking great. Could you remove the explicit types from the let
bindings unless necessary? :)
Co-authored-by: Martin Kröning <[email protected]>
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.
Thanks! :)
No problem! I'll do similar changes in the other pull request in a little bit |
Closes #905
Also see hermit-os/loader#336