core/modules/book/book.module
89: * Implements hook_node_links_alter().
91:function book_node_links_alter(array &$node_links, NodeInterface $node, array &$context) {

core/modules/comment/comment.module
195: * Implements hook_node_links_alter().
197:function comment_node_links_alter(array &$node_links, NodeInterface $node, array &$context) {

core/modules/node/node.api.php
489:function hook_node_links_alter(array &$links, NodeInterface $entity, array &$context) {

core/modules/statistics/statistics.module
49: * Implements hook_node_links_alter().
51:function statistics_node_links_alter(array

In the documentation for hook_node_links_alter(), the first parameter is called $links. In all the implementations however, it's called $node_links.

CommentFileSizeAuthor
#4 node-links-2587337-4.patch4.81 KBsdstyles
#2 node-links-2587337-2.patch959 bytessdstyles
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

sdstyles’s picture

sdstyles CreditAttribution: sdstyles at FFW commented
Status: Active » Needs review
FileSize
node-links-2587337-2.patch959 bytes

Corrected it in documentation of hook_node_links_alter()

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Priority: Normal » Minor

In this way, all hook_node_links_alter() calls and definition are consistent, but i'm not sure naming $links $node_links is consistent with the others naming in same context.

For example:

  • hook_comment_links_alter() also use $links, so basing on this patch, we should rename it to $comment_links to.
  • hook_language_switch_links_alter() also use $links, so basing on this patch, we should rename it to $language_switch_links to.
  • hook_contextual_links_alter() also use $links, so basing on this patch, we should rename it to $contextual_links to.
  • hook_views_ui_display_top_links_alter() also use $links, so basing on this patch, we should rename it to $views_ui_display_top_links to.

We are on hook_SOMETHING_links_alter, so $links make reference to the something_links.

If we want to keep consistence, it's should be better to replace $node_links in implementations by $links and replace existing $links in this implementations by something else more appropriate.

sdstyles’s picture

sdstyles CreditAttribution: sdstyles at FFW commented
FileSize
node-links-2587337-4.patch4.81 KB

Patch based on approach from #3

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon as a volunteer commented
Component: documentation » other

Not documentation.

artfrick’s picture

artfrick CreditAttribution: artfrick as a volunteer commented
Status: Needs review » Reviewed & tested by the community

After looking into this, hook_node_links_alter only exists in these 3 modules: 

core/modules/book/book.module:function book_node_links_alter(array &$links, NodeInterface $node, array &$context)
core/modules/comment/comment.module:function comment_node_links_alter(array &$links, NodeInterface $node, array &$context)
core/modules/statistics/statistics.module:function statistics_node_links_alter(array &$links, NodeInterface $entity, array &$context)

The patch applies and functions as recommended. I made sure these match the API documentation for parameter names.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x, thanks!

  • catch committed 0555906 on 8.1.x 
    Issue #2587337 by sdstyles: hook_node_links_alter() names its parameters...

Status: Fixed » Closed (fixed)

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