[VarDumper] Fix CliDumper coloration on light arrays #36230
Merged
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
| @@ -268,7 +268,8 @@ public function enterHash(Cursor $cursor, $type, $class, $hasChild) | |||
| } elseif (Cursor::HASH_RESOURCE === $type) { | |||
| $prefix = $this->style('note', $class.' resource').($hasChild ? ' {' : ' '); | |||
| } 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.
I'm not sure I get the logic - why styling the empty string?
There was a problem hiding this comment.
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.
the issue in #37674 is precisely complaining about that.
|
Can you provide a simple script to reproduce the issue? I failed at it. |
|
Of course 🙂 use Symfony\Component\VarDumper\Cloner\VarCloner;
use Symfony\Component\VarDumper\Dumper\AbstractDumper;
use Symfony\Component\VarDumper\Dumper\CliDumper;
require __DIR__.'/vendor/autoload.php';
$cliDumper = new CliDumper(null, null, AbstractDumper::DUMP_LIGHT_ARRAY);
$cliDumper->dump((new VarCloner())->cloneVar([]));
$var = ['foo' => 'bar'];
$cliDumper->dump((new VarCloner())->cloneVar(['foo' => 'bar']));
|
|
Can you check the failures on Windows? Maybe you'd like to skip the test case there btw. |
4645e0a to
4881712
Compare
|
I didn't notice that the tests failed on windows; It's fixed, the test is now skipped on this environment. |
d52165f to
48116af
Compare
48116af to
8ad26a8
Compare
nicolas-grekas
approved these changes
Jun 15, 2020
When using AbstractDumper::DUMP_LIGHT_ARRAY
8ad26a8 to
7af3469
Compare
|
Thank you @l-vo. |
This was referenced Jul 24, 2020
Merged
Merged
Merged
Merged
fabpot
added a commit
that referenced
this pull request
Jul 31, 2020
… (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 to fix #37674 (revert previous fix and use a solution that looks better). Commits ------- cef16f5 [VarDumper] Improve previous fix on light array coloration
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.
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.