Add a Display variant to Value. #85
Conversation
valuable/src/value.rs
Outdated
| @@ -104,6 +125,7 @@ macro_rules! value { | |||
| $(#[$attrs])* | |||
| $variant(v) => fmt::Debug::fmt(v, fmt), | |||
| )* | |||
| Display(d) => d.fmt(fmt), | |||
There was a problem hiding this comment.
i wonder if we want to quote this while debug printing it? it's essentially a different way of recording a string, and strings are quoted in fmt::Debug. @carllerche WDYT?
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
This indicates that `Display` values are essentially being recorded as strings, rather than structured data, in the `Debug` output. Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
suggestions for tokio-rs#85
|
Updated with your changes, thank you! |
There was a problem hiding this comment.
this looks good to me, thanks for applying the changes i suggested!
it would be nice to get an approval from @carllerche or @taiki-e before merging this, since i wrote some of the code in it :)
Co-authored-by: Eliza Weisman <[email protected]>
| // XXX(eliza): this, sadly, does not work for `Path`s; the | ||
| // only way to return them as a `Display` impl is | ||
| // `Path::display`, which creates a value owned by _this_ | ||
| // function; we can't return that as a trait object because | ||
| // we're borrowing it from the function's scope rather than | ||
| // from the value itself. |
There was a problem hiding this comment.
#43 (comment) is one of alternatives that can handle this case well.
|
I think it would be preferable to compare this with the approach discussed in #43 before merging. (having both is confusing.) |
|
This is going to be a blocker for fully replacing |
Per discussion in Discord - @hawkw saw this as a desirable feature, to support emitting generic text in tracing without requiring that there be an underlying &str value.
Note that I used the Display output directly in the Debug impl for Value::Display - this seems a little awkward, but I'm not sure of a better option.