Repository: 3Hren/msgpack-rust
Pull request #171 (2018-07-24 11:13:49)

[WIP] Provide blanket From impls for homogeneous collections
First shot at https://github.com/3Hren/msgpack-rust/issues/167.

This creates a wrapper type (all names non-final, of course) together with a `From` impl for `Value`. If this is done, we should make all function that take a `Value` take a `T: Into<Value>` instead, so we can directly pass the wrapper type instead having to call `.into()` on it beforehand.

This is a non-breaking change as far as I can see.

I've not connected all the dots, I wanted to hear what you think. I'll offer another alternative at the issue I opened.

KillTheMule: I spoke to soon, I missed something and I'm not sure I can fix it. Closing for now.
KillTheMule: Ok, here's the next shot. Please ignore the deletions at the beginning of `lib.rs`, I'll fix this later. If this is how you imagined it, I'll also implement this for `ValueRef` and take care of formatting.

We've also won a bit of generality, since the conversion for [`&' [ ]` and `Cow`](https://github.com/3Hren/msgpack-rust/pull/171/files#diff-10e8cdae60578a9dace4968576e1fa26R880) now take a generic argument, which is nice.
KillTheMule: I'd like to revive this PR. Seeing the maintainership of this crate has changed a bit, any thoughts on this, @kornelski?
kornelski: I don't like changing serialization from binary to an array of integers. This is a breaking change, and an unexpected and undesirable behavior.

Other than that conversion from other collections to value may be useful. I suggest using `FromIterator` for this purpose. 
KillTheMule: Ok, I understand not wanting to break anything. Unfortunatey, using `FromIterator` will defeat my original purpose, namely having an impl that works recursively.

Nevertheless, this seems usefull, so I'll try to implement it anyways.

Patch:
diff --git a/rmpv/src/lib.rs b/rmpv/src/lib.rs
index 6e63562..a7fa285 100644
--- a/rmpv/src/lib.rs
+++ b/rmpv/src/lib.rs
@@ -13,6 +13,7 @@ use std::borrow::Cow;
 use std::fmt::{self, Debug, Display};
 use std::ops::Index;
 use std::str::Utf8Error;
+use std::iter::FromIterator;
 
 use num_traits::NumCast;
 
@@ -994,6 +995,17 @@ impl From<Vec<(Value, Value)>> for Value {
     }
 }
 
+/// Note that an `Iterator<Item = u8>` will be collected into an
+/// [`Array`](crate::Value::Array), rather than a
+/// [`Binary`](crate::Value::Binary)
+impl<V> FromIterator<V> for Value 
+where V: Into<Value> {
+  fn from_iter<I: IntoIterator<Item = V>>(iter: I) -> Self {
+    let v: Vec<Value> = iter.into_iter().map(|v| v.into()).collect();
+    Value::Array(v)
+  }
+}
+
 impl Display for Value {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
         match *self {
@@ -1257,6 +1269,17 @@ impl<'a> From<Vec<ValueRef<'a>>> for ValueRef<'a> {
     }
 }
 
+/// Note that an `Iterator<Item = u8>` will be collected into an
+/// [`Array`](crate::Value::Array), rather than a
+/// [`Binary`](crate::Value::Binary)
+impl<'a, V> FromIterator<V> for ValueRef<'a>
+where V: Into<ValueRef<'a>> {
+  fn from_iter<I: IntoIterator<Item = V>>(iter: I) -> Self {
+    let v: Vec<ValueRef<'a>> = iter.into_iter().map(|v| v.into()).collect();
+    ValueRef::Array(v)
+  }
+}
+
 impl<'a> From<Vec<(ValueRef<'a>, ValueRef<'a>)>> for ValueRef<'a> {
     fn from(v: Vec<(ValueRef<'a>, ValueRef<'a>)>) -> Self {
         ValueRef::Map(v)


Test patch:
diff --git a/rmpv/tests/value.rs b/rmpv/tests/value.rs
index c035310..124ea1e 100644
--- a/rmpv/tests/value.rs
+++ b/rmpv/tests/value.rs
@@ -122,6 +122,24 @@ fn from_f64() {
     assert_eq!(Value::F64(3.1415), Value::from(3.1415f64));
 }
 
+#[test]
+fn from_iterator() {
+    let v: Vec<u8> = vec![0u8, 1u8, 2u8];
+    let w: Value = v.into_iter().collect();
+
+    let w2 = Value::Array(vec![
+                         Value::from(0u8),
+                         Value::from(1u8),
+                         Value::from(2u8)
+                         ]); 
+
+    assert_eq!(w, w2);
+
+    let w3 = Value::Binary(vec![0u8, 1u8, 2u8]);
+
+    assert!(w != w3);
+}
+
 #[test]
 fn is_nil() {
     assert!(Value::Nil.is_nil());