Merge lp:~niemeyer/goyaml/fix-map-init into lp:~gophers/goyaml/trunk

Proposed by Gustavo Niemeyer
Status: Merged
Merged at revision: 32
Proposed branch: lp:~niemeyer/goyaml/fix-map-init
Merge into: lp:~gophers/goyaml/trunk
Diff against target: 0 lines
To merge this branch: bzr merge lp:~niemeyer/goyaml/fix-map-init
Reviewer Review Type Date Requested Status
The Go Language Gophers Pending
Review via email: mp+86396@code.launchpad.net

Description of the change

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Reviewers: mp+86396_code.launchpad.net,

Message:
Please take a look.

Description:

https://code.launchpad.net/~niemeyer/goyaml/fix-map-init/+merge/86396

(do not edit description out of merge proposal)

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

Affected files:
   M decode.go
   M decode_test.go

Index: decode.go
=== <email address hidden> >
<email address hidden>
=== modified file 'decode.go'
--- decode.go 2011-11-24 19:47:20 +0000
+++ decode.go 2011-12-20 13:31:40 +0000
@@ -213,9 +213,9 @@
  //
  // It's a slightly convoluted case to handle properly:
  //
-// - Nil pointers should be zeroed out, unless being set to nil
-// - We don't know at this point yet what's the value to SetYAML() with.
-// - We can't separate pointer deref/init and setter checking, because
+// - nil pointers should be initialized, unless being set to nil
+// - we don't know at this point yet what's the value to SetYAML() with.
+// - we can't separate pointer deref/init and setter checking, because
  // a setter may be found while going down a pointer chain.
  //
  // Thus, here is how it takes care of it:
@@ -417,6 +417,10 @@
   if out.Kind() != reflect.Map {
    return false
   }
+ if out.IsNil() {
+ out.Set(reflect.MakeMap(out.Type()))
+ }
+
   outt := out.Type()
   kt := outt.Key()
   et := outt.Elem()

Index: decode_test.go
=== <email address hidden> >
<email address hidden>
=== modified file 'decode_test.go'
--- decode_test.go 2011-11-24 19:47:20 +0000
+++ decode_test.go 2011-12-20 13:31:40 +0000
@@ -81,16 +81,9 @@

   // Structs and type conversions.
   {"hello: world", &struct{ Hello string }{"world"}},
- {"a: {b: c}", &struct {
- A struct {
- B string
- }
- }{struct{ B string }{"c"}}},
- {"a: {b: c}", &struct {
- A *struct {
- B string
- }
- }{&struct{ B string }{"c"}}},
+ {"a: {b: c}", &struct{ A struct{ B string } }{struct{ B string }{"c"}}},
+ {"a: {b: c}", &struct{ A *struct{ B string } }{&struct{ B string }{"c"}}},
+ {"a: {b: c}", &struct{ A map[string]string
}{map[string]string{"b": "c"}}},
   {"a: 1", &struct{ A int }{1}},
   {"a: [1, 2]", &struct{ A []int }{[]int{1, 2}}},
   {"a: 1", &struct{ B int }{0}},

lp:~niemeyer/goyaml/fix-map-init updated
33. By Gustavo Niemeyer

Minor tweak in ordering.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :
Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM

https://codereview.appspot.com/5503044/diff/1/decode_test.go
File decode_test.go (right):

https://codereview.appspot.com/5503044/diff/1/decode_test.go#newcode86
decode_test.go:86: {"a: {b: c}", &struct{ A map[string]string
}{map[string]string{"b": "c"}}},
should this work with struct{ A *map[string]string } too?

https://codereview.appspot.com/5503044/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

https://codereview.appspot.com/5503044/diff/1/decode_test.go
File decode_test.go (right):

https://codereview.appspot.com/5503044/diff/1/decode_test.go#newcode86
decode_test.go:86: {"a: {b: c}", &struct{ A map[string]string
}{map[string]string{"b": "c"}}},
> should this work with struct{ A *map[string]string } too?

It does work already. Copied the test.

https://codereview.appspot.com/5503044/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Preview Diff

Empty

Subscribers

People subscribed via source and target branches