8000 [DebugBundle][VarDumper] Fix dump labels compatibility by fancyweb · Pull Request #50347 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[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

Merged

Conversation

fancyweb
Copy link
Contributor
Q A
Branch? 6.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

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:
Screenshot from 2023-05-16 18-31-01
Could you take care of this part plz? 🙏

@carsonbot carsonbot added this to the 6.3 milestone May 16, 2023
@carsonbot carsonbot changed the title [VarDumper][DebugBundle] Fix dump labels compatibility [DebugBundle][VarDumper] Fix dump labels compatibility May 16, 2023
@nicolas-grekas
Copy link
Member

can you please confirm that the label is a link to the source?

@fancyweb
Copy link
Contributor Author

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.

@fancyweb fancyweb force-pushed the var-dumper/fix-labels-debug-bundle branch from 05d5dca to 7c55551 Compare May 17, 2023 08:45
@@ -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)) {
Copy link
Contributor Author

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

@nicolas-grekas nicolas-grekas force-pushed the var-dumper/fix-labels-debug-bundle branch from 7c55551 to 8a8b5cd Compare May 19, 2023 11:56
@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

@nicolas-grekas nicolas-grekas merged commit 761d915 into symfony:6.3 May 19, 2023
@fancyweb fancyweb deleted the var-dumper/fix-labels-debug-bundle branch May 19, 2023 12:13
@fabpot fabpot mentioned this pull request May 22, 2023
@bobthecow
Copy link
Contributor

@fancyweb @nicolas-grekas This change introduces a backwards compatibility break. See PsySH dependency test failures at dev stability starting with this change.

@bobthecow
Copy link
Contributor

Specifically, PsySH extends VarDumper and uses its styles map. It looks like this change doesn't add label to the map, but expects callers to use default instead, e.g.:

$this->styles['label' === $style ? 'default' : $style]

nicolas-grekas added a commit that referenced this pull request May 23, 2023
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);
```

![image](https://github.com/symfony/symfony/assets/2445045/5615db1d-d48d-4720-85ce-239d3921f624)

Commits
-------

07d318a [VarDumper] Fix `dd()` showing line with `null`
nicolas-grekas added a commit that referenced this pull request May 25, 2023
…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:

![image](https://github.com/symfony/symfony/assets/243674/956f7c7d-5569-4ea2-97f3-01d000f34532)

![image](https://github.com/symfony/symfony/assets/243674/6a497d1e-5291-45a7-8af3-1cf11cafded6)

Commits
-------

e824eb0 [VarDumper][HttpKernel] Fix dumping with labels
symfony-splitter pushed a commit to symfony/var-dumper that referenced this pull request May 25, 2023
…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:

![image](https://github.com/symfony/symfony/assets/243674/956f7c7d-5569-4ea2-97f3-01d000f34532)

![image](https://github.com/symfony/symfony/assets/243674/6a497d1e-5291-45a7-8af3-1cf11cafded6)

Commits
-------

e824eb051e [VarDumper][HttpKernel] Fix dumping with labels
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0