Merge lp://staging/~niemeyer/goyaml/fix-1191981 into lp://staging/goyaml

Proposed by Gustavo Niemeyer
Status: Merged
Merged at revision: 45
Proposed branch: lp://staging/~niemeyer/goyaml/fix-1191981
Merge into: lp://staging/goyaml
Diff against target: 131 lines (+39/-15)
4 files modified
apic.go (+4/-4)
decode_test.go (+28/-4)
scannerc.go (+1/-1)
yamlprivateh.go (+6/-6)
To merge this branch: bzr merge lp://staging/~niemeyer/goyaml/fix-1191981
Reviewer Review Type Date Requested Status
goyaml maintainers Pending
Review via email: mp+170372@code.staging.launchpad.net

Description of the change

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

Reviewers: mp+170372_code.launchpad.net,

Message:
Please take a look.

Description:
Test and fix bug #1191981.

https://code.launchpad.net/~niemeyer/goyaml/fix-1191981/+merge/170372

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M apic.go
   M decode_test.go
   M scannerc.go
   M yamlprivateh.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: apic.go
=== modified file 'apic.go'
--- apic.go 2013-04-30 18:59:24 +0000
+++ apic.go 2013-06-19 14:22:59 +0000
@@ -83,10 +83,10 @@
  // Create a new emitter object.
  func yaml_emitter_initialize(emitter *yaml_emitter_t) bool {
   *emitter = yaml_emitter_t{
- buffer: make([]byte, output_buffer_size),
- raw_buffer: make([]byte, 0, output_raw_buffer_size),
- states: make([]yaml_emitter_state_t, 0, initial_stack_size),
- events: make([]yaml_event_t, 0, initial_queue_size),
+ buffer: make([]byte, output_buffer_size),
+ raw_buffer: make([]byte, 0, output_raw_buffer_size),
+ states: make([]yaml_emitter_state_t, 0, initial_stack_size),
+ events: make([]yaml_event_t, 0, initial_queue_size),
   }
   return true
  }

Index: decode_test.go
=== modified file 'decode_test.go'
--- decode_test.go 2013-05-29 17:23:20 +0000
+++ decode_test.go 2013-06-19 14:22:34 +0000
@@ -300,7 +300,7 @@
    &struct{ B []int }{[]int{1, 2}},
   },

- // BUG #1133337
+ // Bug #1133337
   {
    "foo: ''",
    map[string]*string{"foo": new(string)},
@@ -317,22 +317,46 @@
     B int "-"
    }{1, 0},
   },
+
+ // Bug #1191981
+ {
+ "" +
+ "%YAML 1.1\n" +
+ "--- !!str\n" +
+ `"Generic line break (no glyph)\n\` + "\n" +
+ ` Generic line break (glyphed)\n\` + "\n" +
+ ` Line separator\u2028\` + "\n" +
+ ` Paragraph separator\u2029"` + "\n",
+ "" +
+ "Generic line break (no glyph)\n" +
+ "Generic line break (glyphed)\n" +
+ "Line separator\u2028Paragraph separator\u2029",
+ },
  }

  func (s *S) TestUnmarshal(c *C) {
   for i, item := range unmarshalTests {
    t := reflect.ValueOf(item.value).Type()
    var value interface{}
- if t.Kind() == reflect.Map {
+ switch t.Kind() {
+ case reflect.Map:
     value = reflect.MakeMap(t).Interface()
- } else {
+ case reflect.String:
+ t := reflect.ValueOf(item.value).Type()
+ v := reflect.New(t)
+ value = v.Interface()
+ default:
     pt := reflect.ValueOf(item.value).Type()
     pv := reflect.New(pt.Elem())
     value = pv.Interface()
    }
    err := goyaml.Unmarshal([]byte(item.data), value)
    c.Assert(err, IsNil, Commentf("Item #%d", i))
- c.Assert(value, DeepEquals, item.value)
+ if t.Kind() == reflect.String {
+ c.Assert(*value.(*string), Equals, item.value, Commentf("Item #%d", i))
+ } else {
+ c.Assert(value, DeepEquals, item.value, Commentf("Item #%d", i))
+ }
   }
  }

Index: scannerc.go
=== modified file 'scannerc.go'
--- sc...

Read more...

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

https://codereview.appspot.com/10417043/diff/3001/yamlprivateh.go
File yamlprivateh.go (right):

https://codereview.appspot.com/10417043/diff/3001/yamlprivateh.go#newcode117
yamlprivateh.go:117: return ( // is_break:
how about changing all of these to omit the brackets?

return b[i] == '\r' ||
    etc

https://codereview.appspot.com/10417043/

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

On 2013/06/19 14:57:59, rog wrote:
> https://codereview.appspot.com/10417043/diff/3001/yamlprivateh.go
> File yamlprivateh.go (right):

https://codereview.appspot.com/10417043/diff/3001/yamlprivateh.go#newcode117
> yamlprivateh.go:117: return ( // is_break:
> how about changing all of these to omit the brackets?

> return b[i] == '\r' ||
> etc

That's unrelated to this fix. I've just run go fmt.

https://codereview.appspot.com/10417043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches