Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for handling YAML Merge Key (#41) #1243

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

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)

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.

3 participants