Control Indent Mapping Context

[spgarbet]

Problem: Downstream consumer of libyaml has some divergent formatting changes. Would prefer to use libyaml without modification. Possibly towards using system installed library: vubiostat/r-yaml#102

It was felt important enough to downstream users to change this bit of formatting on output, so this is important to some users.

Goal: Merge this change in with necessary tests, and understand why the formatting directive was necessary.

Needs: (1) Blessing from dev team. (2) Test cases.

This pull requests has the divergence between the two with one line commented out due to breaking two tests. If the line is commented in the following test snippet occurs with 2 broken tests:

...
ok 89 F6MC: More indented lines at the beginning of folded block scalars
not ok 90 F8F9: Spec Example 8.5. Chomping Trailing Lines
# --- data/F8F9/out.yaml	2022-02-14 09:59:24.889902598 -0600
# +++ /tmp/test.out	2022-02-14 09:59:26.145888379 -0600
# @@ -5,4 +5,3 @@
#  keep: |+
#    # text
#  
# -...
ok 91 FP8R: Zero indented block scalar
ok 92 FQ7F: Spec Example 2.1. Sequence of Scalars
ok 93 FTA2: Single block sequence with anchor and explicit document start
ok 94 FUP4: Flow Sequence in Flow Sequence
ok 95 G4RS: Spec Example 2.17. Quoted Scalars
ok 96 G5U8: Plain dashes in flow sequence
ok 97 G992: Spec Example 8.9. Folded Scalar
ok 98 GH63: Mixed Block Mapping (explicit to implicit)
ok 99 H2RW: Blank lines
ok 100 H3Z8: Literal unicode
ok 101 HMK4: Spec Example 2.16. Indentation determines scope
ok 102 HMQ5: Spec Example 6.23. Node Properties
ok 103 HS5T: Spec Example 7.12. Plain Lines
ok 104 HWV9: Document-end marker
ok 105 J5UC: Multiple Pair Block Mapping
ok 106 J7PZ: Spec Example 2.26. Ordered Mappings
ok 107 J7VC: Empty Lines Between Mapping Elements
ok 108 J9HZ: Spec Example 2.9. Single Document with Two Comments
ok 109 JHB9: Spec Example 2.7. Two Documents in a Stream
ok 110 JQ4R: Spec Example 8.14. Block Sequence
ok 111 JS2J: Spec Example 6.29. Node Anchors
ok 112 K3WX: Colon and adjacent value after comment on next line
ok 113 K4SU: Multiple Entry Block Sequence
ok 114 K527: Spec Example 6.6. Line Folding
not ok 115 K858: Spec Example 8.6. Empty Scalar Chomping
# --- data/K858/out.yaml	2022-02-14 09:59:24.894902541 -0600
# +++ /tmp/test.out	2022-02-14 09:59:26.436885084 -0600
# @@ -2,4 +2,3 @@
#  clip: ""
#  keep: |2+
#  
# -...
ok 116 KMK3: Block Submapping
ok 117 L94M: Tags in Explicit Map
...

[spgarbet]

Possibly related to #224

[perlpunk]

The test suite is passing although this was commented out. That seems suspicious.

[spgarbet]

That was a change that exists upstream that I'm trying to harmonize. If I leave that in, it breaks tests. I suspect it may be a reverted change from earlier and can be safely removed.

[spgarbet]

I removed this.

[perlpunk]

See above comment. This is basically a revert from a previous commit, but the test suite doesn't catch that.

[spgarbet]

I pushed this change up.

[abitrolly]

@perlpunk Hi. Any chance to get this reviewed? Looks useful so that libyaml output can pass default yamllint checks https://yamllint.readthedocs.io/en/stable/configuration.html#default-configuration

Ruby .to_yaml converter is also affected ruby/psych#591

[perlpunk]

To me the description of the PR isn't really helpful. It has a lot of text and then some testsuite output diff, but it doesn't show any example of what it is doing.
How does the new output look like for various test cases, and various indentations?
(Note that I'm not a maintainer, just a contributor. I don't make decisions about libyaml behaviour.)
I commented earlier mainly because the original PR did some apparently unrelated changes, and I was wondering why the testsuite didn't complain about this.
So several questions for @ingydotnet here.

[abitrolly]

@perlpunk yea, the diff looks like there an additional flag that disables indentation of sequences. Not sure if it helps ruby/psych#591

[spgarbet]

I have the code from this library copied into a project, r-yaml. The only change at present is this indentation change. If I take it out, my issue bucket lights up with complaints. I NEED this behavior. It'd be nice to enable it somehow either via compile or via flag. Then the library in the r-yaml project would not be divergent.

[abitrolly]

@spgarbet if you can provide an example of output without the patch and with it, it would be most helpful to understand what the PR does.

[skipkayhil]

I think the idea here is good so I want to add some context with the hope that this discussion can continue:

Given a (for example) Ruby object like:

{ "a" => [1, 2, 3] }

When dumping this object to YAML with libyaml 0.2.5, you currently get this output:

---
a:
- 1
- 2
- 3

With this PR (or a similar change which makes the indentless param given to increase_indent false) the output is:

---
a:
  - 1
  - 2
  - 3

It seems like this is a relatively common request in libyaml based yaml libraries:

• python: Incorrect indentation with lists pyyaml#234
• ruby: Linter friendly YAML output ruby/psych#591 (as mentioned above)
• go: go-yaml/yaml@a5ece68 (the mapping_context && !indention was changed to always be false so that mapping value sequences are always indented)

I do think the strategy in this PR is a good solution as it allows the emitter to be configured with either style, and I would love to see this configuration consistently available in other libyaml based libraries in the future.