Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
function features_get_components($feature_source = FALSE, $reset = FALSE) {
...
$components_all = $components_source = array();
foreach (module_implements('features_api') as $module) {
$info = module_invoke($module, 'features_api');
/**
* Checks whether a component implements the given hook.
*
* @return
* The function implementing the hook, or FALSE.
*/
function features_hook($component, $hook, $reset = FALSE) {
$info = &drupal_static(__FUNCTION__);
if (!isset($info) || $reset) {
$info = module_invoke_all('features_api');
}
/**
* Gets the available default hooks keyed by components.
*/
function features_get_default_hooks($component = NULL, $reset = FALSE) {
static $hooks;
if (!isset($hooks) || $reset) {
$hooks = array();
features_include();
foreach (module_implements('features_api') as $module) {
$info = module_invoke($module, 'features_api');
foreach ($info as $k => $v) {
if (isset($v['default_hook'])) {
$hooks[$k] = $v['default_hook'];
}
At least 3 different areas are invoking hook_features_api. It'd be better to add one centerlized cached function, right?
Comment | File | Size | Author |
---|---|---|---|
#23 | features-centralize-hook_features_api-1390848-23-D6.patch | 6.38 KB | scottrigby |
#19 | 1390848_features_api_19_7.x.patch | 662 bytes | hefox |
#19 | 1390848_features_api_19_6.x.patch | 5.9 KB | hefox |
#13 | 1390848_features_api_12.patch | 6.94 KB | hefox |
#9 | 1390848_features_api_9.patch | 6.89 KB | hefox |
Comments
Comment #1
Grayside CreditAttribution: Grayside commentedAgreed. Seems like a fairly standard invoke_all wrapper.
Comment #2
hefox CreditAttribution: hefox commentedThings to consider with this patch
Why I think this patch is cool
Comment #3
hefox CreditAttribution: hefox commentedfeatures_get_components is what I went with cause it is already in heavy use and was already caching all the information; not sure why it wasn't already being used in the other places.
Comment #4
hefox CreditAttribution: hefox commentedsigh, the patch, oops. How many comments does it take to post a patch.. lala
Comment #5
Grayside CreditAttribution: Grayside commentedVariable names in features_get_components() are a bit confusing. Lots of in_ :P
Combined with lack of parameter documentation, the potential for confusion is high enough that I call it needs work.
Comment #6
hefox CreditAttribution: hefox commentedThe only $in_'s are the function arguments, aka two variables. Added the @params, though not the best explanation.
Better to do $in_'s then say, accidentally override the arguments in the function. *cough*
Comment #7
hefox CreditAttribution: hefox commentedActually, ya, the $in_'s should change, but that's easy enough.
Patch still reviewable, just need a tiny bit of work to change some variables.
Comment #8
Xen CreditAttribution: Xen commentedI've only eyeballed it, but it needs to be rerolled against head, doesn't apply any more.
Agree that the $in_'s should be changed.
In features_get_components:
if (!isset($all) || $reset) {
$all is not defined. Should probably be:
if (!isset($components) || $reset) {
Then:
Shouldn't that be:
? Apart from that, it looks good to me.
Comment #9
hefox CreditAttribution: hefox commentedPatch address the above and...
adds cache-ing of module_invoke_all features_api and drupal_alter. Not sure if it's worth caching this information, but generally think it's a good idea.
Also switches around args for features_get_components cause $component, $key makes more sense to me/seems more usual.
Comment #10
hefox CreditAttribution: hefox commentedRan all the current features test, and other than #1399440: Feature tests failure on image style against drupal 7.10 all passed.
Comment #11
Xen CreditAttribution: Xen commentedI don't see any setting of cache?
Comment #12
hefox CreditAttribution: hefox commentedThat's the 2nd time in the last two weeks I've forgotten that part. Duh. Updating patch.
Comment #13
hefox CreditAttribution: hefox commentedWhen porting this patch to d6, nearly everything is the same but drupal_static and cache_set (saying to remind myself.
Comment #14
hefox CreditAttribution: hefox commentedSome other notes on this patch:
Comment #15
Xen CreditAttribution: Xen commentedLooks good to me.
Comment #16
mpotter CreditAttribution: mpotter commentedExcellent! Commited to 7.x-1.x.
Comment #17
hefox CreditAttribution: hefox commentedDon't see any reason not to backport this (keeping the two version as close as possible makes them easier to maintain). Need to update the patch for drupal_static and cache_clear differences.
Comment #18
Grayside CreditAttribution: Grayside commentedMaybe features should just have a conditional backport of drupal_static, a la Real Name.
Comment #19
hefox CreditAttribution: hefox commentedFound a bug when porting.
features_get_feature_components needs to return only components that have true value 'feature_source', instead returning components that have that key set. array_filter fixes that up. Updated description of function to try and clarify what it means.
Fix for bug attached and back port to 6.x (version number in patch name).
Comment #20
Grayside CreditAttribution: Grayside commentedComment #21
hefox CreditAttribution: hefox commentedTalked to mpotter on IRC, he said I should commit it, so thus it is.
Anyhow, 6.x, up to bat!
Comment #22
hefox CreditAttribution: hefox commentedComment #23
scottrigbyRerolled for 6.x-1.x.
So far seems ok but could use a bit more testing. Posting this patch for now & will look again asap
Comment #24
DamienMcKennaBump for testbot.