kv::value::Value constructed from String uses Debug output for Display

yoshuawuyts commented

2020-05-18 14:50:35 -05:00

(Migrated from github.com)

@isgj reported an issue in Tide https://github.com/http-rs/tide/pull/495#discussion_r426254201 that led us to file in issue in Femme https://github.com/lrlna/femme/issues/12 about output that was being printed with extra quotes:

{"level":30,"time":1589717147052,"msg":"Response sent","method":"\"GET\"","status":"200","duration":"\"157ms\"","path":"\"/api/v1/users\""}

The printing logic in femme looks like this:

    impl<'kvs, 'a> kv::Visitor<'kvs> for Visitor<'a> {
        fn visit_pair(
            &mut self,
            key: kv::Key<'kvs>,
            val: kv::Value<'kvs>,
        ) -> Result<(), kv::Error> {
            write!(self.string, "\n    {} {}", style(key).bold(), val).unwrap();
            Ok(())
        }

It turns out that the problem stems from the Display path using the Debug logic: https://docs.rs/log/0.4.8/src/log/kv/value/internal.rs.html#111-117. Currently in master this code seems refactored, but still inherently the same: https://github.com/rust-lang/log/blob/master/src/kv/value/internal/mod.rs#L49-L51.

We should probably introduce a new Display path for values so that they can correctly be displayed. Thanks!

cc/ @KodrAus

@isgj reported an issue in Tide https://github.com/http-rs/tide/pull/495#discussion_r426254201 that led us to file in issue in Femme https://github.com/lrlna/femme/issues/12 about output that was being printed with extra quotes: ```json {"level":30,"time":1589717147052,"msg":"Response sent","method":"\"GET\"","status":"200","duration":"\"157ms\"","path":"\"/api/v1/users\""} ``` The printing logic in `femme` looks like this: ```rust impl<'kvs, 'a> kv::Visitor<'kvs> for Visitor<'a> { fn visit_pair( &mut self, key: kv::Key<'kvs>, val: kv::Value<'kvs>, ) -> Result<(), kv::Error> { write!(self.string, "\n {} {}", style(key).bold(), val).unwrap(); Ok(()) } ``` It turns out that the problem stems from the `Display` path using the `Debug` logic: https://docs.rs/log/0.4.8/src/log/kv/value/internal.rs.html#111-117. Currently in master this code seems refactored, but still inherently the same: https://github.com/rust-lang/log/blob/master/src/kv/value/internal/mod.rs#L49-L51. We should probably introduce a new `Display` path for values so that they can correctly be displayed. Thanks! cc/ @KodrAus

KodrAus commented

2020-06-03 23:08:17 -05:00

(Migrated from github.com)

Ah yes, that’s a bug we should fix up!

I’m spending a bit of time on the kv side of log now so will dig into it unless somebody else beats me to it.

Ah yes, that’s a bug we should fix up! I’m spending a bit of time on the `kv` side of `log` now so will dig into it unless somebody else beats me to it.

KodrAus commented

2020-06-04 00:01:29 -05:00

(Migrated from github.com)

Somewhat related, we should make sure we respect formatting flags too.

quininer commented

2020-07-03 06:51:20 -05:00

(Migrated from github.com)

Can you release a new version for this?

Rows
Columns

kv::value::Value constructed from String uses Debug output for Display #395