-
-
Save rmccue/1626492 to your computer and use it in GitHub Desktop.
<?php | |
/***** | |
All new versions will be posted at | |
https://github.com/rmccue/Rotor_WPPlugin | |
Please use that repository instead of this Gist. | |
******/ | |
/* | |
Plugin Name: WP_Plugin class test | |
Description: A better test! | |
Author: Ryan McCue | |
Version: 1.0 | |
Author URI: http://ryanmccue.info/ | |
*/ | |
class WP_Plugin { | |
public function __construct() { | |
$self = new ReflectionClass($this); | |
foreach ($self->getMethods(ReflectionMethod::IS_PUBLIC) as $method) { | |
$params = $method->getNumberOfParameters(); | |
$hooks = array('filter' => array(), 'action' => array()); | |
if (strpos($method->name, 'filter_') === 0) { | |
$hook = substr($method->name, 7); | |
$hooks['action'][$hook] = 10; | |
} | |
elseif (strpos($method->name, 'action_') === 0) { | |
$hook = substr($method->name, 7); | |
$hooks['action'][$hook] = 10; | |
} | |
else { | |
$doc = $method->getDocComment(); | |
if (empty($doc) || (strpos($doc, '@wordpress-filter') === false && strpos($doc, '@wordpress-action') === false)) { | |
continue; | |
} | |
preg_match_all('#^\s+\*\s*@wordpress-(action|filter)\s+([\w-]+)(\s*\d+)?#im', $doc, $matches, PREG_SET_ORDER); | |
var_dump($matches); | |
if (empty($matches)) { | |
continue; | |
} | |
foreach ($matches as $match) { | |
$type = $match[1]; | |
$hook = $match[2]; | |
$priority = 10; | |
if (!empty($match[3])) { | |
$priority = (int) $match[3]; | |
} | |
$hooks[$type][$hook] = $priority; | |
} | |
} | |
var_dump(__LINE__, $hooks); | |
foreach ($hooks['filter'] as $hook => $priority) { | |
add_filter($hook, array($this, $method->name), $priority, $params); | |
} | |
foreach ($hooks['action'] as $hook => $priority) { | |
add_action($hook, array($this, $method->name), $priority, $params); | |
} | |
} | |
} | |
} | |
// Here's an example of a plugin that would benefit from the above | |
class Some_Plugin extends WP_Plugin { | |
// Will filter the_title | |
public function filter_the_title( $title ) { | |
return $title . '123'; | |
} | |
// Will filter the_content | |
public function filter_the_content( $content ) { | |
return $content . ' Add more content.'; | |
} | |
// Will run during wp_footer | |
public function action_wp_footer() { | |
echo "I'm in the footer!"; | |
} | |
/** | |
* @wordpress-action init | |
* @wordpress-action admin_init 25 | |
*/ | |
public function my_init() { | |
echo "I'm in the footer!"; | |
} | |
} | |
$some_plugin = new Some_Plugin(); |
@markjaquith I like the idea of the variable parameters to add_filter
, but I'm also still a fan of the PHPDoc comments. I still think the benefits outweigh the problems. (Especially given that eAccelerator doesn't appear to be supported any longer.)
As per your original implementation, I'm not a fan of having the priority in the name, however it's otherwise identical to my implementation with:
parent::__construct(array('filter' => 'filter__', 'action' => 'action__'));
I agree with not always hooking, but implementing an alternative to that is probably going to be hard (e.g, adding a @wp-add-hook-on plugins-loaded
).
Implemented the variable parameters to add_filter
/add_action
Questions about your proposal:
- Do you propose this syntax:
$this->add_filter( 'the_title', 'the_title2', 25 );
when a plugin needs hookname <> method name and when you need priority? - How would you propose support when static methods are used for hooks? This is a preferred architecture for some plugin developers including myself because it's easier to remove hooks and doesn't require an otherwise useless instance to hang around in memory.
- How do you envision themes that need to implement hooks using the proposed
WP_Plugin
class that want to use a class to limit public name usage in namespace for their themes? Would themes also useWP_Plugin
, or would you introduce aWP_Theme
class that supports the same methods? - What about a plugin that contains multiple classes? Would you envision all classes inherit from
WP_Plugin
? - Do you envision core adding
class:static_method()
APIs in the future that theme developers would need to call vs. the more simplisticthe_function_as_template_tag()
standard currently provided for theme developers?
Non-standard method name for hooks
You mention in your code comment that they are almost never used, and yes they are rare but I wanted to delineate for others who may not recognize (some of) the places where they are required or preferred:
- Hooks that include filenames such as
'load-plugins.php'
. - Hooks with dashes such as and
'admin_head-$page'
. - Multiple hooks using one function such as one used for
'load-page.php'
and'load-page-new.php'
.
Automatic Hooks
As discussion above in the long thread, it would be possible to do automatic hooking on an exclusion basis vs. an inclusion basis.
Your auto-hook example
I noticed you used 99 as number of parameters for all hooks. I have not explored that, does it not cause problems? And if not, why doesn't WordPress core use it?
I noticed you used 99 as number of parameters for all hooks. I have not explored that, does it not cause problems? And if not, why doesn't WordPress core use it?
It's essentially equivalent to all parameters, since no action/filter will ever have that many parameters. It shouldn't cause any problems, but some functions which are hooked in multiple times might need different numbers of parameters for each hook, so that could cause a problem. I do agree though, it should be the default in WordPress.
@rmccue - Thanks. I'm also curious about the political reason for not having it in core in addition to any technical reason.
Very basic implementation here: https://gist.github.com/1645537
Do you propose this syntax:
$this->add_filter( 'the_title', 'the_title2', 25 );
when a plugin needs hookname <> method nameand
when you need priority?
Yes. Or you can flip the priority and method name parameters. As one is always a string and the other is always an integer, we can easily tell what was intended. This allows you to leave off the one you don't need to provide (method name with default priority, or priority with default method name).
How would you propose support when static methods are used for hooks? This is a preferred architecture for some plugin developers including myself because it's easier to remove hooks and doesn't require an otherwise useless instance to hang around in memory.
Not really interested in that pattern. You have to store your ephemeral instance data somewhere. It's rare for an instance to have no data. And if it does, there's no reason to worry about a memory footprint. Would rather just go with one pattern rather than go "oops, gotta hold on to some data here... better change this to an instance now!"
How do you envision themes that need to implement hooks using the proposed
WP_Plugin
class that want to use a class to limit public name usage in namespace for their themes? Would themes also useWP_Plugin
, or would you introduce aWP_Theme
class that supports the same methods?
I might just have WP_Theme
extend from WP_Plugin
, and then have themes extend from WP_Theme
.
What about a plugin that contains multiple classes? Would you envision all classes inherit from
WP_Plugin
?
Depends. If they want to trigger things in the main plugin instance, they should have an interface to the main plugin... some sort of global plugin factory that gives you the instance. If they want to use WP_Plugin
methods, then they should inherit from it. At a certain point, a plugin is so complex that someone can just design their own personalized convenience wrapper.
I noticed you used 99 as number of parameters for all hooks. I have not explored that, does it not cause problems? And if not, why doesn't WordPress core use it?
We don't use it in WordPress core because built-in PHP functions cannot be passed more parameters than they accept without causing problems. The built-in PHP functions aren't build with PHP, so they don't behave the same way. It was discussed in this ticket. http://core.trac.wordpress.org/ticket/14671 I proposed an idea, but didn't really like it, and it's just sat there.
5ubliminal had some interesting comments towards the end about using reflection which might be worth investigating.
One other closing thought: do not underestimate how mind-meltingly confusing it is to someone reading your code when a comment changes how the code works.
You should also check out @scribu's wp-scb-framework.
You should also check out @scribu's wp-scb-framework.
Oh cool, wish I'd known about that before. Looks very similar in terms of hooking ability, although the PHPDoc tags aren't prefixed.
Not really interested in that pattern. You have to store your ephemeral instance data somewhere. It's rare for an instance to have no data. And if it does, there's no reason to worry about a memory footprint. Would rather just go with one pattern rather than go "oops, gotta hold on to some data here... better change this to an instance now!"
That can be stored in static properties if needed. Using objects is an antipattern in the WordPress plugin community that I'd very much like to see die.
5ubliminal had some interesting comments towards the end about using reflection which might be worth investigating.
See https://github.com/rmccue/Rotor_WPPlugin/blob/master/plugin.php#L48 - Works fantastically.
One other closing thought: do not underestimate how mind-meltingly confusing it is to someone reading your code when a comment changes how the code works.
I think that's a good point. Most people do see it in terms of being ignored, but I think there's a certain familiarity with it from plugin headers. In any case, it's certainly worth noting in a plugin class how methods are hooked in.
@markjaquith - Thanks for your quick reply.
Not really interested in that pattern.
That's disheartening to learn, at least if you are considering this for inclusion in core.
You have to store your ephemeral instance data somewhere. It's rare for an instance to have no data.
I find the static pattern works better than the instance pattern. For example I have a set of plugins that have over 50 classes where most use static methods and its rare for any of them to have any ephemeral data. Yes there are classes that are used for instances, but they are typically not coupled to WordPress and thus don't implement hooks using their methods.
We don't use it in WordPress core because built-in PHP functions cannot be passed more parameters than they accept without causing problems.
Gotcha, thanks. That clarifies why for methods in a class it wouldn't be an issue.
You should also check out @scribu's wp-scb-framework.
What specifically about it? It seems rather simple.
That can be stored in static properties if needed. Using objects is an antipattern in the WordPress plugin community that I'd very much like to see die.
How are they appreciably different? I mostly see it as a syntactic difference:
$this->foo
vsself::$foo
$this->bar()
vsself::bar()
add_action( 'foo', array( $this, 'foo' ) )
vsadd_action( 'foo', array( __CLASS__, 'foo' ) )
How are they appreciably different? I mostly see it as a syntactic difference:
new PluginClass();
is a misuse of PHP objects. Objects should be used when you actually need objects, not simply for grouping methods together (for that, the best option is a namespace, but static methods in a class is good enough). You'll only ever have one copy of your plugin in memory, so having the ability to create a new instance has no use.
In addition, $myplugin = new PluginClass();
is using global variables, which is another antipattern I hate (especially when it's for no reason).
@markjaquith - I concur with @rmccue on his points and will add that it's much easier to remove a hook with a static method than an instance method. You only need the class name to remove the hooked static method but you need the actual instance to remove the hooked instance method.
That's disheartening to learn, at least if you are considering this for inclusion in core.
We're not considering it for core at this time. I was just talking in terms of "if I were going to write my own plugin framework...". We've been talking about plugin frameworks for years, and I'm happy to let people go in a bunch of directions with it independent of core and see what happens. The only real difference between putting it in core and having it be a third party thing is that as a third party thing you have to take care of versioning (i.e. WP_Plugin_v34
) so that plugin A which expects version 34 doesn't get served version 22 of the framework class.
I find the static pattern works better than the instance pattern.
Works better how? Honestly asking here. I've always seen it as a syntactic difference, and I just have a (weak) preference for the instance pattern. Skinny arrows look nicer than a T_PAAMAYIM_NEKUDOTAYIM
to me. :-)
What specifically about it? It seems rather simple.
Just some prior art. And I always find @scribu's code fascinating to read!
Ah, we cross-posted.
In addition,
$myplugin = new PluginClass();
is using global variables
Well, that is its own issue, and certainly something to discourage.
Objects should be used when you actually need objects, not simply for grouping methods together (for that, the best option is a namespace, but static methods in a class is good enough). You'll only ever have one copy of your plugin in memory, so having the ability to create a new instance has no use.
I can see that argument... but that's still a bit of a stylistic observation.
I concur with @rmccue on his points and will add that it's much easier to remove a hook with a static method than an instance method. You only need the class name to remove the hooked static method but you need the actual instance to remove the hooked instance method.
Aha. This is a solid objection (get it? objection? — ::crickets::). I've always stashed the instance in Class_Name::$instance
to allow for people to hook in, but that isn't as obvious as just using the class name. Okay, you've given me something to think about!
You only need the class name to remove the hooked static method but you need the actual instance to remove the hooked instance method.
Great point.
Works better how? Honestly asking here. I've always seen it as a syntactic difference, and I just have a (weak) preference for the instance pattern. Skinny arrows look nicer than a
T_PAAMAYIM_NEKUDOTAYIM
to me. :-)
"Works better" meaning "works better with my mindset". Using an object (to me) makes no sense, because there's no sense of differentiation between objects. If I make two PluginClass
objects, what's the difference? Compare to the WP_User
class, which is a correct use of objects.
My mindset is always to default to static classes and only use objects if necessary, rather than the other way around.
Well, that is its own issue, and certainly something to discourage.
The problem is that one leads to another. I also don't particularly like the look of new PluginClass
without assignment, it looks ugly to me. :)
I've always stashed the instance in
Class_Name::$instance
So, you're using the singleton pattern? That's another thing I hate (although, I have used them where it made sense). Singletons are essentially just a reimplementation of static classes, since you go from Class_Name::method()
to Class_Name::$instance->method()
The only real difference between putting it in core and having it be a third party thing is that as a third party thing you have to take care of versioning (i.e. WP_Plugin_v34) so that plugin A which expects version 34 doesn't get served version 22 of the framework class.
This can be a problem, but it's simply a case of keeping backwards and forwards compatibility.
This is a solid objection (get it? objection? — ::crickets::)
slow clap
(I get the feeling this all deserves a proper blog post rather than discussion in comments.)
We're not considering it for core at this time.
Cool then.
The only real difference between putting it in core and having it be a third party thing is that as a third party thing you have to take care of versioning (i.e. WP_Plugin_v34) so that plugin A which expects version 34 doesn't get served version 22 of the framework class.
Yeah, versioning is a major PITA, one we are trying to figure out a best practice for now. And I really do not like having to embed the version number into the class name.
We especially need it for loading shared libraries within clients plugins. For example we have two plugins, one for Client A and another for Client B and we want to use in both client's plugins our own (for example) whizbang.php
library that is not a plugin (plugins seem best suited for things an end-user can and should control, and putting the dependency into mu-plugins
is not viable for a plugin distributed via wordpress.org to end-users.) Best we can come up with it a registration system that will only load the latest version if the two plugins have different versions of whizbang.php
. Then it will be up to us to ensure backward compatibility with new releases.
Any chance of getting something into core that can help multiple plugins share the same library? Essentially a standard feature that would allow a plugin to call a function like register_library( $name, $filename, $version )
and then load the latest one after 'plugins_loaded'
hook?
(static pattern) Works better how? Honestly asking here.
The biggest reasons were already stated but here they are again, and two more:
- No unnecessary instance lying around, especially one that might otherwise get stored in a global variable.
- It's easier to remove static method hooks.
- It enforces the notion that hooks are a singular thing, not one per instance. With instance methods people who don't understand the architecture might include the hooks in classes designed for creating numerous instances and thus the hooks get triggered multiple times, potentially with negative consequences. That won't happen if the static pattern is used.
- Least important, a personal preference is to avoid recommending
__construct()
to would-be plugin developers because the leading underscores make it look more cryptic to the themers who are already afraid of PHP and people might forget to callparent::__construct()
if they create their own__construct()
in their plugin's class. I always create a static methodon_load()
and that's where I hook'init'
and potentially others.
Just some prior art. And I always find @scribu's code fascinating to read!
I've always respected your opinion on WordPress coding since my very first WordPress-related project was working with you so I'm hopeful when we release Sunrise to open beta you will find its code similarly fascinating. Given your public comments regarding the future of WordPress I think you are the core team member who will be most likely to appreciate what we've been building.
Any chance of getting something into core that can help multiple plugins share the same library?
Oh. I can haz a vizion?
BTW I prefer real instances because it is easier to extend such classes. Yes, PHP now inherits static methods too, but you still cannot rely on it. As for removing filters/actions: If you want to allow this, you can store the object in a registry (WP still hasn’t one) or in a global variable (bah!).
One trouble with using methods that are named in a certain way, is that you can't have two methods hooked to the same hook with the same priority - for example, scripts() and styles() methods hooked to wp_enqueue_scripts that you may want to be individually unhookable or only conditionally hook able depending if a user wants to not use plugin styles, etc.
BTW I prefer real instances because it is easier to extend such classes. Yes, PHP now inherits static methods too, but you still cannot rely on it.
Yes you can, PHP has always inherited static methods. The only problem with it was that self
was always bound to the class it was defined in. With late static binding, you can use static
instead, but that sucks.
You'll notice in my add_filter
/add_action
methods for the static class, it works around this for PHP <5.3
you can't have two methods [...] that you may want to be individually unhookable or only conditionally hook able depending if a user wants to not use plugin styles, etc
That's one place where you definitely need code, so I agree with Mark's sentiment on making hooking itself easier.
We recently made a library management class for our premium plugins since they often share code. We went off the assumption that we'll want to use the latest version since we can't load the same class name at different versions.
I find myself hating the process of having to declare add_action, etc far away from the method they're calling and so I search big old google and found this.
Was surprised to find that your style of declaring method names so that the action, priority, etc can be parsed for add_action, etc is similar to what I did years before. I guess great minds think alike. I used said method for one of our company's internal WP plugins and it's still running now.
What I do not like about it is the fact that your method names will always look ugly.
I then saw your approach of using Reflection to get the doc comment which is smart and I almost went that route. Two reasons however prevented me from doing so
- It's easy to break. Some coder might mess it up and the code will still run without errors since it's only a comment.
- Optimizers like eAccelerator can mess it up as well as previously noted in one of the comments.
But that inspired me to think further and asked... what if it's not a comment? What if it's actual code that is declared in the method itself? I thought why not use static variables. A static variable named a certain way so we can easily find it with Reflection the same way you did with the doc comment.
Here's what I came up with:
https://github.com/easterncoder/wp_autohooks
Would love to hear any feedback.
(I can go to sleep in peach now)
@toscho — I'd rather focus on something that all plugin authors could use, not company-internal code that will never see the light of day.
To that end, I'd rather do something that simplifies hooking, without requiring esoteric PHP setups, strict PHPDoc comments, or clunky method names.
Something like this, for instance: