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

v3 marshalling of "boolean" strings breaks YAML 1.2 interoperability. #582

Closed
tcolgate opened this issue Mar 12, 2020 · 5 comments
Closed

Comments

@tcolgate
Copy link
Contributor

tcolgate commented Mar 12, 2020

In v2:

package main

import (
        "fmt"
        "log"

        yaml "gopkg.in/yaml.v2"
        //yaml "gopkg.in/yaml.v3"
)

func main() {
        data := struct {
                ID string `yaml:"id"`
        }{ID: "off"}
        bs, err := yaml.Marshal(data)
        if err != nil {
                log.Fatalf("err: %v", err)
        }
        fmt.Println(string(bs))
}

this prints:

id: "off"

in v3:

id: off

The field is a string, and should marshal, and re-unmarshal as one.

@tcolgate
Copy link
Contributor Author

I'm pretty confused now, I can't recreate this in the tests. I've checked the commit I'm using, it's a6ecf24 , which is the same as the checkout I'm running the tests on.

@tcolgate
Copy link
Contributor Author

okay, the test I added was bad. it works if the string is "true", it doesn't work if the string is "off"

@tcolgate
Copy link
Contributor Author

okay, it has just occurred to me as to what is happening here. I saw a comment around the yaml spec change for these values, and you are now only handling true/false with the quote enforced marshaling.
This means the output effectively needs to be read with a reader that also adheres to the spec, and cleanly.
I'm not really sure how/if to fix this. Ideally we'd marshal these out as quoted, because it fixes my problem, but the world doesn't revolve around me.

@tcolgate
Copy link
Contributor Author

So, in a sort of begging/pleading sense, I would suggest that marshaling the old 1.2 "boolean" strings as quoted strings would not, strictly speaking, be a break from the spec maybe?

@niemeyer
Copy link
Contributor

Yes, you're right and the change makes sense. We should quote strings that match 1.1 booleans.

@tcolgate tcolgate changed the title v3 incorrectly marshals "boolean" strings v3 marshalling of "boolean" strings breaks YAML 1.2 interoperability. Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants