Merge pull request #400 from KodrAus/fix/value-fmt

Respect fmt trait and flags in Value
rust-lang · Jun 5, 2020 · 9ae6778 · 9ae6778
2 parents c5c8e48 + c89a4a1
commit 9ae6778
Show file tree

Hide file tree

Showing 6 changed files with 217 additions and 90 deletions.
diff --git 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/error.rs b/src/kv/error.rs
@@ -67,10 +67,4 @@ mod std_support {
             Error::boxed(err)
         }
     }
-
-    impl From<Error> for io::Error {
-        fn from(err: Error) -> Self {
-            io::Error::new(io::ErrorKind::Other, err)
-        }
-    }
 }
diff --git 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
@@ -126,15 +126,12 @@ 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\""
-        );
+        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::<bool>.to_value().to_string(), "None");

diff --git a/src/kv/value/internal/fmt.rs b/src/kv/value/internal/fmt.rs
@@ -65,67 +65,148 @@ 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(())
     }
 }
 
 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)?;
 
-    #[cfg(feature = "kv_unstable_sval")]
-    fn sval(&mut self, v: &dyn super::sval::Value) -> Result<(), Error> {
-        super::sval::fmt(self.0, v)
+                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 DisplayVisitor(f)).map_err(|_| fmt::Error)?;
+
+        Ok(())
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
+    use crate::kv::value::ToValue;
+
     #[test]
     fn fmt_cast() {
         assert_eq!(
@@ -142,4 +223,27 @@ 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
@@ -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)),
+        );
+    }
 }