Use hook_module_implements_alter to allow modules to alter the weight of hooks in module_implements

Looks very nice, subscribing for a proper look later.

Ahh, except calling module_implements from module_implements() would cause an infinite loop. Probably needs a re-implementation to make it work.

I am wondering whether just disallowing altering the module_implements hook is enough.

subscribe. code looks good to me. should be rtbc after green.

I mean module_implements_alter, of course...

So this is really late, but it'd be so, so nice ;)

* When we added the registry, one thing we had planned was to allow individual hook weighting, but there were lots of other things to worry about, then it got ripped out. This is a minimal, completely optional solution and I can't see a way we'd actually get it cleaner than this.

* Using module weights for hook ordering is really nasty and gets us into all kinds of pickles. It's not a critical bug, but it is confusing - and new developers often run into problems doing things like trying to alter form elements which don't exist yet because another module is adding them later. The phpdoc here makes the reasons for this explicit, and provides a workaround which should mean less of the current module weights arms race (see #601642: Old databases might have a small {system}.weight for a critical bug that arms race is causing).

dunno what bot is weighting for. rtbc.

Oh, this is just so damn cool! Huge +1 and subscribing. I have often wanted the ability for a module to run certain hooks at a different weight than the module weight, and I opened #195275: Proposing change to allow modules to specify weight(s) per hook over two years ago, but it didn't get any traction. Thank you, aaron, catch, and chx!

Tsk, tsk, tsk you guys. :P Feature freeze was 5 months ago. :P~ Though catch summarized well on IRC when he said something to the effect that this gets us all the things we wanted from the registry in like 4 lines of code.

If this is going to go in, it needs tests. I'll need to run this past Dries, though. I'm terrified at what kind of subtle bugs this might introduce that we spend the next 6 months cleaning up and not cleaning up the criticals queue.

I'm unable to find this in HEAD - please commit again?

catch, webchick said "needs tests".

I'm a little confused why this needs tests. Do we have tests for other places where we add a drupal_alter() step?

I was thinking the same thing. There is nothing to test here AFAICT. drupal_alter() is already tested. Please elaborate and move back to CNW if we are mistaken.

See. Who needs new tests when we're already failing 800 existing ones (see #8). CNR for bot. Maybe we need tests after all because of the bootstrappiness issue (see patch), but I'm not thinking of what to test at the moment. Anyone else have any ideas?

And here's a variant that doesn't introduce the bootstrappiness mind-bender, and just puts drupal_alter() into module.inc.

#18 looks great to me.

+++ modules/system/system.api.php	13 Feb 2010 01:58:32 -0000
@@ -971,6 +971,28 @@ function hook_mail_alter(&$message) {
+ *  Alter the registry of modules implementing a hook.
+ *
+ *  This hook is invoked during module_implements(). A module may implement
+ *  this hook in order to reorder the implementing modules, which are otherwise
+ *  ordered by the module's system weight.
+ *
+ *  @param &$implementations
+ *    An array keyed by the module's name. The value of each item corresponds
+ *    to a $group, which is usually FALSE, unless the implementation is in a
+ *    file named $module.$group.inc.
+ *  @param $hook
+ *    The name of the module hook being implemented.

Duplicate leading space everywhere here.

Powered by Dreditor.

Please RTBC if satisfied.

Thanks!

Love your patch. Really. But

    // Move my_module_rdf_mapping() to the end of the list.
    unset($implementations['my_module']);
    $implementations['my_module'] = FALSE;

this made even me go o_O for a second until I realized that $implementations will be used in array order so it actually makes sense to remove and re-set the same key-value in an array but if you don't comment so this is a major WTF.

With #25. If this still seems like a WTF, please see #66183: Add helper methods to inject items into a particular position in associative arrays which I bumped to D8, but maybe we want to bring it back to D7.

Removing "needs test" tag as per #15-#17.

.

I am fine with this. I asked for a comment and yay i got a nice one.

subscribe

As I said elsewhere, I like the potential superpowers this provides: Imagine doing one thing with module A before module B, and another thing with module B before A! Now that is really eeeeevillll ... And I love it!

Wow, this is pretty jaw dropping functionality indeed. Code looks sane to me and comments are clear.

I definitely wanted this the other day in D6. Exciting stuff.

#692950 by effulgentsia, chx, aaron, catch, moshe weitzman, sun: Added hook_module_implements_alter() to allow modules to alter the weight of hooks in module_implements().

Talked this over with Dries, and we both agreed this is a far better solution than the previous attempt, and gives the nice advantage of achieving what we set out with the registry in just a couple of lines of code. He says he's cool with it, soooo....

Committed to HEAD! Great work, folks.