WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#11090 closed defect (bug) (fixed)

Standardize checking for WP_DEBUG

Reported by: nacin Owned by: westi
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.9
Component: Warnings/Notices Keywords:
Focuses: Cc:

Description

The Codex recommends define('WP_DEBUG', true) or define('WP_DEBUG', false). However, WP_DEBUG is inconsistently checked whether should WP_DEBUG be identical or equal to those values.

Thus, define('WP_DEBUG', 1) will not turn all feedback on, and define('WP_DEBUG', 0) will not turn all feedback off.

Two patches attached, depending on whether it should be standardized to true == WP_DEBUG or true === WP_DEBUG.

Attachments (4)

wp_debug_identical.patch (4.9 KB) - added by nacin 4 years ago.
true === WP_DEBUG
wp_debug_equal.patch (3.6 KB) - added by nacin 4 years ago.
true == WP_DEBUG
11090_2.patch (3.4 KB) - added by nacin 4 years ago.
Better patch for true == WP_DEBUG route
11090_3.patch (4.8 KB) - added by nacin 4 years ago.
wp-settings.php will now set {{WP_DEBUG == false}}, making the {{defined(WP_DEBUG)}} check no longer needed in core or in plugins.

Download all attachments as: .zip

Change History (13)

nacin4 years ago

true === WP_DEBUG

nacin4 years ago

true == WP_DEBUG

comment:1 scribu4 years ago

  • Milestone changed from Unassigned to 2.9

comment:2 hakre4 years ago

the expression of true == WP_DEBUG is completely equal to the expression of WP_DEBUG. To keep things simple, I would prefer the later one. Same applies to false == WP_DEBUG which is equal to !WP_DEBUG.

comment:3 follow-up: scribu4 years ago

Consider the case where WP_DEBUG is not defined at all:

var_dump(true == WP_DEBUG); // Result: bool(true)

I vote for '==='.

comment:4 strider724 years ago

Per discussion on wp-hackers: The docs say that the values should be boolean, thus === is the way to go.

http://lists.automattic.com/pipermail/wp-hackers/2009-November/028395.html

comment:5 azaozz4 years ago

Don't see a good reason to explicitly forbid other values that evaluate to true. Also true == WP_DEBUG doesn't seem to have a purpose. Looking at other boolean constants, most are checked with:

if ( defined('CONSTANT') && CONSTANT )
// true
...
if ( !defined('CONSTANT') || !CONSTANT )
// false

which seems clear and concise. So if we are standardizing this perhaps that is the way to go.

nacin4 years ago

Better patch for true == WP_DEBUG route

comment:6 dd324 years ago

Is there any good reason for code like this:

 if ( ! defined('WP_DEBUG') || ( defined('WP_DEBUG') && !WP_DEBUG ) ) {

Which is exactly the same as:

 if ( ! defined('WP_DEBUG') || !WP_DEBUG ) {

I agree that adding == and === into it complicates things.

We could even add to wp-settings:

if ( ! defined('WP_DEBUG') ) define('WP_DEBUG', false);

if that makes it easier?

comment:7 westi4 years ago

  • Status changed from new to accepted

I'll look into this and fix it later today :-)

nacin4 years ago

wp-settings.php will now set {{WP_DEBUG == false}}, making the {{defined(WP_DEBUG)}} check no longer needed in core or in plugins.

comment:8 in reply to: ↑ 3 nacin4 years ago

(Oops, poor Wiki formatting on my part.)

Thanks, westi.

New patch following dd32's logic, and to prevent an undefined constant from evaluating to true as scribu pointed out.

The only concern I might see with this route is that a plugin might be only checking for define( WP_DEBUG ) and not actually checking if ( WP_DEBUG ) as well.

This reminds me of #10602 as well, regarding WP_CACHE.

comment:9 automattor4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [12207]) Ensure WP_DEBUG is always defined and simplify the checks on it. Fixes #11090 props nacin.

Note: See TracTickets for help on using tickets.