Skip to content

var_export - provide 'null' instead of 'NULL' #2422

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

keradus
Copy link

@keradus keradus commented Mar 15, 2017

var_export returns true and false but NULL.
Let us make it consistent and return null as well.
This approach is also encouraged by PSR-2.

I did update obvious places, if you like the idea I will make build passing (by adjusting non-obvious (at least for me) places as well).

@Majkl578
Copy link
Contributor

You silently changed output of var_dump, but only mention var_export.
This is pretty serious BC break, it will break tons of tests for no good reason. Expect tons of failed tests on CI, you can see already failed ones here: https://travis-ci.org/php/php-src/jobs/211407088

@keradus
Copy link
Author

keradus commented Mar 15, 2017

I said it will break CI in current state, right? :)

Is there a BC promise on those methods other than output: string?

Also, that's reason why I did not target 5.6.x line

@nikic
Copy link
Member

nikic commented Mar 15, 2017

As @Majkl578 already pointed out, this change is going to break the world. The Travis build for php-src already comes in with ~2k failures (!!!), not to mention that pretty much all 3rd party extensions use phpt tests and as such rely on specific var_dump output. They would be in an even worse situation than php-src, because they would have to accept both output formats depending on the PHP version used.

This is too much breakage for an otherwise rather cosmetic change. As such, I'm closing this PR.

@nikic nikic closed this Mar 15, 2017
@keradus keradus deleted the null branch March 15, 2017 16:54
@keradus
Copy link
Author

keradus commented Mar 15, 2017

Thanks for reasoning Nikita.
I still do hope that one day it would be standardized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants