-
Notifications
You must be signed in to change notification settings - Fork 453
std.parseYaml fails on non-standard yaml feature, supported in other implementations #1109
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
Comments
I'm upgrading Rapid YAML in #1134, which fixes the segfault. However the behaviour still doesn't match go-jsonnet due to other implementation differences. Specifically, (C++) jsonnet relies on Rapid YAML to emit JSON (ie, to do a direct YAML to JSON conversion) and then parses the output of that just like it would parse any other JSON. But when Rapid YAML converts the input So the 'type' of the value is lost. We would need to rewrite the |
I was looking at google/jsonnet#1109 and realized that we did not parse yaml correctly for primitive types: `std.parseYaml("0777")` returned `[511]` instead of `511` (something that other implementations of jsonnet do)
Same thing seems to happen with the following yaml:
Even when the ip is set to be in quotes, unfortunately |
Note that go-jsonnet seems to handle that just fine. |
Consider this input:
std.parseYaml("0777")
cpp-jsonnet outputs:
go-jsonnet outputs:
jrsonnet outputs:
sjsonnet outputs:
But this is caused by sjsonnet only supporting objects in parseYaml, so
std.parseYaml("a: 0777")
works:Why does it happens?
Well, there is two yaml specs: yaml 1.1, and yaml 1.2, and even if it is a minor update by semver... It isn't a semver.
The one breaking change in yaml 1.2, is that octal literals in
0777
form are now forbidden,0o777
should be used instead.However, many yaml implementations are not following the spec, especially golang, which will even output octals in
0777
format: go-yaml/yaml#420For jrsonnet, I had to modify serde-yaml rust library to also accept this input: dtolnay/serde-yaml#225
And rapidyaml did not implemented this compatibility feature: biojppm/rapidyaml#291
The text was updated successfully, but these errors were encountered: