Skip to content
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

Merged
merged 1 commit into from
Jun 3, 2016

Conversation

asterite
Copy link
Member

@asterite asterite commented Jun 3, 2016

No description provided.

assert_raw %("hello"), %(hello)
assert_raw %(["hello"])
assert_raw %(["hello","world"])
assert_raw %({"hello":"world"})
Copy link
Member

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 :)

Copy link
Member Author

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.

Copy link
Member

@jhass jhass Jun 3, 2016

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 :)

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link
Member

@jhass jhass Jun 3, 2016

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 ;)

Copy link
Member Author

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.

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member Author

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 :-)

@asterite asterite force-pushed the feature/yaml_union branch from 7f8ee06 to 86192ff Compare June 3, 2016 20:11
@asterite asterite merged commit 8bfa519 into master Jun 3, 2016
@asterite asterite deleted the feature/yaml_union branch June 3, 2016 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants