[wp-trac] [WordPress Trac] #23259: Allow filtering the $function parameter of add_filter(), has_filter() and remove_filter()

WordPress Trac noreply at wordpress.org
Tue Jan 22 11:20:27 UTC 2013


#23259: Allow filtering the $function parameter of add_filter(), has_filter() and
remove_filter()
------------------------------------+------------------------------
 Reporter:  MikeSchinkel            |       Owner:
     Type:  feature request         |      Status:  new
 Priority:  normal                  |   Milestone:  Awaiting Review
Component:  Plugins                 |     Version:  3.5
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |
------------------------------------+------------------------------

Comment (by rmccue):

 Giant -1 on this. This is adding a layer of abstraction where it's not
 needed.

 ----

 There's basically four types of callbacks I can hook in:
 1. Normal functions: `add_action('init', 'myfunction')`. These are dead
 easy to unhook: `remove_action('init', 'myfunction')`
 2. Class methods: `add_action('init', ['MyClass', 'method'])` or
 `add_action('init', 'MyClass::method')`. Again, dead easy to unhook:
 `remove_action('init', array('MyClass', 'method')` and
 `remove_action('init', 'MyClass::method')`
 3. Object methods: `add_action('init', [$this, 'method'])`. This is where
 it gets harder and more complicated. I'll break it down into the
 techniques commonly used with this:
     1. `$myplugin = new MyPlugin()`: This is pretty easy to unhook, since
 we're almost always in the global scope when loading plugins (by design).
 `global $myplugin; remove_action('init', [$myplugin, 'method'])`
     2. `MyPlugin::instance()`: Again, easy since these are global state
 (although they're not in the global scope. `remove_action('init',
 [MyPlugin::instance(), 'method'])`
     3. `new MyPlugin()`: This is the main issue, since although the object
 is instantiated, it's never actually assigned to anything. This is one
 case where it would be handy to be able to filter the callback, but that's
 simply a band-aid fix over the real problem, which is that this is sloppy
 coding that doesn't consider other plugins.
 4. Anonymous functions/Closures: `add_action('init', function () { /* ...
 */ })`. Again, this is a problem because the function is never assigned to
 a variable. Also again, this would be a band-aid solution, since this is
 just sloppy coding.

 (Please note: when I said "sloppy coding", I'm talking about in the
 context of playing nice with other plugins, not saying these things are
 bad in general.)

 So basically, we have two problem callback types. Both of these are a
 direct result of the plugin author not caring about other plugins enough
 to give us anything to work with. I personally don't think we need to hold
 those plugin authors' hands and/or encourage them to do this. Name and
 shame those who use this, and if that doesn't work, fork their plugin and
 fix it.

 Another issue with this is that callbacks become indirect and may lead to
 ill-defined behaviour. Using the example from your suggestion
 (`My_Plugin::the_content`), maybe you filter this to change it to actually
 point to `[$myplugin, 'the_content']` (where `$myplugin = new
 My_Plugin_Main_Class()`). What if I have a separate class called
 `My_Plugin`, and I want to hook `My_Plugin::the_content()` in? Suddenly,
 the hook has been hijacked from underneath me, and that would be so much
 worse to debug when anything could be hijacking it.

 You said [comment:3 previously]:

 > Did you read the commenter's comments? His issue wasn't with developers
 who expose, it was that when they expose it's complicated for someone who
 is not a full time developer to grok how to work with it.

 To me, this feels like it's getting '''more''' complicated, not less.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/23259#comment:4>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list