API page: http://api.drupal.org/api/drupal/modules%21system%21system.api.php/funct...

The description of the return value covers the case where you supply a theme but not the other case where you do not supply a theme. The example code shows a function not returning anything. I suggest both cases be explicitly described in the Return description.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Title: Return value description does not say what you should return if you no not have a theme. » Return value description for hook_custom_theme() should say to not return a value if you don't want a custom theme for the page
Version: 7.x-dev » 8.x-dev
Issue tags: +Novice, +Needs backport to D7

Thanks! That sounds like a good idea. We need to patch Drupal 8.x first, then backport to 7.x.

andybroomfield’s picture

Status: Active » Needs review
FileSize
715 bytes

Patch to improve the comment on this function

(being shown how to do this as part of the London 'Everything about Drupal' meetup!)

jhodgdon’s picture

Status: Needs review » Needs work

Thanks, that's a good start! It looks like the people at your meetup got you going on git and patching -- and welcome to Drupal Core patch contributing!

A few things to update in this patch:
- Keeping in mind that this is hook documentation, we're documenting what your hook invocation should return. So the new sentence should start "Do not return..." rather than "Does not return...".
- The new sentence needs to end in "."
- The new text should start at the end of the previous line (wrap the whole paragraph to 80-character lines).
- Let's not be quite so specific. We're trying to convey that your hook invocation should not return anything if you don't want to specify a particular theme for this page. So let's not say "if no custom theme is specified in the query string", which only applies to the example function body, not the general case. Instead, say something like "Do not return anything if you do do not want to overwrite the theme", or something like that.

andybroomfield’s picture

Status: Needs work » Needs review
FileSize
787 bytes

Thanks for the encouragement.

I've added those changes, hopefully this is good to go.

jhodgdon’s picture

Assigned: Unassigned » jhodgdon
Status: Needs review » Reviewed & tested by the community

Excellent: short and clear. I'll get this committed shortly.

jhodgdon’s picture

Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to both 8.x and 7.x.

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