As part of #2299715: [meta] Move core hooks from system.api.php to core.api.php or other files, we need to move all the menu-related hooks out of the core/modules/system/system.api.php file.

Since there are a lot of them, I think they should go into their own menu.api.php file.

So the task here is:

a) Create a new file in core/modules/system called menu.api.php

b) At the top of the file, add a @file doc block. Use the one in core/modules/system/file.api.php as a model.

c) Then after that we need the @addtogroup hooks doc block, and at the bottom we need the "End of addtogroup" doc block.

d) In between, cut and paste the following hooks that are currently in system.api.php to this new file:
hook_menu_links_discovered_alter()
hook_menu_local_tasks()
hook_menu_local_tasks_alter()
hook_menu_local_actions_alter()
hook_local_tasks_alter()
hook_contextual_links_alter()
hook_contextual_links_plugins_alter()
hook_system_breadcrumb_alter()

e) I also think we should take the "@defgroup menu" doc block that is currently at the top of core/includes/menu.inc and move it into this file. It should go between the @file doc block and the @addtogroup doc block, near the top of the file. That way the topic and all the related hooks will be in one file.

Comments

arpit_nnd’s picture

Assigned: Unassigned » arpit_nnd
arpit_nnd’s picture

Assigned: arpit_nnd » Unassigned
Status: Active » Needs review
Issue tags: +SprintWeekend2015
StatusFileSize
new42.21 KB

Moved all the listed hooks:
hook_menu_links_discovered_alter()
hook_menu_local_tasks()
hook_menu_local_tasks_alter()
hook_menu_local_actions_alter()
hook_local_tasks_alter()
hook_contextual_links_alter()
hook_contextual_links_plugins_alter()
hook_system_breadcrumb_alter()

and the topic related to menu from:

  • core/modules/system/system.api.php
  • core/includes/menu.inc

to a new file: "core/modules/system called menu.api.php" .

sinniger’s picture

StatusFileSize
new42.21 KB
new356 bytes

Removed whitespace

Status: Needs review » Needs work

The last submitted patch, 3: drupal-menu-api-2407933-3.patch, failed testing.

The last submitted patch, 3: drupal-menu-api-2407933-3.patch, failed testing.

romina.nayak’s picture

Status: Needs work » Needs review
StatusFileSize
new19.3 KB

Removed white spaces.

chakrapani’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/menu.api.php
@@ -0,0 +1,451 @@
+  ¶
+}
+
+/**
+ * Alter local actions plugins.
+ *
+ * @param array $local_actions
+ *   The array of local action plugin definitions, keyed by plugin ID.
+ *
+ * @see \Drupal\Core\Menu\LocalActionInterface
+ * @see \Drupal\Core\Menu\LocalActionManager
+ *
+ * @ingroup menu
+ */
+function hook_menu_local_actions_alter(&$local_actions) {
+  ¶

There are still couple of white spaces in line 332 and line 347.
And also please provide a interdiff while re-rolling a patch.

Regards,
Chakrapani

romina.nayak’s picture

StatusFileSize
new600 bytes

Spaces removed & interdiff added.

romina.nayak’s picture

Status: Needs work » Needs review
StatusFileSize
new600 bytes
new42.42 KB

Spaces removed & interdiff added.

Status: Needs review » Needs work

The last submitted patch, 11: drupal-menu-api-2407933-10.patch, failed testing.

romina.nayak’s picture

Status: Needs work » Needs review
StatusFileSize
new19.06 KB
new23.77 KB

I am sorry I am somewhat new to patching. Above instructions incorporated!

Status: Needs review » Needs work

The last submitted patch, 13: drupal-menu-api-2407933-13.patch, failed testing.

romina.nayak’s picture

Status: Needs work » Needs review
StatusFileSize
new19.29 KB
new23.77 KB

Spaces removed & interdiff added.

Status: Needs review » Needs work

The last submitted patch, 15: drupal-menu-api-2407933-15.patch, failed testing.

adci_contributor’s picture

Status: Needs work » Needs review
StatusFileSize
new19.06 KB

Just fixed the corrupted line in patch from #15

jhodgdon’s picture

Status: Needs review » Needs work

PLEASE on this issue do NOT reroll patches. You need to go back to system.api.php and copy/paste the CURRENT hooks and their documentation to the new file.

So can someone please start over from scratch? The problem is that if you just do a "reroll" in a generic way you will miss copying over anything that has changed in the original file.

adci_contributor’s picture

Status: Needs work » Needs review
StatusFileSize
new0 bytes
new14.05 KB

I'm trying to start over from scratch. Copy/paste hooks from system.api.php in according to #18.
In this patch i'm skipped item e) from issue summary. Please, tell me if it need to do too.

adci_contributor’s picture

StatusFileSize
new9.35 KB

I'm sorry about the 0 bytes patch. Please ignore #19.

jhodgdon’s picture

Status: Needs review » Needs work

Yes, please do everything that is in the issue summary. Thanks!

sidharrell’s picture

StatusFileSize
new47.1 KB

First patch, so don't laugh.

sidharrell’s picture

Status: Needs work » Needs review

see, I said don't laugh

jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the new patch! I'm not laughing -- even people who have made hundreds of patches occasionally forget to set the status to "needs review" when they upload a patch. ;}

So... I guess there are a few more things that need to be done.

a) There is an error in menu.inc: that separate doc block:

- */
-
-/**
- * @section Rendering menus

That needs to be combined with the main @defgroup block in the new file -- take out the */, following blank line, and the /**.

b) We need to add an @addtogroup block to menu.inc (and an "end" block at the end of the file), so that the functions in there still are displayed on the topic page. See
https://www.drupal.org/node/1354#defgroup

c) In menu.api.php, end the @defgroup menu block with * @}.

d) Make sure that all the hooks moved into menu.api.php have @ingroup menu in their doc block (near the end).

sidharrell’s picture

StatusFileSize
new47.23 KB

for (c), there was an enddef block at the end of the file, but I took that out and did (d), per instructions. That has the same effect, yes?

for (a), your diff above has a minus in front of the @section line, but your instructions said just those 3 lines, not the @section line, so I left it in. Also, I noticed that it differed slightly from the @section lines above and below it, in that it those had a machine name for the section (?), I'm just guessing now.
* @section sec_controller Route controllers for simple routes
So I changed the rendering section line to:
* @section sec_rendering Rendering menus

sidharrell’s picture

Status: Needs work » Needs review

  • jhodgdon committed fd088ff on
    Issue #2407933 by romina.nayak, adci_contributor, sidharrell, sinniger,...
jhodgdon’s picture

Status: Needs review » Fixed

Excellent! You did exactly the right things. Thanks for reading my mind. ;}

The only very very minor thing now is a couple of extra blank lines in the new menu.api.php between hook_contextual_links_plugins_alter() and the next hook, but that is not a big deal. I fixed it on commit.

Beta note: This is just documentation and is unfrozen.

Committed to 8.0.x before Core changes again. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.