Skip to content

Respect fmt trait and flags in Value#400

Merged
KodrAus merged 3 commits into
rust-lang:masterfrom
KodrAus:fix/value-fmt
Jun 5, 2020
Merged

Respect fmt trait and flags in Value#400
KodrAus merged 3 commits into
rust-lang:masterfrom
KodrAus:fix/value-fmt

Conversation

@KodrAus
Copy link
Copy Markdown
Contributor

@KodrAus KodrAus commented Jun 4, 2020

Fixes #395

This PR makes the fmt::Debug and fmt::Display impls on Value respect the trait being used and any flags present. I've added a separate visitor for Display that uses the right trait, and pass the value directly to the formatter instead of going through format_args.

There are some extra tests for formatting and I've shifted a few pieces around to be consistent. Tests won't pass until I fix sval-rs/sval#84 as well.

r? @yoshuawuyts

Comment thread src/kv/value/internal/sval.rs
}
}

struct SvalVisitor<'a, 'b: 'a>(&'a mut sval::value::Stream<'b>);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moved up into the impl block, like the visitors in the fmt module.

Comment thread src/kv/error.rs
}
}

impl From<Error> for io::Error {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed a similar external From impl in #398 that was causing problems with inference.

Copy link
Copy Markdown
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 -- thanks heaps!

@KodrAus KodrAus merged commit 9ae6778 into rust-lang:master Jun 5, 2020
@KodrAus KodrAus deleted the fix/value-fmt branch June 5, 2020 11:21
Fishrock123 added a commit to Fishrock123/femme that referenced this pull request Nov 24, 2020
rust-lang/log#400
was merged upstream and caused this crate's json key-value logging to break.

This is problem for Tide, which uses key-value logging in it's default log middleware.

Fixes: lrlna#17
Refs: http-rs/tide#752
lrlna added a commit to lrlna/femme that referenced this pull request Dec 4, 2020
rust-lang/log#400
was merged upstream and caused this crate's json key-value logging to break.

This is problem for Tide, which uses key-value logging in it's default log middleware.

Fixes: #17
Refs: http-rs/tide#752

Co-authored-by: Irina Shestak <lrlna@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Respect format flags in sval::fmt::debug kv::value::Value constructed from String uses Debug output for Display

3 participants