hook_theme variables vs render elements

The 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...

Here's a patch for review...

d8 at this point

#2: 1021568.patch queued for re-testing.

#2: 1021568.patch queued for re-testing.

I know it isn't applying, but those docs are far more understandable than before.

Looks like this needs a reroll. Probably a good novice project?

Here is the rerolled patch.

Sorry, initiating a test run.

Looks like we have a viable patch (thanks for the reroll!), and Everett reviewed the text. Let's get it in (8.x/7.x).

+++ b/modules/system/system.api.phpundefined
@@ -1994,21 +1994,27 @@ function hook_permission() {
+ *   containing information about the hook. Each information array must contain
+ *   either a 'variables' element or a 'render element' element, but not both.

This is great, instead of the required if not construct.

+++ b/modules/system/system.api.phpundefined
@@ -1994,21 +1994,27 @@ function hook_permission() {
+ *   Use 'variables' if you are theming an item that has inputs for the data and
+ *   style; in this case, the variables not supplied by the calling function
+ *   will be given default values and passed to the template or theme function.
+ *   Use 'render element' if you are theming a single element or element tree
+ *   composed of elements defined in hook_element() implementations, such as a
+ *   form array, a page array, or a single checkbox element. The theme

However couldn't this be the first sentence of the two descriptions that follow - this would save the cross-referencing/see above.

+++ b/modules/system/system.api.phpundefined
@@ -1994,21 +1994,27 @@ function hook_permission() {
+ *     to determine how to render its inputs, such as the data to be rendered

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.

The last patch had a space at the end of one of the lines. Re-roll attached.

This 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.

Also, 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".

Attaching 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 ...

Elaborating 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

The functions declared by this hook have one of two purposes: Either they specify how a particular render array is to be rendered as HTML (this is usually the case if the theme function is assigned to the render array's #theme property), or they specify how an invocation of theme() should be handled ...

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.

Seems 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...

Okay, 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.

I like this version's structure and wording as it is!

A couple of minor grammar/formatting/punctuation stuff that needs to be fixed:

a)

two purposes: Either 

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.

Rerolling. 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.

Right, 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!

Nota bene, committers, that I joined this issue rather late. @jhodgdon and @skottler deserve at least as much credit for this patch as I do.

Great improvement! Committed and pushed to 8.x and 7.x. Thanks!