Hook_help() gets removed completely.
Marking this as critical since removing the function is not better than making a comment about it. I was, until today, under the wrongful impression that hook_help was canned.

Deadwood:

/**
 * Implementation of hook_help().
 * @param $section
 *    The name of the section or url of the page for which help should be displayed.
 */
function foo_help($section = 'admin/help#foo') {
  switch ($section) {
    case 'admin/help#foo':
      return t('Foo can be very helpful.');
  }
}

Goodwood:

/**
 * Implementation of hook_help().
 * @param $section
 *    The name of the section or url of the page for which help should be displayed.
 */

Even the extra line from deadwood after the function was removed.
Hope this helps shed some light...
I had a look at the implementation to see if I can spot anything amiss but alas, my regex kunfu is lacking.

Thank you all for this very useful project!

CommentFileSizeAuthor
#1 hook_help-default-argument-HEAD.patch961 bytesnickl

Comments

nickl’s picture

StatusFileSize
new961 bytes

My kunfu is stronger now, stronger than this little problem.
When checking for the name of the hook_help() parameter don't assume default arguments weren't used in the deadwood module.
Remove parameter name from before the equals sign before changing the function paramater signature. This would previously result in the whole function retrieved from deadwood_find_hook() to be lost because no pattern replace would succeed with the default argument statement instead of the parameter name.

Attached is the patch rolled against HEAD which will also retrieve the paramater name if default arguments like _help($section = 'admin/help#foo') was used in deadwood implementations of hook_help().

nickl’s picture

Version: 6.x-1.5 » 6.x-1.x-dev
Assigned: Unassigned » nickl
Status: Active » Needs review

Please review and commit or shout if you need me to make any changes.

solotandem’s picture

Assigned: nickl » solotandem
Status: Needs review » Fixed

Fixed in next dev release created after this comment.

Thanks for finding this bug. Your solution works, but the commit takes a different approach and maintains the default parameter (in case that is of value).

Status: Fixed » Closed (fixed)

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