Reviewers: mp+96669_code.launchpad.net,

Message:
Please take a look.

Description:


https://code.launchpad.net/~niemeyer/goyaml/interfaces-are-immutable-values/+merge/96669

(do not edit description out of merge proposal)


Please review this at https://codereview.appspot.com/5784060/

Affected files:
   A .lbox
   M decode.go
   M decode_test.go


Index: .lbox
=== <email address hidden> >  
<email address hidden>
=== added file '.lbox'
--- .lbox	1970-01-01 00:00:00 +0000
+++ .lbox	2012-03-08 21:51:14 +0000
@@ -0,0 +1,1 @@
+propose -cr -for=lp:goyaml


Index: decode.go
=== <email address hidden> >  
<email address hidden>
=== modified file 'decode.go'
--- decode.go	2011-12-21 17:47:08 +0000
+++ decode.go	2012-03-08 21:50:15 +0000
@@ -375,11 +375,11 @@
  	if set := d.setter("!!seq", &out, &good); set != nil {
  		defer set()
  	}
+	var iface reflect.Value
  	if out.Kind() == reflect.Interface {
  		// No type hints. Will have to use a generic sequence.
-		iface := out
+		iface = out
  		out = settableValueOf(make([]interface{}, 0))
-		iface.Set(out)
  	}

  	if out.Kind() != reflect.Slice {
@@ -394,6 +394,9 @@
  			out.Set(reflect.Append(out, e))
  		}
  	}
+	if iface.IsValid() {
+		iface.Set(out)
+	}
  	return true
  }



Index: decode_test.go
=== <email address hidden> >  
<email address hidden>
=== modified file 'decode_test.go'
--- decode_test.go	2011-12-21 17:47:08 +0000
+++ decode_test.go	2012-03-08 21:50:15 +0000
@@ -41,7 +41,7 @@
  	{"fixed: 685_230.15", map[string]interface{}{"fixed": 685230.15}},
  	//{"sexa: 190:20:30.15", map[string]interface{}{"sexa": 0}}, //  
Unsupported
  	{"neginf: -.inf", map[string]interface{}{"neginf": math.Inf(-1)}},
-	{"notanum: .NaN", map[string]interface{}{"notanum": math.NaN()}},
+	//{"notanum: .NaN", map[string]interface{}{"notanum": math.NaN()}}, //  
Equality of NaN fails.
  	{"fixed: 685_230.15", map[string]float64{"fixed": 685230.15}},

  	// Bools from spec
@@ -136,11 +136,18 @@
  			value = pv.Interface()
  		}
  		err := goyaml.Unmarshal([]byte(item.data), value)
-		c.Assert(err, IsNil, Bug("Item #%d", i))
-		c.Assert(value, Equals, item.value)
+		c.Assert(err, IsNil, Commentf("Item #%d", i))
+		c.Assert(value, DeepEquals, item.value)
  	}
  }

+func (s *S) TestUnmarshalNaN(c *C) {
+	value := map[string]interface{}{}
+	err := goyaml.Unmarshal([]byte("notanum: .NaN"), &value)
+	c.Assert(err, IsNil)
+	c.Assert(math.IsNaN(value["notanum"].(float64)), Equals, true)
+}
+
  var unmarshalErrorTests = []struct {
  	data, error string
  }{
@@ -155,7 +162,7 @@
  	for _, item := range unmarshalErrorTests {
  		var value interface{}
  		err := goyaml.Unmarshal([]byte(item.data), &value)
-		c.Assert(err, ErrorMatches, item.error, Bug("Partial unmarshal: %#v",  
value))
+		c.Assert(err, ErrorMatches, item.error, Commentf("Partial  
unmarshal: %#v", value))
  	}
  }

@@ -198,9 +205,9 @@
  		err := goyaml.Unmarshal([]byte(item.data), obj)
  		c.Assert(err, IsNil)
  		c.Assert(obj.Field, NotNil,
-			Bug("Pointer not initialized (%#v)", item.value))
+			Commentf("Pointer not initialized (%#v)", item.value))
  		c.Assert(obj.Field.tag, Equals, item.tag)
-		c.Assert(obj.Field.value, Equals, item.value)
+		c.Assert(obj.Field.value, DeepEquals, item.value)
  	}
  }

@@ -211,7 +218,7 @@
  	c.Assert(obj.tag, Equals, setterTests[0].tag)
  	value, ok := obj.value.(map[interface{}]interface{})
  	c.Assert(ok, Equals, true)
-	c.Assert(value["_"], Equals, setterTests[0].value)
+	c.Assert(value["_"], DeepEquals, setterTests[0].value)
  }

  func (s *S) TestUnmarshalWithFalseSetterIgnoresValue(c *C) {