-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DebugBundle][VarDumper] Fix dump labels compatibility #50347
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DebugBundle][VarDumper] Fix dump labels compatibility #50347
Conversation
can you please confirm that the label is a link to the source? |
DebugBundle behavior is to have links on the file (FooAction) not on the label. But wait I've found one more problem and I'm crafting a better patch. |
05d5dca
to
7c55551
Compare
@@ -25,7 +25,7 @@ function dump(mixed ...$vars): mixed | |||
return null; | |||
} | |||
|
|||
if (isset($vars[0]) && 1 === count($vars)) { | |||
if (array_key_exists(0, $vars) && 1 === count($vars)) { |
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.
Fix dump(null) showing 1^ null
7c55551
to
8a8b5cd
Compare
Thank you @fancyweb. |
@fancyweb @nicolas-grekas This change introduces a backwards compatibility break. See PsySH dependency test failures at dev stability starting with this change. |
Specifically, PsySH extends VarDumper and uses its $this->styles['label' === $style ? 'default' : $style] |
This PR was merged into the 6.3 branch. Discussion ---------- [VarDumper] Fix `dd()` showing line with `null` | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Similar to #50347 (comment) but for the `dd()` function: ```php dd(null); ```  Commits ------- 07d318a [VarDumper] Fix `dd()` showing line with `null`
…rekas) This PR was merged into the 6.3 branch. Discussion ---------- [HttpKernel][VarDumper] Fix dumping with labels | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Should fix #50347 (comment) /cc `@bobthecow` Also improves the display in the WDT:   Commits ------- e824eb0 [VarDumper][HttpKernel] Fix dumping with labels
…rekas) This PR was merged into the 6.3 branch. Discussion ---------- [HttpKernel][VarDumper] Fix dumping with labels | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Should fix symfony/symfony#50347 (comment) /cc `@bobthecow` Also improves the display in the WDT:   Commits ------- e824eb051e [VarDumper][HttpKernel] Fix dumping with labels
This PR fixes the handling of dumps labels when the DebugBundle is enabled and stays BC with older versions.
@javiereguiluz Unfortunately, the CSS of the web profiler toolbar is not perfect for labels and invisible characters:

Could you take care of this part plz? 🙏