[config] added remove option to ignoreExtraKeys #14238
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
can you please link to the respective ticket? |
|
ok i know one that is related, i can do a PR perhaps on that inspired on this one |
|
Thanks. |
| { | ||
| $this->ignoreExtraKeys = (bool) $boolean; | ||
| $this->removeExtraKeys = $this->ignoreExtraKeys ? (bool) $remove : false; |
There was a problem hiding this comment.
can be simplyfied to $this->ignoreExtraKeys && $remove
|
@xabbuh Changes made |
|
The failed travis check doesn't seem to be related to my modifications. Is there anything I need to do here? |
|
I relaunched tests, they are green now. |
|
👍 on 2.8 |
|
👍 |
|
Thank you @twifty. |
fabpot
added a commit
that referenced
this pull request
Sep 30, 2015
…haviors (ogizanagi) This PR was merged into the 2.8 branch. Discussion ---------- [Config] Fix ArrayNode extra keys "ignore" and "remove" behaviors | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Due to #14238 , no more exception is thrown when submitting extra keys to an `ArrayNode`. For instance: ```php $builder = new TreeBuilder(); $nodeDefinition = $builder->root('root') ->children() ->scalarNode('foo') ->end() ->end(); $node = $nodeDefinition->getNode(true); $node->normalize(array( 'foo' => 'ok', 'bar' => 'ko', )); ``` will not throw a > Symfony\Component\Config\Definition\Exception\InvalidConfigurationException: Unrecognized option "bar" under "root"` anymore, as it does in 2.7. I think the expected behavior is: `Submitted data: ['bar' => 'ko']` Ignore | Remove | Expected | OK | Comment ---------| ------------ | ------------- | ------ | ---------- true | true | `[ ]` | ✔︎ | Previous behavior when ignoring. true | false | `['bar' => 'ko']` | ✔︎ | This is the result targeted by #14238. false | true | exception | ✘ | Removing makes no sense when not ignoring extra keys. <br/>The exception should still be thrown. false | false | exception | ✘ | Previous behavior (2.7). <br/>Should not have changed Commits ------- d961f7f [Config] Fix ArrayNode extra keys "ignore" and "remove" behaviors
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have added a remove option to the
ignoreExtraKeys()method ofConfig/Definition/Builder/ArrayNodeDefinition. The current implementation removes all such keys from the resulting array. This can at times be an inconvenience as we have to write extra methods to remove them before the validation then add them back again when done. Passing a boolean to the method now leaves them in place.