Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[VarDumper] Fix bug introduced by adding null coalescing incorrectly #48038

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

Closed
wants to merge 1 commit into from

Conversation

PhilETaylor
Copy link
Contributor

@PhilETaylor PhilETaylor commented Oct 28, 2022

Q A
Branch? 5.4
Bug fix? no
New feature? no
Deprecations? no
License MIT

This PR fixes a bug introduced by (@javiereguiluz) in #42165 (EDIT: No, I think it predates that now... #19623 @nicolas-grekas who replaced a bool with this code, which #42165 just refactored) ...

The code that was replaced was

if ($this->headerIsDumped !== (null !== $this->outputStream ? $this->outputStream : $this->lineDumper)) {

That was replaced in #42165 with code

if ($this->headerIsDumped !== ($this->outputStream ?? $this->lineDumper)) {

The correct code is proposed in this PR and is

if ($this->headerIsDumped !== (null !== ($this->outputStream ?? $this->lineDumper))) {

What is the bug/Why ?

So.... To replicate create a demo app with Symfony 5.4

composer create-project symfony/symfony-demo mySymfony54 1.8.0
cd mySymfony54
symfony serve
# Visit http://127.0.0.1:8000/en/blog/ to confirm its working
# Edit BlogController.php and throw an \Exception('test') in the index function
# Visit http://127.0.0.1:8000/en/blog/ to confirm you see the exception 
# Click the toolbar at the bottom to get to the Symfony Profiler
# Click LOGS in the left menu 
# Check console.log 

see console.log:

Screenshot 2022-10-29 at 00 14 52

Click to open the error message and see overlapping text and no pretty-ness.

Screenshot 2022-10-29 at 00 15 10

Once this PR is applied you then can repeat and you will see no console.log error message and you will see a pretty output like:

Screenshot 2022-10-29 at 00 16 24

@carsonbot carsonbot added this to the 5.4 milestone Oct 28, 2022
@PhilETaylor PhilETaylor changed the title [VarDumper] Fix bug introduced by adding null collapse incorrectly [VarDumper] Fix bug introduced by adding null coalescing incorrectly Oct 28, 2022
@PhilETaylor
Copy link
Contributor Author

PhilETaylor commented Oct 28, 2022

Maybe Im wrong on this one - maybe @nicolas-grekas could comment please? the fact is there is a reproducible bug here that needs fixing, this PR fixes it, but it might be the wrong solution... (Hint: It was the wrong solution, and reverted good work Nicolas did years ago - sorry - I'll think again)

@PhilETaylor
Copy link
Contributor Author

Closing and opening as an Issue as I cannot fix this #48039

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.

2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.