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.
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
Replace all ansi_term crates #2923
Replace all ansi_term crates #2923
Changes from 6 commits
d1ed7b0
52ed6f8
577b9ec
7b5846e
02ebaa2
cb7ad77
96ccbe3
2f610df
2de565f
33f7d36
6c94770
e48b305
d8e7bb8
9dd1c71
658eabf
1c13aaf
2891d3f
9c6fe60
4339ec5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
@bkchr
console
API design is not good here. Do you have any suggestions?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.
What is the problem here? I don't see it.
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.
In
ansi_term
, the function can just passColour::Red.bold()
to get red and bold style, butconsole
doesn't provide such fluent API, I separatecolor
andattribute
, and because there are usages do not haveattribute
, so I have to make it optional which feels not good than previous oneThere 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 think this is fine. This is some internal api any way.
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.
The newer
tracing-subscriber
has removedwith_ansi
format support.to keep it simple, I copy the
dimmed
style and replace its usage inwrite!
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 like
console
supports this now? https://docs.rs/console/latest/console/struct.Style.html#method.dimThere 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 doesn't allow to get
prefix
andsuffix
.timer.format_time(writer)?;
will dowrite!
so we can't wrap it withstyle
hereThere 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.
Here is how
console
render ANSI string https://github.com/console-rs/console/blob/master/src/utils.rs#L653-L656There 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.
Yeah I see what you mean now!