[VarDumper] Add filters to casters #14058
Conversation
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | - |
| License | MIT |
| Doc PR | - |
14a79fa to
31daf2f
Compare
| break; | ||
| case isset($k[1]) && "\0" === $k[0]: | ||
| switch ($k[1]) { | ||
| case '~': $type = self::EXCLUDE_VIRTUAL; break; |
There was a problem hiding this comment.
This violates PSR-2.
There was a problem hiding this comment.
You have multiple statements on one line.
There was a problem hiding this comment.
It's way more readable this way to me...
There was a problem hiding this comment.
I would make this multiline to make it more readable (seeing easily that we have some logic in the case, not only a condition falling through to the next case).
currently, the code here is very similar to the code on lines 52 and 53 at first look, while the structure of the code is actually totally different
31daf2f to
68d3ac5
Compare
68d3ac5 to
015928e
Compare
| * | ||
| * @return array The filtered array | ||
| */ | ||
| public static function filter(array $a, $filter, array $listedProperties = array()) |
There was a problem hiding this comment.
is this method actually used anywhere ?
There was a problem hiding this comment.
Not in Symfony but I hope to use it in PsySH bobthecow/psysh#184
7ac8cc4 to
73291bd
Compare
| * | ||
| * @param array $a The array containing the properties to filter. | ||
| * @param int $filter A bit field of Caster::EXCLUDE_* constants specifying which properties to filter out. | ||
| * @param array $listedProperties List of properties to exclude when Caster::EXCLUDE_VERBOSE is set, and to preserve when Caster::EXCLUDE_NOT_IMPORTANT is set. |
There was a problem hiding this comment.
this means we cannot set EXCLUDE_VERBOSE and EXCLUDE_NOT_IMPORTANT at the same time without doing weird stuff, right ? If so, it should trigger an InvalidArgumentException in this case.
and the type should be string[]
There was a problem hiding this comment.
I added a test case with both enabled. The result is excluding everything. I agree that it's not usefull, but since this is a consistent behavior, I'd prefer not adding an exception.
73291bd to
d101989
Compare
d101989 to
84a80d1
Compare
|
Any objection to merge this @symfony/deciders? Tests are OK for what is concerned by this PR. |
|
Thank you @nicolas-grekas. |
This PR was merged into the 2.7 branch. Discussion ---------- [VarDumper] Add filters to casters | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- 84a80d1 [VarDumper] Add filters to casters
…as-grekas) This PR was merged into the 2.7 branch. Discussion ---------- [VarDumper] Add and use Caster::PREFIX_* consts | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Will need a rebase on top of #14058 once it is merged Commits ------- 86cf8de [VarDumper] Make use of Caster::PREFIX_* consts