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

Make 'y' & 'n' into booleans (true/false) #448

Closed
wants to merge 1 commit into from
Closed

Make 'y' & 'n' into booleans (true/false) #448

wants to merge 1 commit into from

Conversation

esotericpig
Copy link

Issue #443

In order to be compatible with Ruby's original Syck library, Psych made y/n default to strings. However, now, the Syck library is dead, and I think it makes since to make Psych compliant with YAML spec v1.1.

In the issue, this caused a problem when Psych produced a non-compliant file that then broke in a compliant parser in another language. I think it's important that Psych makes YAML files that can be consumed by anything that speaks YAML.

https://yaml.org/type/bool.html

However, this is a potentially breaking change. If previous gems relied on y/n as strings, they will now need to be updated to use single/double quotes. This is rare, but should be considered, especially for YAML common as this:

MySprite:
  x: 100
  y: 100
  z: 1

It will need to be updated to this:

MySprite:
  x: 100
  'y': 100
  z: 1

Maybe this should be a major version change? However, these gems relied on a broken/non-compliant parser, so it's a difficult decision to make.

Instead of this solution, we could...

  • add some type of strict_compliance keyword arg option,
  • or, output a deprecated warning to users for a couple of versions to update their YAML files.

- https://yaml.org/type/bool.html
- This breaks compatibility with Ruby's original Syck library, but it
  complies with YAML spec v1.1.
- This makes it cross-compatible with other parsers, producing
  standards-compliant YAML files.
@esotericpig
Copy link
Author

The unsuccessful builds on Windows don't seem to be a problem from my code.

@tenderlove
Copy link
Member

I think a strict mode would probably be good. We were talking about it in #426

I'm reluctant to change the behavior without asking people to "opt-in" because people are no doubt depending on this behavior. Even doing it on a major version bump is a tough sell. From a consumer's perspective they would think "I have to audit all of my YAML documents when I do this upgrade, but what do I get out of the upgrade?". Unless we can make the upgrade transparent (IOW users don't need to do anything), I'm not partial to a change like this as the default behavior.

@esotericpig
Copy link
Author

A strict mode sounds good to me. I'll close this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants