As per #2006152: [meta] Don't call theme() directly anywhere outside drupal_render(), apparently since D7 was launched we're supposed to be calling drupal_render() exclusively and not call theme() directly.

The current docs for theme() https://api.drupal.org/api/drupal/core%21includes%21theme.inc/function/t... say this:

All requests for themed output must go through this function. It examines the request and routes it to the appropriate theme function or template, by checking the theme registry.

While this is technically true, it is confusingly worded. While all themed output must go through this function, the preferred way to ensure the themed output goes through this function is to build a render array with a #theme key set and then pass that array to drupal_render().

Files: 
CommentFileSizeAuthor
#25 theme-docs-2031319-25.patch1.44 KBjlindsey15
PASSED: [[SimpleTest]]: [MySQL] 57,207 pass(es).
[ View ]
#21 theme-docs-2031319-21.patch1.15 KBjlindsey15
PASSED: [[SimpleTest]]: [MySQL] 57,544 pass(es).
[ View ]
#16 theme-docs-2031319-16.patch1.14 KBjlindsey15
PASSED: [[SimpleTest]]: [MySQL] 57,496 pass(es).
[ View ]
#12 theme-docs-2031319-12.patch1.14 KBjlindsey15
PASSED: [[SimpleTest]]: [MySQL] 57,666 pass(es).
[ View ]
#9 theme-docs-2031319-9.patch1.14 KBjlindsey15
PASSED: [[SimpleTest]]: [MySQL] 57,132 pass(es).
[ View ]
#6 theme-docs-2031319-6.patch1.14 KBjlindsey15
PASSED: [[SimpleTest]]: [MySQL] 57,152 pass(es).
[ View ]

Comments

I don't necessarily agree that we should not ever call theme() directly... but I agree that it's preferable to call drupal_render() or to return renderable arrays rather than markup strings wherever possible from functions. So yes, please provide a patch with your suggested change, thanks!

Depending on what happens at #2008450: Provide for a drillable variable structure in Twig templates it may become a really bad idea to call theme() directly.

Regardless, in the meantime I'm happy to go with the "preferable" wording. Anything is better than nothing :)

We should also change/remove:

This function is usually called from within another function, like drupal_get_form() or a theme function.

From the docs of drupal_render().

Issue tags:+Novice

this could be a novice task

Assigned:Unassigned» jlindsey15

Well, since I am a novice, might as well get a few more simple tasks under my belt!

Status:Active» Needs review
StatusFileSize
new1.14 KB
PASSED: [[SimpleTest]]: [MySQL] 57,152 pass(es).
[ View ]

I think this makes it clear that calling theme() directly should be avoided, but doesn't say that the world will end if you do call it.

Status:Needs review» Needs work

Nitpick: one line in documentation is missing a '*'

Other than that, gets my +1

Also:

+ * Avoid calling this function directly.  It is preferable to replace direct calls
+ * to the theme function with calls to drupal_render() by passing a render array

put parentheses after theme() in the second line.

The wording is good though and thanks for the patch!

Status:Needs work» Needs review
StatusFileSize
new1.14 KB
PASSED: [[SimpleTest]]: [MySQL] 57,132 pass(es).
[ View ]

Fixed.

Status:Needs review» Reviewed & tested by the community

I'm going to dare mark this as RTBC as I see that all the comments in the issue thread have been fulfilled, it's been otherwise approved to be clear and it seems clear to me : )

Status:Reviewed & tested by the community» Needs work

trailing whitespace in #9

StatusFileSize
new1.14 KB
PASSED: [[SimpleTest]]: [MySQL] 57,666 pass(es).
[ View ]

Whitespace fixed.

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

Looks good to me.

Status:Reviewed & tested by the community» Needs work

One more nitpick: There is an extra space after the . in this line:

+ * Avoid calling this function directly.  It is preferable to replace direct calls

StatusFileSize
new1.14 KB
PASSED: [[SimpleTest]]: [MySQL] 57,496 pass(es).
[ View ]

Here you go.

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

Thanks! I'll get this committed shortly.

Status:Needs work» Reviewed & tested by the community

Nitpick:

* the theme() function directly is strongly discouraged - see next paragraph). It
* Avoid calling this function directly.  It is preferable to replace direct calls
* to the theme() function with calls to drupal_render() by passing a render array

These are 81 character lines.

OT: dreditor (greasemonkey userscripts) makes it real easy to spot these small errors.

Status:Reviewed & tested by the community» Needs work

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new1.15 KB
PASSED: [[SimpleTest]]: [MySQL] 57,544 pass(es).
[ View ]

Okay hopefully this is the last one!

Status:Needs review» Reviewed & tested by the community

I hope so too.

Seems like a huge improvement in documentation, looks great!

Status:Reviewed & tested by the community» Needs work

Can we get an @see drupal_render() in the docblock as well?

Status:Needs work» Needs review
StatusFileSize
new1.44 KB
PASSED: [[SimpleTest]]: [MySQL] 57,207 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

Thanks for the iterations and thanks Kiphaas7 for catching the 80-character limit! I'll get this committed shortly.

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

Thanks again everyone! I committed this patch to 8.x, and since the information was also correct for 7.x and the patch applied (with a little fuzz), I also committed it to 7.x.

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