Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This is practically undocumented.
Comment | File | Size | Author |
---|---|---|---|
#24 | doc-hook_theme-1021568-24.patch | 3.59 KB | sven.lauer |
#22 | doc-hook_theme-1021568-22.patch | 3.6 KB | sven.lauer |
#18 | doc-hook_theme-1021568-18.patch | 3.07 KB | sven.lauer |
#14 | hook_permission_1021568_14.patch | 3.18 KB | skottler |
#13 | hook_permission_1021568_13.patch | 3.18 KB | skottler |
Comments
Comment #1
jhodgdonThe idea here is that in
hook_theme()
http://api.drupal.org/api/drupal/modules--system--system.api.php/functio...
in the Return Values section, variables vs. renderable elements makes no sense.
chx said in IRC:
* [it should explain what] is a renderable element, when should i use one instead the other and so on
* a form is a renderable element. the page array. it has children, it has # properties.if you pass it as a variable then it wont fly cos whatever you pass as a variable is extracted, ie array('foo' => 1) is passed into the template as $foo = 1;
I will write a better doc, just a moment...
Comment #2
jhodgdonHere's a patch for review...
Comment #3
jhodgdond8 at this point
Comment #4
jhodgdon#2: 1021568.patch queued for re-testing.
Comment #5
jhodgdon#2: 1021568.patch queued for re-testing.
Comment #7
Everett Zufelt CreditAttribution: Everett Zufelt commentedI know it isn't applying, but those docs are far more understandable than before.
Comment #8
jhodgdonLooks like this needs a reroll. Probably a good novice project?
Comment #9
skottler CreditAttribution: skottler commentedHere is the rerolled patch.
Comment #10
skottler CreditAttribution: skottler commentedSorry, initiating a test run.
Comment #11
jhodgdonLooks like we have a viable patch (thanks for the reroll!), and Everett reviewed the text. Let's get it in (8.x/7.x).
Comment #12
catchThis is great, instead of the required if not construct.
However couldn't this be the first sentence of the two descriptions that follow - this would save the cross-referencing/see above.
Inputs looks a bit odd here. I don't have a better word for it, but at the same time I don't think I've ever written 'inputs' anywhere. Maybe stick to using argument for the functions and variables for the template?
-1 days to next Drupal core point release.
Comment #13
skottler CreditAttribution: skottler commentedComment #14
skottler CreditAttribution: skottler commentedThe last patch had a space at the end of one of the lines. Re-roll attached.
Comment #15
jhodgdonThis is a bit odd as it is, in my opinion.
The information about when to use render element is in the main description, but the information about when to use variables is in the variables list item only. I think both should be treated the same (i.e., move the "use variables" phrase up to just after the "use render element" sentence)? maybe? I found it a bit jarring as it was, because I read that "use render elements" sentence, and immediately thought "and what are variables for"? and it wasn't answered until later.
Comment #16
sven.lauer CreditAttribution: sven.lauer commentedAlso, there is no "hook_element" (and there never has been). The hook used to be called "hook_elements" in D6, but now is called "hook_element_info".
Comment #17
moshe weitzman CreditAttribution: moshe weitzman commentedComment #18
sven.lauer CreditAttribution: sven.lauer commentedAttaching a conservative re-roll of @skottler's #14, fixing the hook name and addressing jhodgdon's #15.
The explanation of 'variables' (which I now pulled up to be part of the intro text before the list of array key/value pairs) might still need some work to be as clear as possible ...
Comment #19
sven.lauer CreditAttribution: sven.lauer commentedElaborating on my uneasiness with the 'variables' explanation mentioned above: The reworked text is much clearer that what we used to have, but it still does not quite tell someone who does his first hook_theme()-implementation what he should do.
Perhaps we should insert a paragraph that makes clear what the registered theme functions do, esp. since in D7, there are two rather distinct use cases. Something like
Of course, this would need quite some work, but such an explanation could be very helpful. Might be something for a follow-up issue, though.
Comment #20
jhodgdonSeems like a reasonable plan that would clarify this exact issue rather than a follow-up? But ... are they really two separate uses for hook_theme()? I guess so...
Comment #22
sven.lauer CreditAttribution: sven.lauer commentedOkay, re-rolled a version that includes a clarification of the kind suggested in #19. I thought that the proper place for this is at the top of the doc, as it specifies the general purpose of the function. Perhaps we could also pull more of the paragraph from the beginning of the return value description up.
Comment #23
jhodgdonI like this version's structure and wording as it is!
A couple of minor grammar/formatting/punctuation stuff that needs to be fixed:
a)
Either should not be capitalized here, I think?
b) You cannot have a blank line in the middle of a @return section, or the API module will think the rest is not part of the @return.
Comment #24
sven.lauer CreditAttribution: sven.lauer commentedRerolling. No wording change.
As far as I understand, there is no way to force a line break in the @return directive? I would have thought that the API module uses the indentation to figure out where the @return ends, but according to the doxygen standards, it does not ...
Patch applies cleanly to 7.x, as well.
Comment #25
sven.lauer CreditAttribution: sven.lauer commentedComment #26
jhodgdonRight, as far as I know there is no way to have a paragraph break inside a @return. This is usually not a huge limitation, because the vast majority of functions do not need them.
Anyway, thanks for all the iterations -- this looks good to go!
Comment #27
sven.lauer CreditAttribution: sven.lauer commentedNota bene, committers, that I joined this issue rather late. @jhodgdon and @skottler deserve at least as much credit for this patch as I do.
Comment #28
webchickGreat improvement! Committed and pushed to 8.x and 7.x. Thanks!