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.For consistency with other registry style 'info' hooks in core. For example:
hook_aggregator_fetch_info()
hook_aggregator_parse_info()
hook_field_info()
hook_filter_info()
hook_image_effect_info()
hook_node_info()
hook_entity_info()
hook_action_info()
...
Also, it's corresponding , hook_block_list_alter() should be renamed to hook_block_info_alter()
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | hook_block_info_alter_is_a_joke.patch | 12.28 KB | chx |
| #11 | hook_block_info_alter_is_a_joke.patch | 12.04 KB | chx |
| #3 | rename-block-list-3.patch | 14.85 KB | dropcube |
| #1 | rename-block-list.patch | 14.37 KB | dropcube |
Comments
Comment #1
dropcube CreditAttribution: dropcube commentedThe attached patch rename hook_block_list() to hook_block_info() and hook_block_list_alter() to hook_block_info_alter().
Comment #3
dropcube CreditAttribution: dropcube commentedFixed. Changed drupal_alter('block_list').
Comment #4
JohnAlbinThis rename is definitely needed for consistency. +1
And an eyeball review of this patch looks good. I'll apply it and give it a whirl later today when I have a bit more time.
Comment #5
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Let's update this in the upgrade instructions, and mark as 'fixed' once this change is documented.
Comment #6
dropcube CreditAttribution: dropcube commentedComment #7
eojthebraveWe've already got documentation that includes the new hook_block_info(), and since hook_block_list() never existed in Drupal 6 I think noting that hook_block() is now hook_block_info() is sufficient.
http://drupal.org/update/modules/6/7#remove_op
If not, I can write something else.
Marking as needs review, set it back to needs work if you we still need to write documentation for this.
Comment #8
sunI think that's sufficient.
Comment #10
chx CreditAttribution: chx commentedComment #11
chx CreditAttribution: chx commentedThis is broken. #732082: hook_block_info_alter does not alter hook_block_info was the fix but it totally belongs to the issue where a 14.85kb patch gets submitted and in seven hours it's in without any serious review of any sorts. Other patches linger for months in an RTBC state. No comment.
Comment #12
chx CreditAttribution: chx commentedSo why did I not simply change back the drupal_alter, added another drupal_alter and move on? Because there is no sane place to drupal_alter the hook_block_info results in the current block_rehash.
The whole thing is a sorry mess. We should have a block.module and a block_ui.module where block module takes the block info information from code, stores it only because it's faster to query a database than iterate over every defined block and then calls the relevant block view and puts the results in the page array. The block_ui should allow for customization and actually it should hook in via hook_block_info_alter -- but that hook only gets added properly in this patch so previously it was not available.
And then, of course, we got block_custom module also baked into all this sorry mess which creates blocks with their body and format stored in the block_custom table but a bunch of information only stored in the main block table... don't ask what happens to a custom block title when you switch on a new theme because that question only leads to a very messy new bug report :)
Comment #13
catchYep, the original hook_block_list_alter() was always for cutting down the array of blocks, not for altering the info. Nothing we can do here makes this good, but since we have a hook which is relied on to do one thing, but claims to do another, splitting it in two seems best. Given existing tests pass here and hook_block_list_alter() is explicitly tested that should be fine.
Comment #14
chx CreditAttribution: chx commentedI have written better docs ($theme was passed to hook_block_info_alter but not documented), while there I have added another context variable. There is no functionality change however so keeping at RTBC.
Comment #15
webchickThis patch murders kittens in a most vicious, horrible way, but I agree that the hook name as is is extremely confusing, and chx seems convinced that it was required to do all three of these changes together. ;)
Committed to HEAD.
Please document this change in the 6.x => 7.x upgrade docs.
Comment #16
andypostSuppose we could close #148531: make _block_rehash() reusable
Comment #17
chx CreditAttribution: chx commentedDocument exactly what?
Comment #18
rfayWhen changing APIs at this late date, we should make sure that the 6/7 update docs get checked (in this case they're too generic to matter) and please send a note out to the dev list at least. This broke block_example module and it would have been nice to have some notice. There may have been other contrib modules broken.
Please remember that people are developing against D7.
When making an API change, the best approach we've come up with is an explanatory note to the development list.
Comment #19
rfayHmm... We still have a hook_block_info_alter() (according to the doc). But it only gets called in the block admin interface? I probably don't understand, but it sure looks to me like hook_block_info_alter() doesn't do anything anymore.
Were the semantics completely changed from a runtime to an admin-time hook? There's nothing (clear) about that in the hook_block_info_alter() doc. Like why we would want to use it?
Do we intend not to have the runtime capability any more? Or is it to be done with hook_page_alter or something?
Can we add an @see to whatever should be used for runtime changes?
Comment #20
chx CreditAttribution: chx commented