The Wayback Machine - https://web.archive.org/web/20220819181809/https://github.com/symfony/symfony/pull/47086
Skip to content
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

Workaround disabled "var_dump" #47086

Merged
merged 1 commit into from Jul 27, 2022
Merged

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Jul 27, 2022

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #46881
License MIT
Doc PR -

@nicolas-grekas nicolas-grekas merged commit 9a4d7df into symfony:4.4 Jul 27, 2022
9 of 11 checks passed
@nicolas-grekas nicolas-grekas deleted the vd branch Jul 27, 2022
$handler = set_error_handler('var_dump');
$handler = set_error_handler('is_int');
$handler = \is_array($handler) ? $handler[0] : null;
restore_error_handler();
Copy link
Contributor

@guilliamxavier guilliamxavier Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ever is_int was called, it would throw an ArgumentCountError, so a few questions:

  • Is that actually a good thing?
  • Are there reasons for not passing null here? or e.g. function () {}?
  • Would it be safer to move up the restore_{error|exception}_handler(); immediately after the set_{error|exception}_handler call, with no operation in-between?
  • Should add get_error_handler() and get_exception_handler() php/php-src#969 be revived?

Copy link
Member Author

@nicolas-grekas nicolas-grekas Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should php/php-src#969 be revived?

Maybe. For your other questions, they're mostly theoretical at the moment so I wouldn't care much :)

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.

None yet

4 participants