feat: configurable threshold and colorful output #6
Conversation
|
Please run the |
BugenZhao
left a comment
BugenZhao
left a comment
There was a problem hiding this comment.
Thanks for your contribution! May run cargo fmt and cargo clippy to pass the checks.
.idea/workspace.xml
Outdated
There was a problem hiding this comment.
Would you please remove this file?
There was a problem hiding this comment.
Yeah. I would do this, sorry for irrelevant file.
| pub struct Config { | ||
| /// Whether to include the **verbose** span in the await-tree. | ||
| verbose: bool, | ||
| pub verbose: bool, |
There was a problem hiding this comment.
We may keep these fields private (or pub(crate)) and use ConfigBuilder to construct a Config.
There was a problem hiding this comment.
We may keep these fields private (or
pub(crate)) and useConfigBuilderto construct aConfig.
Current config is simple, only three fields, but the disign pattern is up to you. Maybe the current version is enough.
There was a problem hiding this comment.
We already have ConfigBuilder now (by deriving Builder) so there's no extra effort required. :)
|
|
||
| /// Whether to coloring the terminal | ||
| colored: bool, | ||
|
|
||
| /// if the time of execution is beyond it, warn it | ||
| warn_threshold: time::Duration, |
There was a problem hiding this comment.
What about directly storing a Config here?
There was a problem hiding this comment.
What about directly storing a
Confighere?
If put Config here, the field verbose is not used by context.
There was a problem hiding this comment.
Yeah, you're right. So I'm considering whether colored and warn_threshold should be properties of tree formatting, instead of the tree or the registry. 🤔 For example, we have...
struct FormatConfig {
colored: bool,
warn_threshold: time::Duration,
}
pub struct FormatTree<'a> {
tree: &'a Tree,
config: FormatConfig
}
impl Display for FormatTree<'_> { .. }
impl Tree {
pub fn display(&self, config: FormatConfig) -> FormatTree<'_> { .. }
}
impl Display for Tree { /* delegate to `self.display(FormatConfig::default()).fmt(f)` */ }There was a problem hiding this comment.
Yes, It could be done this way.
Co-authored-by: Bugen Zhao <[email protected]>
Co-authored-by: Bugen Zhao <[email protected]>
Made it~ |
|
Hey, what is the current progress of this MR? Is there something need to do from my side? |
| pin-project = "1" | ||
| tokio = { version = "1", features = ["rt"] } | ||
| tracing = "0.1" | ||
| colored = "2" |
There was a problem hiding this comment.
Maybe we should hide this under a feature?
There was a problem hiding this comment.
Yeah, both LGTM. They are about the balance between performance and convenience.
| pin-project = "1" | ||
| tokio = { version = "1", features = ["rt"] } | ||
| tracing = "0.1" | ||
| colored = "2" |
BugenZhao
changed the title
3 participants
The time of warn threshlod may be configurable and colorable.