This repository has been archived by the owner. It is now read-only.

Discourage use of the silence operator. #1390

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@realityking
Contributor

realityking commented Jul 20, 2012

No description provided.

@pasamio

This comment has been minimized.

Show comment
Hide comment
@pasamio

pasamio Jul 28, 2012

Contributor

Not sure I entirely agree, PHP's obsession with returning PHP warnings/errors with the actual error message being in the warning (only partially following the C convention here). The only way to work around this and get useful information is to use $php_errormsg however the PHP example includes the use of the silence operate in it's canonical example:
http://us2.php.net/manual/en/reserved.variables.phperrormsg.php

Contributor

pasamio commented Jul 28, 2012

Not sure I entirely agree, PHP's obsession with returning PHP warnings/errors with the actual error message being in the warning (only partially following the C convention here). The only way to work around this and get useful information is to use $php_errormsg however the PHP example includes the use of the silence operate in it's canonical example:
http://us2.php.net/manual/en/reserved.variables.phperrormsg.php

@realityking

This comment has been minimized.

Show comment
Hide comment
@realityking

realityking Jul 28, 2012

Contributor

I agree we'll never be able to completely get rid of them (at least not with PHP's current error capabilities). But there are a number of places where we wouldn't need them if we'd use a isset() or other checks like that. That's why this would only add a warning, not an error.

Contributor

realityking commented Jul 28, 2012

I agree we'll never be able to completely get rid of them (at least not with PHP's current error capabilities). But there are a number of places where we wouldn't need them if we'd use a isset() or other checks like that. That's why this would only add a warning, not an error.

@@ -15,9 +15,9 @@
if (extension_loaded('mbstring'))
{
// Make sure to suppress the output in case ini_set is disabled
- @ini_set('mbstring.internal_encoding', 'UTF-8');

This comment has been minimized.

@LouisLandry

LouisLandry Oct 9, 2012

Contributor

These I'm not sure I like. The others I'm fine with.

@LouisLandry

LouisLandry Oct 9, 2012

Contributor

These I'm not sure I like. The others I'm fine with.

@LouisLandry

This comment has been minimized.

Show comment
Hide comment
@LouisLandry

LouisLandry Nov 9, 2012

Contributor

This is no longer mergeable. Additionally I have reservations on the silence operator being used on the mbstring settings. If errors, notices, or warnings are suppressed for that we don't have any sort of backup to know what went wrong in using the mbstring functions.

I'm going to close this one for now until you can get it rebased @realityking. Do that and we can continue the discussion. Remove those supression operators on the ini_set() calls for mbstring and I'll just merge it. Reopen when you are ready, thanks!

Contributor

LouisLandry commented Nov 9, 2012

This is no longer mergeable. Additionally I have reservations on the silence operator being used on the mbstring settings. If errors, notices, or warnings are suppressed for that we don't have any sort of backup to know what went wrong in using the mbstring functions.

I'm going to close this one for now until you can get it rebased @realityking. Do that and we can continue the discussion. Remove those supression operators on the ini_set() calls for mbstring and I'll just merge it. Reopen when you are ready, thanks!

@LouisLandry LouisLandry closed this Nov 9, 2012

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.