Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Config] Add ExprBuilder::ifEmpty()#19764
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Config] Add ExprBuilder::ifEmpty()#19764
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| * @return ExprBuilder | ||
| */ | ||
| publicfunctionifEmpty() | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What if$v equals0 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
having the same behavior thanempty($v) is fine with me. If we do something else, we should change the name away fromifEmpty to avoid WTF moments IMO
ogizanagiAug 30, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As@stof said, it isn't an issue to me, because anyone using it must understand it behaves like its phpempty function counterpart, and know the limitations.
Otherwise, we have to rename it toisEmptyArray, or whatever, but right now I think the expression builder should stick with simple expressions like this.
If you really expect'',false,null,'0' or0 to be valid for your node, you shouldn't useifEmpty.
fabpot commentedAug 31, 2016
Thank you@ogizanagi. |
This PR was merged into the 3.2-dev branch.Discussion----------[Config] Add ExprBuilder::ifEmpty()| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |symfony/symfony-docs#6922Useful for instance when you don't expect to have a key set in the resolved config if its content is empty:```php$builder = new TreeBuilder();$tree = $builder ->root('matcher') ->children() ->arrayNode('references_to_exclude') ->validate() ->ifEmpty() ->thenUnset() ->end() ->prototype('scalar')->end() ->end() ->end() ->end() ->buildTree();$tree->finalize(['references_to_exclude' => ['foo', 'bar']]);>>> ['references_to_exclude' => ['foo', 'bar']]$tree->finalize(['references_to_exclude' => []]);>>> []```Commits-------4e46f64 [Config] Add ExprBuilder::ifEmpty()
This PR was merged into the master branch.Discussion----------[Config] Add ExprBuilder::ifEmpty()Aftersymfony/symfony#19764Commits-------f0fe739 [WCM][Config] Add ExprBuilder::ifEmpty()
Uh oh!
There was an error while loading.Please reload this page.
Useful for instance when you don't expect to have a key set in the resolved config if its content is empty: