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.
Comment | File | Size | Author |
---|---|---|---|
#4 | template_documentation-1889738-4.patch | 787 bytes | andybroomfield |
#2 | template_documentation-1889738-2.patch | 715 bytes | andybroomfield |
Comments
Comment #1
jhodgdonThanks! That sounds like a good idea. We need to patch Drupal 8.x first, then backport to 7.x.
Comment #2
andybroomfield CreditAttribution: andybroomfield commentedPatch to improve the comment on this function
(being shown how to do this as part of the London 'Everything about Drupal' meetup!)
Comment #3
jhodgdonThanks, 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.
Comment #4
andybroomfield CreditAttribution: andybroomfield commentedThanks for the encouragement.
I've added those changes, hopefully this is good to go.
Comment #5
jhodgdonExcellent: short and clear. I'll get this committed shortly.
Comment #6
jhodgdonThanks again! Committed to both 8.x and 7.x.