-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add YAML::PullParser#read_raw and support parsing unions. Fixes #2714 #2739
Conversation
assert_raw %("hello"), %(hello) | ||
assert_raw %(["hello"]) | ||
assert_raw %(["hello","world"]) | ||
assert_raw %({"hello":"world"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use some more of these, especially for sequence and mapping :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
read_raw
actually produces a JSON string that, because YAML is a superset of JSON, will parse fine. It doesn't return the original string as I don't know how to do it. I'll try the case @spalladino had, see if it can be parsed with a Union, and then I might add a few more specs before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine, I'd just like to make sure stuff like
foo: &foo
- "foo"
- false
bar:
- foo: 1
bar:
<<: *foo
- "hello world\n\nyay"
is converted correctly :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, that doesn't work. I'll fix it to throw an error for the alias, those are not supported in the pull parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, not supported in mapping and also in read_raw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sad to hear but out of scope here. Take out the alias if you want, just wanted to show a more complex example ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, the pull parser receives events, "mapping start" and "sequence start", it doesn't matter in what form you wrote the YAML. That's tested in the regular YAML pull parser specs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not about parsing, it's that the correct string is serialized, since you do manual string building?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but those are the only cases to consider: string, array, hash. There's nothing else in the default YAML pull parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, feel free to add a few more specs if you want :-)
7f8ee06
to
86192ff
Compare
No description provided.