Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Quote YAML 1.1 bools during serialization #412

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

NiklasRosenstein
Copy link

@NiklasRosenstein NiklasRosenstein commented Feb 22, 2024

Closes #319

This PR handles pre-YAML 1.2 bool keywords (on, off, yes, no, etc.) during serialization to add single quotes which adds backwards compatibility when the serialized YAML is later loaded by a YAML parser for an older spec.

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

Notable changes

  • Examples/tests often have a field called y which is now serialized with quotes as well (e.g. 'y': 0). I think this is desirable because some YAML implementations can interpret the field as a regular YAML value (and you end up with a boolean key), however I have yet to find that in the YAML spec.
Previous content

This PR modifies crate::de::parse_bool() to treat on/ON/yes/YES/off/OFF/no/NO as bool as per the YAML 1.1 spec. The same change also causes string values with these values to be quoted, which fixes #319.

I couldn't find whether serde-yaml aims to be compatible with a particular YAML spec version; the fact that it did not support on/off/etc. before suggest that it may be targeting YAML 1.2(.2?).

Serializing these words with quotes is backwards compatible, however if we start to deserialize them as bool this may be breaking to consumers of serde-yaml. I would suggest to consider either

  1. We introduce some kind of YamlVersion enum and default to YamlVersion::V1_2, which foregoes matching the on/off/etc. words
  2. We pass a flag to parse_bool() during serialization so that we serialize treating these words as bool but don't during deserialization

@NiklasRosenstein
Copy link
Author

Now I find the test_tag_resolution() test, which makes it pretty clear the crate is targeting YAML 1.2.2. I would still like to go forward with this PR, and at least ensure that the serialized result is backwards compatible with old YAML specs.

@NiklasRosenstein
Copy link
Author

Ok, so now I just add a special case for handling the pre-YAML 1.2.2 keywords in the serializer. This avoids a breaking change on the deserializer while adding backwards compatibility to the serialized output.

@NiklasRosenstein NiklasRosenstein changed the title Interpret on/off/yes/no as bool and serialize their string form with quotes Quote YAML 1.1 bools during serialization Feb 22, 2024
@uschi2000
Copy link

@NiklasRosenstein I suspect you'll have to find a way (cargo feature?) to make the behavior change opt-in. (Assuming here that @dtolnay isn't super keen on rolling out a change that will potentially break a lot of ossified/broken YAML users...) David?

@uschi2000
Copy link

Also, what does this change mean for serde roundtrips? (ie, de(ser(yaml)) ?= yaml) (I'm not sure if there are any roundtrip guarantees at the moment.)

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

Successfully merging this pull request may close these issues.

Preserve double quotes around "yes" and "no" strings
2 participants