[buddypress-trac] [BuddyPress] #2023: BP_DISABLE_ADMIN_BAR should be checked for true and not just existence

[buddypress-trac at lists.automattic.com]

#2023: BP_DISABLE_ADMIN_BAR should be checked for true and not just existence
------------------------+---------------------------------------------------
Reporter:  TobiasBg     |       Owner:       
    Type:  enhancement  |      Status:  new  
Priority:  minor        |   Milestone:  1.2.2
Keywords:  has-patch    |  
------------------------+---------------------------------------------------
Changes (by TobiasBg):

  * keywords:  has-patch reporter-feedback => has-patch


Comment:

 I don't really agree.

 I know that it is not pre-defined, but user defined - so it may exist, but
 does not always. That's what the defined() check is for. However, I
 believe that the actual value should also be checked:
 For one: It is meant to be a boolean constant ("disable": yes/no).
 Second: It's consistency: I have not seen a line in WP core, where only
 the existence but not the value of a constant is checked. This is just
 common code convention, in my eyes.

 My use case is this:
 I want to develop a more fine grained control of who can see the Admin Bar
 and who can not.
 For that I would like to use something like this (just a short code
 fragment):
 {{{
 if ( is_user_logged_in() && current_user_can( 'manage_options' ) {
   define( 'BP_DISABLE_ADMIN_BAR', true );
 } else {
   define( 'BP_DISABLE_ADMIN_BAR', false );
 }
 }}}
 However, this does not work with just the checks for defined(), which I
 learned the hard way, as it took me forever to find the actual cause.

 And if the recommended way actually is to set it to true, then the checks
 should also check for that.
 (Keep in mind, that adding the checks is also backwards compatible, in
 case someone actually has set the constant to 'ninjas', as that will
 evaluate to true, also.)

-- 
Ticket URL: <http://trac.buddypress.org/ticket/2023#comment:2>
BuddyPress <http://buddypress.org/>
BuddyPress