Fix bug with resolving merging scalars
This request adds a patch to the problem I described in topic #814
I went back to the BaseConstructor class and analyzed method construct_mapping. What I ended up with was a new method, which I put in a safe constructor since it involved tags.
I described both the case of merging with a single scalar and with an array of scalars, each of which, after construction, should become a dictionary. Scalars are merged sequentially and in the order in which they appear in the Yaml code.
I hope I was able to create a simple code.
There are existing issues about this, and it is not that trivial to fix it. I tried it myself but my code isn't ready yet.
There are a few problems with the PR as you can see from the test failures (although the actual failures are hard to find in those very verbose stacktraces):
Handling of value tags
You removed handling of tag:yaml.org,2002:value completely. Although probably rarely used, this needs to stay. This is responsible for most of the test failures.
The order of the merge key inserting is now wrong
Example:
- &defaults
a: default a
b: default b
c: default c
- &defaults2
a: default2 a
b: default2 b
c: default2 c
- a: new a
<<: [*defaults, *defaults2]
<<: { INLINE: MAPPING }
d: new d
- a: new a
<<: [*defaults2, *defaults]
<<: { INLINE: MAPPING }
d: new d
The expected result is (dumped as YAML for readability):
- a: default a
b: default b
c: default c
- a: default2 a
b: default2 b
c: default2 c
- INLINE: MAPPING
a: new a
b: default b
c: default c
d: new d
- INLINE: MAPPING
a: new a
b: default2 b
c: default2 c
d: new d
With this PR, I get:
- a: default a
b: default b
c: default c
- a: default2 a
b: default2 b
c: default2 c
- a: default2 a
b: default2 b
c: default2 c
d: new d
- a: default a
b: default b
c: default c
d: new d
1. INLINE: MAPPING is missing completely
2. Order of aliases in a sequence
In <<: [*defaults2, *defaults], *defaults2 should win over *defaults. The PR handles it in the wrong order
Also, when there are multiple merge keys, e.g.
<<: [*A, *B]
<<: *C
it should be handled as if it was
<<: [*A, *B, *C]
3. Precedence of inline keys
- a: new a
<<: *foo
d: new d
For this the inline a: new a and d: new d should both win. Merge keys have lower priority. The position of the merge key(s) in the mapping doesn't matter at all.
That's why in the original code, the merge keys are collected first and then are together put at the beginning of the mapping nodes list at the end of flatten_mapping.
Also, flatten_mapping should stay its own function.
Also it would be good to actually have a test case for this (it doesn't need to implement an include, just a constructor that returns a mapping).
See also https://yaml.org/type/merge.html