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

Encode strings with YAML 1.1 boolean values with quotes #574

Open
wants to merge 1 commit into
base: v3
Choose a base branch
from

Conversation

dsharp-pivotal
Copy link

To allow better compatibility with YAML 1.1 parsers, when outputting a
string that is equal to one of the YAML 1.1 boolean values ("yes", "no",
"on", "off", etc), enclose the string in quotes. The string will then
parse correctly by either YAML 1.1 or YAML 1.2 parsers.

This change does not affect decoding: Bare yes and no will still be
decoded as strings.

See: #214

To allow better compatibility with YAML 1.1 parsers, when outputting a
string that is equal to one of the YAML 1.1 boolean values ("yes", "no",
"on", "off", etc), enclose the string in quotes. The string will then
parse correctly by either YAML 1.1 or YAML 1.2 parsers.

This change does not affect decoding: Bare `yes` and `no` will still be
decoded as strings.

See: go-yaml#214
@laverya
Copy link
Contributor

laverya commented Feb 6, 2020

Is this functionally different than #490?

(I don't care which fix gets merged. I just want a fix to get merged)

@dsharp-pivotal
Copy link
Author

@laverya Oh, I didn't see your PR.

They look pretty similar, but I see one functional difference: This PR also forces quotes when a node is explicitly tagged as !!str (the diff in encoder.node()). My desire for that specifically is described in mikefarah/yq#341

@laverya
Copy link
Contributor

laverya commented Feb 18, 2020

They both make the same (functional) change to encoder.node though?

		canUsePlain = rtag == strTag && !isBase60Float(s) && !isYaml11Bool(s)

vs

		canUsePlain = rtag == strTag && !isBase60Float(s)

		// Check to see if this is a v1.1 yaml bool string
		// if it is, require quotes to preserve backwards compatibility
		_, is11BoolString := boolMap[s]
		canUsePlain = !is11BoolString && canUsePlain

Not that it matters - as I said before, as long as one gets merged I'm happy.

@dsharp-pivotal
Copy link
Author

@laverya Yeah, in terms of those changes, looks equivalent to me.

@niemeyer Can you take a look?

@diamondburned
Copy link

#583 actually left out the change in node() that this PR includes.

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

Successfully merging this pull request may close these issues.

3 participants