Do not discard comments at beginning of YAML documents

[felixfontein]

I couldn't find documentation mentioning that the top-of-document comments are discarded (that needs updating). @autrilla you mentioned that this behavior is documented in #695 (comment); can you point me to where?

Also, I couldn't run the functional tests since I don't have Rust set up right now, but I guess they'll run in CI :)

[autrilla]

I don't think it's in the readme, but it's in several issues, e.g. #300, #374, and probably others. I've found a few instances scattered in comments on several other issues. I shouldn't have called it documented, users shouldn't have to go searching in the issues for it.

This also fixes #384

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (develop@13d64c9). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop     #757   +/-   ##
==========================================
  Coverage           ?   34.62%           
==========================================
  Files              ?       23           
  Lines              ?     3076           
  Branches           ?        0           
==========================================
  Hits               ?     1065           
  Misses             ?     1877           
  Partials           ?      134           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13d64c9...3dd7c9a. Read the comment docs.

[autrilla]

Thanks again!

[felixfontein]

@autrilla thanks a lot for reviewing and merging! :)

BTW, is there a schedule/... for when a new release will happen?

[patricknelson]

Since this hasn't been released yet: Bump 😄

Also, since I cannot test this, I want to confirm: Will this retain or discard comments throughout the document? I have a particular use case for ytt (YAML templating) such as an encrypted values file:

#@data/values
---
testing: 'foobar'

And since there are no values above that document divider ---, it could still end up being discarded, but not sure.

[patricknelson]

p.s. If anyone lands here from a google search who also happens to be using both sops and ytt, here's my current hack/workaround:

echo -e "#@data/values\n---\n$(sops -d encrypted-values.yml)" | ytt -f - -f template-file.yml

This prepends the necessary ytt comment and document separator and then uses the STDIN syntax -f -.

[felixfontein]

@patricknelson #791 together with go-yaml/yaml#684 now preserves constructs such as

#@data/values
---
testing: 'foobar'

[patricknelson]

Oh ok cool @felixfontein! I commented here mainly since I assumed that this merged PR would already perform that function. Is there a difference between your example and this new one below when it comes to this PR and the one you created?

# comment
testing: 'foobar'

vs.

# comment but as separate document
---
testing: 'foobar'

I only have to ask since I cannot test this since I'm unable to compile this and get it to work on my machine. So my goal is for this to hopefully get released soon 😄 (pretty please @autrilla 🙏).

[autrilla]

I'm not a project owner, so I can't create releases unfortunately.

[felixfontein]

@patricknelson this PR mainly bumps the version of the modified copy of yaml.v2 that is used by sops, which improves comment parsing. Since comment parsing is only part of the modified copy of yaml.v2, and not of the official yaml.v2, this is kind of a maintenance nightmare, since only sops developers are using this modified copy of yaml.v2 (at least that I'm aware of).

My new PR switches this modified copy of yaml.v2 by the official yaml.v3 API, which does support comments. Unfortunately yaml.v3 is a work in progress itself, has bugs (w.r.t. comment processing) and not that actively developed (just compare https://github.com/go-yaml/yaml/commits/v3 with https://github.com/go-yaml/yaml/issues?q=is%3Aissue+is%3Aopen+v3). But still, since this is used by other projects, the chances of yaml.v3 being improved are higher than our modified copy of yaml.v2 being improved.

I've also created a PR for yaml.v3 which fixes some comment problems, including a bug which causes yaml.v3 to read your construct

#@data/values
---
testing: 'foobar'

as a single document

#@data/values
testing: 'foobar'

(which thus won't allow sops to treat it correctly). So when the yaml.v3 PR is merged (and a new version released), and the sops PR for using yaml.v3 is updated to use the latest yaml.v3 version, my new sops PR should be ready to go.

It's probably better to wait for the yaml.v3 version of sops, instead of releasing a new version with this PR included, to make sure that there's only one breaking change coming up (see #791 (comment)).

@autrilla is @ajvb the only project owner at the moment?

[autrilla]

As far as I know, yes, but I think technically anyone in the @mozilla org has permission to do everything in this repo.

[patricknelson]

Just confirming that I'm using version 3.7.1 (released April 2021) and this feature still isn't yet available after a full year. I know it was merged into develop, but still... this isn't a breaking or backward incompatible, is it? Technically it does change output but I can't see how emitting a comment at the beginning of a YAML document would break things.

@ajvb just curious: what is the process/workflow for incorporating changes from develop into master so that they're ultimately released as an executable that we can use?

[felixfontein]

@patricknelson for me it's working with 3.7.1.

[patricknelson]

@felixfontein Got a little egg on my face here; turns out I was actually using 3.6.1 and not 3.7.1. So the good news is that thankfully I can put comments at the top of YAML files which will help a little bit, however...

I just upgraded and found that my multi-document format (the format I still need it to support from ytt) is doing what you were describing in your last comment, specifically: The comment above the document separator --- ends up coalescing incorrectly into a single document, which isn't valid (as ytt parses these comments and they still have semantic meaning).

#@data/values
---
testing: 'foobar'

as a single document

#@data/values
testing: 'foobar'

So for now I'll still be using my old polyfill script above in the meantime until a solution is found for that. I suppose that is a separate issue unrelated to this ticket?

[patricknelson]

Seems like a separate issue (comments as their own document in a YAML stream), so I created #936 to track/discuss.