Detect duplicate keys of mapping nodes #141
Conversation
…in a mapping node
…icate keys in a mapping node are handled
There was a problem hiding this comment.
I'm fine with this approach, but given the clarity of the spec, this does seem like a clear bug, which IMO could be fixed in a patch release even if it breaks existing code - that code shouldn't work.
That said, I can also see the argument for this approach which gives folks a flashing red light, but doesn't stop their code from running. I don't feel strongly one way or the other.
…epending on `strict_unique_keys` Co-authored-by: Kevin Bonham <[email protected]>
|
Depending on implementation details, this may or may not conflict with the override functionality of merge tags, https://yaml.org/type/merge.html. Edit: It's probably fine since there's a merge test which doesn't fail. |
|
Can this be merged? |
Update Project.toml [compat] to restrict YAML version to YAML = "0.4.7 - 0.4.12" This is a workaround for a breaking change in YAML v0.4.13 that breaks merge keys JuliaData/YAML.jl#141 JuliaData/YAML.jl#243
Update Project.toml [compat] to restrict YAML version to YAML = "0.4.7 - 0.4.12" This is a workaround for a breaking change in YAML v0.4.13 that breaks merge keys PR with the change is: JuliaData/YAML.jl#141 Reported as issue: JuliaData/YAML.jl#243
|
Apologies if this is not the right place to raise this but: would it be possible to expose the |
Oh, well, it definitely does interfere.
It doesn't fail but produces lots of |
This (at least partially) fixes #140 by checking for the existence of a
keybefore inserting it intomapping, which requires thedicttypeto implementsetindex!(since it's inside thetry catchI thought it was safe to just assume that all "proper dicttypes" should support that, would otherwise just get caught).I suppose that "bug fix" could be potentially seen as breaking change to some code bases (but maybe a good one?), which is why I've opted to do it in a non-breaking way, defaulting to not throwing and sticking to the previous behavior. Could be worth to consider always throwing here though...