Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Enableable ArrayNodeDefinition is disabled for empty configuration#25789
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| ->then(function ($v) { | ||
| $v['enabled'] =isset($v['enabled']) ?$v['enabled'] :true; | ||
| if (!isset($v['enabled'])) { | ||
| $v['enabled'] =empty($v) ?false :true; |
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.
Can also be written as$v['enabled'] = !empty($v);
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.
damn, you're right :D
Simperfit left a comment
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.
The tests failure seems unrelated to the current PR.
fabpot commentedJan 17, 2018
Thank you @kejwmen. |
…guration (kejwmen)This PR was squashed before being merged into the 2.7 branch (closes#25789).Discussion---------- Enableable ArrayNodeDefinition is disabled for empty configuration| Q | A| ------------- | ---| Branch? | 2.7+| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#25760| License | MITFixes#25760.Currently, documented behavior is not true:https://github.com/symfony/symfony/blob/70c8c2d47bd71861c8205985a17e68cedf828e1f/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php#L207-L208Commits-------a6a330d Enableable ArrayNodeDefinition is disabled for empty configuration
The failing tests relied on the assets integration being enabled. Sincewe never did enable this explicitly, the assets related definitions areremoved now thatsymfony#25789 was merged whichfixedsymfony#25760.
This PR was merged into the 3.3 branch.Discussion----------[FrameworkBundle] fix DI extension tests| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |The failing tests relied on the assets integration being enabled. Sincewe never did enable this explicitly, the assets related definitions areremoved now that#25789 was merged whichfixed#25760.Commits-------1cb8f69 [FrameworkBundle] fix DI extension tests
fabpot commentedJan 21, 2018
| ->then(function ($v) { | ||
| $v['enabled'] =isset($v['enabled']) ?$v['enabled'] :true; | ||
| if (!isset($v['enabled'])) { | ||
| $v['enabled'] = !empty($v); |
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.
Should probably beis_array($v) && !empty($v) to cover thefoo: ~ case, which should work as enable.
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 tests should we add to make sure it won't happen in the future? Component tests covernull case already.~ is just anull, right?
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.
forget my comment. This callback is only triggered for arrays ;)
mateuszsipJan 21, 2018 • 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.
Looks like~ in yaml is represented by empty array (?), simple reproducer is availablehere
* 2.7: [HttpFoundation] fixed return type of method HeaderBag::get [HttpFoundation] Added "resource" type on Request::create docblock Revert "bug#25789 Enableable ArrayNodeDefinition is disabled for empty configuration (kejwmen)" Revert "bug#25851 [Validator] Conflict with egulias/email-validator 2.0 (emodric)" [Validator] add missing parent isset and add test
* 2.8: [HttpFoundation] fixed return type of method HeaderBag::get [HttpFoundation] Added "resource" type on Request::create docblock Revert "bug#25789 Enableable ArrayNodeDefinition is disabled for empty configuration (kejwmen)" Formatting fix in upgrade 3.0 document Revert "bug#25851 [Validator] Conflict with egulias/email-validator 2.0 (emodric)" [Validator] add missing parent isset and add test
* 3.3: Have weak_vendors ignore deprecations from outside [HttpFoundation] fixed return type of method HeaderBag::get [HttpFoundation] Added "resource" type on Request::create docblock [Process] Skip environment variables with false value in Process Revert "bug#25789 Enableable ArrayNodeDefinition is disabled for empty configuration (kejwmen)" Formatting fix in upgrade 3.0 document don't split lines on carriage returns when dumping Revert "bug#25851 [Validator] Conflict with egulias/email-validator 2.0 (emodric)" [DI] compilation perf tweak [Validator] Conflict with egulias/email-validator 2.0 [Validator] add missing parent isset and add test
* 3.4: Have weak_vendors ignore deprecations from outside [HttpFoundation] fixed return type of method HeaderBag::get [HttpFoundation] Added "resource" type on Request::create docblock [Process] Skip environment variables with false value in Process Revert "bug#25789 Enableable ArrayNodeDefinition is disabled for empty configuration (kejwmen)" Formatting fix in upgrade 3.0 document don't split lines on carriage returns when dumping Revert "bug#25851 [Validator] Conflict with egulias/email-validator 2.0 (emodric)" [DI] compilation perf tweak [Validator] Conflict with egulias/email-validator 2.0 [Validator] add missing parent isset and add test
* 4.0: Have weak_vendors ignore deprecations from outside [HttpFoundation] fixed return type of method HeaderBag::get [HttpFoundation] Added "resource" type on Request::create docblock [Console] Fix using finally where the catch can also fail [Process] Skip environment variables with false value in Process Revert "bug#25789 Enableable ArrayNodeDefinition is disabled for empty configuration (kejwmen)" Formatting fix in upgrade 3.0 document don't split lines on carriage returns when dumping trim spaces from unquoted scalar values Revert "bug#25851 [Validator] Conflict with egulias/email-validator 2.0 (emodric)" [DI] compilation perf tweak [Validator] Conflict with egulias/email-validator 2.0 [Validator] add missing parent isset and add test
Fixes#25760.
Currently, documented behavior is not true:
symfony/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php
Lines 207 to 208 in70c8c2d