From ee8c6cd4265f85d94ca2d146dbad377430be9d3f Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Thu, 4 Jun 2020 21:12:31 +1000 Subject: [PATCH 1/3] respect fmt trait and flags in Value --- src/kv/value/fill.rs | 16 +++ src/kv/value/impls.rs | 4 +- src/kv/value/internal/fmt.rs | 177 +++++++++++++++++++++++++++------- src/kv/value/internal/sval.rs | 100 +++++++++++-------- 4 files changed, 218 insertions(+), 79 deletions(-) diff --git a/src/kv/value/fill.rs b/src/kv/value/fill.rs index 9642132f4..d14a64258 100644 --- a/src/kv/value/fill.rs +++ b/src/kv/value/fill.rs @@ -145,4 +145,20 @@ mod tests { .expect("invalid value") ); } + + #[test] + fn fill_debug() { + struct TestFill; + + impl Fill for TestFill { + fn fill(&self, slot: &mut Slot) -> Result<(), Error> { + slot.fill_any(42u64) + } + } + + assert_eq!( + format!("{:04?}", 42u64), + format!("{:04?}", Value::from_fill(&TestFill)), + ) + } } diff --git a/src/kv/value/impls.rs b/src/kv/value/impls.rs index 23e8b1bed..4ec26523e 100644 --- a/src/kv/value/impls.rs +++ b/src/kv/value/impls.rs @@ -126,14 +126,14 @@ mod tests { assert_eq!(42i64.to_value().to_string(), "42"); assert_eq!(42.01f64.to_value().to_string(), "42.01"); assert_eq!(true.to_value().to_string(), "true"); - assert_eq!('a'.to_value().to_string(), "'a'"); + assert_eq!('a'.to_value().to_string(), "a"); assert_eq!( format_args!("a {}", "value").to_value().to_string(), "a value" ); assert_eq!( "a loong string".to_value().to_string(), - "\"a loong string\"" + "a loong string" ); assert_eq!(Some(true).to_value().to_string(), "true"); assert_eq!(().to_value().to_string(), "None"); diff --git a/src/kv/value/internal/fmt.rs b/src/kv/value/internal/fmt.rs index 2f7a00e77..d7d1351c5 100644 --- a/src/kv/value/internal/fmt.rs +++ b/src/kv/value/internal/fmt.rs @@ -65,7 +65,68 @@ pub(in kv::value) use self::fmt::{Arguments, Debug, Display}; impl<'v> fmt::Debug for kv::Value<'v> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.visit(&mut FmtVisitor(f)).map_err(|_| fmt::Error)?; + struct DebugVisitor<'a, 'b: 'a>(&'a mut fmt::Formatter<'b>); + + impl<'a, 'b: 'a, 'v> Visitor<'v> for DebugVisitor<'a, 'b> { + fn debug(&mut self, v: &dyn fmt::Debug) -> Result<(), Error> { + fmt::Debug::fmt(v, self.0)?; + + Ok(()) + } + + fn display(&mut self, v: &dyn fmt::Display) -> Result<(), Error> { + fmt::Display::fmt(v, self.0)?; + + Ok(()) + } + + fn u64(&mut self, v: u64) -> Result<(), Error> { + fmt::Debug::fmt(&v, self.0)?; + + Ok(()) + } + + fn i64(&mut self, v: i64) -> Result<(), Error> { + fmt::Debug::fmt(&v, self.0)?; + + Ok(()) + } + + fn f64(&mut self, v: f64) -> Result<(), Error> { + fmt::Debug::fmt(&v, self.0)?; + + Ok(()) + } + + fn bool(&mut self, v: bool) -> Result<(), Error> { + fmt::Debug::fmt(&v, self.0)?; + + Ok(()) + } + + fn char(&mut self, v: char) -> Result<(), Error> { + fmt::Debug::fmt(&v, self.0)?; + + Ok(()) + } + + fn str(&mut self, v: &str) -> Result<(), Error> { + fmt::Debug::fmt(&v, self.0)?; + + Ok(()) + } + + fn none(&mut self) -> Result<(), Error> { + self.debug(&format_args!("None")) + } + + #[cfg(feature = "kv_unstable_sval")] + fn sval(&mut self, v: &dyn super::sval::Value) -> Result<(), Error> { + super::sval::fmt(self.0, v) + } + } + + self.visit(&mut DebugVisitor(f)).map_err(|_| fmt::Error)?; Ok(()) } @@ -73,52 +134,70 @@ impl<'v> fmt::Debug for kv::Value<'v> { impl<'v> fmt::Display for kv::Value<'v> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.visit(&mut FmtVisitor(f)).map_err(|_| fmt::Error)?; + struct DisplayVisitor<'a, 'b: 'a>(&'a mut fmt::Formatter<'b>); - Ok(()) - } -} + impl<'a, 'b: 'a, 'v> Visitor<'v> for DisplayVisitor<'a, 'b> { + fn debug(&mut self, v: &dyn fmt::Debug) -> Result<(), Error> { + fmt::Debug::fmt(v, self.0)?; -struct FmtVisitor<'a, 'b: 'a>(&'a mut fmt::Formatter<'b>); + Ok(()) + } -impl<'a, 'b: 'a, 'v> Visitor<'v> for FmtVisitor<'a, 'b> { - fn debug(&mut self, v: &dyn fmt::Debug) -> Result<(), Error> { - v.fmt(self.0)?; + fn display(&mut self, v: &dyn fmt::Display) -> Result<(), Error> { + fmt::Display::fmt(v, self.0)?; - Ok(()) - } + Ok(()) + } - fn u64(&mut self, v: u64) -> Result<(), Error> { - self.debug(&format_args!("{:?}", v)) - } + fn u64(&mut self, v: u64) -> Result<(), Error> { + fmt::Display::fmt(&v, self.0)?; - fn i64(&mut self, v: i64) -> Result<(), Error> { - self.debug(&format_args!("{:?}", v)) - } + Ok(()) + } - fn f64(&mut self, v: f64) -> Result<(), Error> { - self.debug(&format_args!("{:?}", v)) - } + fn i64(&mut self, v: i64) -> Result<(), Error> { + fmt::Display::fmt(&v, self.0)?; - fn bool(&mut self, v: bool) -> Result<(), Error> { - self.debug(&format_args!("{:?}", v)) - } + Ok(()) + } - fn char(&mut self, v: char) -> Result<(), Error> { - self.debug(&format_args!("{:?}", v)) - } + fn f64(&mut self, v: f64) -> Result<(), Error> { + fmt::Display::fmt(&v, self.0)?; - fn str(&mut self, v: &str) -> Result<(), Error> { - self.debug(&format_args!("{:?}", v)) - } + Ok(()) + } - fn none(&mut self) -> Result<(), Error> { - self.debug(&format_args!("None")) - } + fn bool(&mut self, v: bool) -> Result<(), Error> { + fmt::Display::fmt(&v, self.0)?; + + Ok(()) + } + + fn char(&mut self, v: char) -> Result<(), Error> { + fmt::Display::fmt(&v, self.0)?; + + Ok(()) + } + + fn str(&mut self, v: &str) -> Result<(), Error> { + fmt::Display::fmt(&v, self.0)?; + + Ok(()) + } + + fn none(&mut self) -> Result<(), Error> { + self.debug(&format_args!("None")) + } - #[cfg(feature = "kv_unstable_sval")] - fn sval(&mut self, v: &dyn super::sval::Value) -> Result<(), Error> { - super::sval::fmt(self.0, v) + #[cfg(feature = "kv_unstable_sval")] + fn sval(&mut self, v: &dyn super::sval::Value) -> Result<(), Error> { + super::sval::fmt(self.0, v) + } + } + + self.visit(&mut DisplayVisitor(f)).map_err(|_| fmt::Error)?; + + Ok(()) } } @@ -126,6 +205,8 @@ impl<'a, 'b: 'a, 'v> Visitor<'v> for FmtVisitor<'a, 'b> { mod tests { use super::*; + use crate::kv::value::ToValue; + #[test] fn fmt_cast() { assert_eq!( @@ -142,4 +223,30 @@ mod tests { .expect("invalid value") ); } + + #[test] + fn fmt_debug() { + assert_eq!( + format!("{:?}", "a string"), + format!("{:?}", "a string".to_value()), + ); + + assert_eq!( + format!("{:04?}", 42u64), + format!("{:04?}", 42u64.to_value()), + ); + } + + #[test] + fn fmt_display() { + assert_eq!( + format!("{}", "a string"), + format!("{}", "a string".to_value()), + ); + + assert_eq!( + format!("{:04}", 42u64), + format!("{:04}", 42u64.to_value()), + ); + } } diff --git a/src/kv/value/internal/sval.rs b/src/kv/value/internal/sval.rs index 12d184290..0d76f4e1f 100644 --- a/src/kv/value/internal/sval.rs +++ b/src/kv/value/internal/sval.rs @@ -42,6 +42,48 @@ impl<'s, 'f> Slot<'s, 'f> { impl<'v> sval::Value for kv::Value<'v> { fn stream(&self, s: &mut sval::value::Stream) -> sval::value::Result { + struct SvalVisitor<'a, 'b: 'a>(&'a mut sval::value::Stream<'b>); + + impl<'a, 'b: 'a, 'v> Visitor<'v> for SvalVisitor<'a, 'b> { + fn debug(&mut self, v: &dyn fmt::Debug) -> Result<(), Error> { + self.0 + .fmt(format_args!("{:?}", v)) + .map_err(Error::from_sval) + } + + fn u64(&mut self, v: u64) -> Result<(), Error> { + self.0.u64(v).map_err(Error::from_sval) + } + + fn i64(&mut self, v: i64) -> Result<(), Error> { + self.0.i64(v).map_err(Error::from_sval) + } + + fn f64(&mut self, v: f64) -> Result<(), Error> { + self.0.f64(v).map_err(Error::from_sval) + } + + fn bool(&mut self, v: bool) -> Result<(), Error> { + self.0.bool(v).map_err(Error::from_sval) + } + + fn char(&mut self, v: char) -> Result<(), Error> { + self.0.char(v).map_err(Error::from_sval) + } + + fn str(&mut self, v: &str) -> Result<(), Error> { + self.0.str(v).map_err(Error::from_sval) + } + + fn none(&mut self) -> Result<(), Error> { + self.0.none().map_err(Error::from_sval) + } + + fn sval(&mut self, v: &dyn sval::Value) -> Result<(), Error> { + self.0.any(v).map_err(Error::from_sval) + } + } + self.visit(&mut SvalVisitor(s)).map_err(Error::into_sval)?; Ok(()) @@ -107,48 +149,6 @@ impl Error { } } -struct SvalVisitor<'a, 'b: 'a>(&'a mut sval::value::Stream<'b>); - -impl<'a, 'b: 'a, 'v> Visitor<'v> for SvalVisitor<'a, 'b> { - fn debug(&mut self, v: &dyn fmt::Debug) -> Result<(), Error> { - self.0 - .fmt(format_args!("{:?}", v)) - .map_err(Error::from_sval) - } - - fn u64(&mut self, v: u64) -> Result<(), Error> { - self.0.u64(v).map_err(Error::from_sval) - } - - fn i64(&mut self, v: i64) -> Result<(), Error> { - self.0.i64(v).map_err(Error::from_sval) - } - - fn f64(&mut self, v: f64) -> Result<(), Error> { - self.0.f64(v).map_err(Error::from_sval) - } - - fn bool(&mut self, v: bool) -> Result<(), Error> { - self.0.bool(v).map_err(Error::from_sval) - } - - fn char(&mut self, v: char) -> Result<(), Error> { - self.0.char(v).map_err(Error::from_sval) - } - - fn str(&mut self, v: &str) -> Result<(), Error> { - self.0.str(v).map_err(Error::from_sval) - } - - fn none(&mut self) -> Result<(), Error> { - self.0.none().map_err(Error::from_sval) - } - - fn sval(&mut self, v: &dyn sval::Value) -> Result<(), Error> { - self.0.any(v).map_err(Error::from_sval) - } -} - #[cfg(test)] mod tests { use super::*; @@ -191,4 +191,20 @@ mod tests { .expect("invalid value") ); } + + #[test] + fn sval_debug() { + struct TestSval; + + impl sval::Value for TestSval { + fn stream(&self, stream: &mut sval::value::Stream) -> sval::value::Result { + stream.u64(42) + } + } + + assert_eq!( + format!("{:04?}", 42u64), + format!("{:04?}", kv::Value::from_sval(&TestSval)), + ); + } } From 43d69c196e6de64ee410970b6c0a9338e838c83e Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Thu, 4 Jun 2020 21:13:38 +1000 Subject: [PATCH 2/3] remove unneeded io::Error From impl --- src/kv/error.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/kv/error.rs b/src/kv/error.rs index 45243ca9b..0439b3a55 100644 --- a/src/kv/error.rs +++ b/src/kv/error.rs @@ -67,10 +67,4 @@ mod std_support { Error::boxed(err) } } - - impl From for io::Error { - fn from(err: Error) -> Self { - io::Error::new(io::ErrorKind::Other, err) - } - } } From c89a4a10b6630e45d55d9e839ad1eac2cab1200b Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Thu, 4 Jun 2020 22:27:58 +1000 Subject: [PATCH 3/3] bump sval to 0.5.2 for fmt fixes run fmt --- Cargo.toml | 4 ++-- src/kv/value/impls.rs | 5 +---- src/kv/value/internal/fmt.rs | 5 +---- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ab8a3b57a..9d1364e0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,8 +51,8 @@ kv_unstable_sval = ["kv_unstable", "sval/fmt"] [dependencies] cfg-if = "0.1.2" serde = { version = "1.0", optional = true, default-features = false } -sval = { version = "0.5", optional = true, default-features = false } +sval = { version = "0.5.2", optional = true, default-features = false } [dev-dependencies] serde_test = "1.0" -sval = { version = "0.5", features = ["test"] } +sval = { version = "0.5.2", features = ["test"] } diff --git a/src/kv/value/impls.rs b/src/kv/value/impls.rs index 4ec26523e..a6169f101 100644 --- a/src/kv/value/impls.rs +++ b/src/kv/value/impls.rs @@ -131,10 +131,7 @@ mod tests { format_args!("a {}", "value").to_value().to_string(), "a value" ); - assert_eq!( - "a loong string".to_value().to_string(), - "a loong string" - ); + assert_eq!("a loong string".to_value().to_string(), "a loong string"); assert_eq!(Some(true).to_value().to_string(), "true"); assert_eq!(().to_value().to_string(), "None"); assert_eq!(Option::None::.to_value().to_string(), "None"); diff --git a/src/kv/value/internal/fmt.rs b/src/kv/value/internal/fmt.rs index d7d1351c5..eabd4dd15 100644 --- a/src/kv/value/internal/fmt.rs +++ b/src/kv/value/internal/fmt.rs @@ -244,9 +244,6 @@ mod tests { format!("{}", "a string".to_value()), ); - assert_eq!( - format!("{:04}", 42u64), - format!("{:04}", 42u64.to_value()), - ); + assert_eq!(format!("{:04}", 42u64), format!("{:04}", 42u64.to_value()),); } }