Skip to content

Conversation

@nlescoua
Copy link

@nlescoua nlescoua commented Nov 8, 2023

Support for YAML Merge keys ( <<: [*dict1, *dict2] ) is added. The merge key is a specific scalar with value << (and tag !!merge) that implies that during node construction, the map (or sequence of maps) are merged into the current map. The priority rules are that each key from maps within the value associated with << are added iff the key is not yet present in the current map (and first map gets higher priority). Test cases have been added accordingly.

Support for YAML Merge keys ( <<: [*dict1, *dict2] ) is added. The merge
key is a specific scalar with value << (and tag !!merge) that implies
that during node construction, the map (or sequence of maps) are merged
into the current map. The priority rules are that each key from maps
within the value associated with  << are added iff the key is not yet
present in the current map (and first map gets higher priority). Test
cases have been added accordingly.
@bradnelson
Copy link

I tested this PR and it appears to work as expected for my project. Thanks for the contribution, @nlescoua !

detail::shared_memory_holder& pMemory) {
const detail::node& const_map_to = map_to;
for (auto j = map_from.begin(); j != map_from.end(); j++) {
detail::node* s = const_map_to.get(*j->first, pMemory);
Copy link

@azat azat May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This compares the shared_ptr of the keys, so it will not work correctly, this can be visible by iterating over keys, here is a fix for this case - #1279

It is based on your PR, with this fix and a test on top - da04fa3 (#1279)

EXPECT_EQ(3, node["z"]["b"].as<int>());
EXPECT_EQ(1, node["z"]["c"].as<int>());

EXPECT_EQ(1, node["w"]["a"].as<int>());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also changed the order of merging, to make it more compatible, see - 2a1aada (#1279)

@nlescoua
Copy link
Author

Superseeded by #1279 -- Thanks @azat

@nlescoua nlescoua closed this Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants