[TwigBundle] fix configuration tree for paths

[cordoval]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#8171
License	MIT
Doc PR	na

This is a joint effort with @mdavis1982 and @cordoval 👶 pairing up and warming for hacking day in Warsaw

[jakzal]

Should be removed.

[cordoval]

@stof if you have suggestions besides the name of the keys would be great

other than that this fixes the other issue so ready to merge @fabpot

[fabpot]

Could you also add a test to cover the fix?

[cordoval]

let's chief

👶

of course after travis is green @fabpot

[cordoval]

ping @fabpot

[cordoval]

@fabpot fixing things related, would be great if you display the branch name on the fabbot related pages so to make them handy

[cordoval]

@fabpot all good here, WCM, even though @fabbot complains about some grammar issues which is nonsense 👶

[cordoval]

bump @fabpot

[fabpot]

@cordoval Can you rebase on current master? Also, what about addressing the comment from @stof?

[cordoval]

@fabpot rebased

@stof could you please clarify your comment #9719 (comment)?

[fabpot]

Any news on this one?

[stof]

👍 The useAttributeAsKey argument is weird, but it is indeed not used for XML either because of the custom normalization done in the tree.

[mdavis1982]

@stof I'm still not sure why this works, to be honest - as I mentioned before, you can set the paths text to anything you like to and it fixes the issue parsing the configuration.

[stof]

@mdavis1982 this is because useAttributeAsKey does 2 things:

• it tells the processor that the array in the node is a hash, not a list, and so it should preserve keys when merging several arrays together
• it tells the processor which attribute contains the key for formats which are not able to have the key as a real key (XML being the format needing this).

Because of the custom normalization done on this node, the second behavior is not necessary. But the first behavior (triggered for any non-null values) is still needed

[mdavis1982]

@stof Thanks for the clarification - I think I understand now. Is this PR good to go then?

[stof]

@mdavis1982 from my PoV, yes. but the rules are that 2 deciders need to vote on it, and currently, there is only me in #9719 (comment)

[fabpot]

👍

[makasim]

is this going to be merged to master? which means that the fix will be available only for symfony 2.6?

I am using prepend extension feature to make twig aware of some templates that comes with a library. And it would be good to have it fixed in 2.3 too.

[stof]

@makasim nope. As it is a bugfix, it will go in 2.3

[stof]

Thanks for fixing this bug @cordoval.

[mdavis1982]

@stof I actually fixed this bug 😉

[stof]

hmm, indeed. Something to improve in GH when there is several contributors in the PR. curently, it only suggest the names of the PR author for the thank message, and I missed it when merging to change it manually.
so thanks @mdavis1982 as well