Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[VarDumper] Fix CliDumper coloration on light arrays#36230
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
[VarDumper] Fix CliDumper coloration on light arrays#36230
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| }else { | ||
| $prefix =$class && !(self::DUMP_LIGHT_ARRAY &$this->flags) ?$this->style('note','array:'.$class).' [' :'['; | ||
| $unstyledPrefix =$class && !(self::DUMP_LIGHT_ARRAY &$this->flags) ?'array:'.$class :''; | ||
| $prefix =$this->style('note',$unstyledPrefix).($unstyledPrefix ?' [' :'['); |
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.
I'm not sure I get the logic - why styling the empty string?
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.
Because if I don't trigger a coloration (even on an empty string), coloration is not enabled and the open bracket stay uncolored.
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 issue in#37674 is precisely complaining about that.
nicolas-grekas commentedJun 14, 2020
Can you provide a simple script to reproduce the issue? I failed at it. |
l-vo commentedJun 14, 2020
Of course 🙂 useSymfony\Component\VarDumper\Cloner\VarCloner;useSymfony\Component\VarDumper\Dumper\AbstractDumper;useSymfony\Component\VarDumper\Dumper\CliDumper;require__DIR__.'/vendor/autoload.php';$cliDumper =newCliDumper(null,null, AbstractDumper::DUMP_LIGHT_ARRAY);$cliDumper->dump((newVarCloner())->cloneVar([]));$var = ['foo' =>'bar'];$cliDumper->dump((newVarCloner())->cloneVar(['foo' =>'bar']));
|
nicolas-grekas 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.
Great thanks.
nicolas-grekas commentedJun 14, 2020
Can you check the failures on Windows? Maybe you'd like to skip the test case there btw. |
4645e0a to4881712Comparel-vo commentedJun 15, 2020
I didn't notice that the tests failed on windows; It's fixed, the test is now skipped on this environment. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
d52165f to48116afCompare48116af to8ad26a8CompareWhen using AbstractDumper::DUMP_LIGHT_ARRAY
8ad26a8 to7af3469Comparenicolas-grekas commentedJun 18, 2020
Thank you@l-vo. |
… (l-vo)This PR was merged into the 3.4 branch.Discussion----------[VarDumper] Improve previous fix on light array coloration| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets |Fix#37674| License | MIT| Doc PR |Improve#36230 tofix#37674 (revert previous fix and use a solution that looks better).Commits-------cef16f5 [VarDumper] Improve previous fix on light array coloration
When
AbstractDumper::DUMP_LIGHT_ARRAYis used with theCliDumper, the first line (opening bracket) is not colored. When an empty array is dumped (with or withoutAbstractDumper::DUMP_LIGHT_ARRAY) the array is not colored too. This PR aims to fix that.