Skip to content

Fix arrays merge #139

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

Merged
merged 4 commits into from
Dec 13, 2019
Merged

Conversation

IgorFedchenko
Copy link
Contributor

Close #137

While debugging, it turned out that what current implementation does is taking value by key (create if not exists) and append array values to it.

So, actually, the following:

c: {
    a: [2. 3]
    b: [4]
    a: [5]
}

was ending up with a = [2, 3, 5]. So, arrays with same paths were always concatenating, as if one was continuation of another.

And as far as I understand the specs, the only case when arrays are concatenated is when they are separated with non-newline while space. From README:

Within an field value or array element, if only non-newline white-space separates the end of a first array or object or substitution from the start of a second array or object or substitution, the two values are concatenated.

So for me, this is a special case that should be explicitly handled. Like when you have ] token, check if next tokens are some white-spaces and then [ token - and in that case just skip this part and keep going.

So, that is what basically my fix is doing:

  1. When we find second key with same path (i.e. second a) and there is already existing array, do not concatenate them (I am calling Clear on old value)
  2. When during array parsing we have a special case described above, do the "special thing"

var config = Parser.Parse(hocon);
config.GetIntList("c.a").Should().Equal(new [] { 6 });
config = Parser.Parse(hocon2);
config.GetIntList("c.a").Should().Equal(new [] { 2, 5, 6 });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this to verify that array merging still works

@Aaronontheweb Aaronontheweb merged commit 48888a2 into akkadotnet:dev Dec 13, 2019
@IgorFedchenko IgorFedchenko deleted the fix/child-arrays-merge branch December 13, 2019 13:21
Aaronontheweb pushed a commit that referenced this pull request Dec 27, 2019
* Added failing test

* Updated array concatenation implementation
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.

Merging objects with array fields does not seem to work correctly
2 participants