Quote YAML 1.1 bools at encoding time for compatibility with other legacy parsers

[mumoshu]

This is rather a feature request with a proposed implementation!

Hey @goccy, thank you so much for your awesome work on this library ☺️

We sometimes cannot produce YAML documents using this awesome library when the consumer of the produced YAML documents uses legacy YAML parsers.

Apparently, the issue is- We have no way to encode a string on as a string in terms of YAML 1.1.

I'm afraid you might say why you need to care about the legacy YAML versions that this library is NOT intended to support. Indeed... but please read on!

A simple failure scenario is this:

• goccy/go-yaml encodes a string (not bool) "on" as on in the YAML data
• Another legacy yaml parser decodes it as a bool true.

This change addresses that, by quoting YAML 1.1 bool-like strings "on", "off", and so on at the encoding time so that the YAML data produced by the goccy/go-yaml's can be fed into other legacy YAML parsers that support YAML up to 1.1.

I do know go-yaml is intended to support YAML 1.2 only. However, this change does not break that (I think). This just enhances compatibility with legacy parsers without quitting to be a YAML 1.2 only library. I presume YAML 1.2 spec says a string on can be encoded as either on or "on" in the YAML document, and we just prefer to use the latter for compatibility.

[mumoshu]

@goccy We've been using c14e882 in helmfile for a while, and it has been doing a great job so far!

Also, your CI discovered some failures due to tests not being up-to-date with the change- I've fixed that in 506b55e.

[codecov-commenter]

Codecov Report

Merging #354 (506b55e) into master (11ad39b) will decrease coverage by 0.04%.
The diff coverage is 87.50%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #354      +/-   ##
==========================================
- Coverage   75.37%   75.34%   -0.04%     
==========================================
  Files          13       13              
  Lines        4545     4551       +6     
==========================================
+ Hits         3426     3429       +3     
- Misses        864      866       +2     
- Partials      255      256       +1     

[goccy]

First, Thank you for creating a great PR. Also, thank you for the sponsor registration.

I understand what you are saying. Indeed, in order to provide this functionality, we need to support encoding to quoted values.
One point I was wondering is what tools or libraries are intended to be used to read the yaml exported by go-yaml. For example, if you use go-yaml for decoding, you should be fine, but what other parser do you expect to need to use ?

[goccy]

This may be intended to output keywords such as true/false as quoted strings. This is consistent in terms of quoting all values that should be output as boolean. However, we should concern that quoting a value that should originally be output as a boolean in the 1.2 spec will result in a breaking change.

[mumoshu]

we should concern that quoting a value that should originally be output as a boolean in the 1.2 spec will result in a breaking change

Hey @goccy Thank you for your feedback! Just to be sure, I believe this doesn't affect the boolean encoding.

I presume that the existing reservedKeywordMap is supposed to "quote strings" (not bools) so that the results can be decoded back to the original strings.

Similarly, the new reservedEncKeywordMap is intended to "quote strings" (again, not bools) so that other legacy YAML parsers can decode the results without losing types and values.

Your confirmation/correction is much appreciated! Thanks.

[goccy]

I see, only for string type, the quote is added. That seems to work 👍

[yxxhero]

/cc @mumoshu

[goccy]

Thank you for your contribution !! LGTM