Allow more granular theming of drupal_render'ed elements

Pushing 2) a little further, Eaton suggests a p() (for 'print') function providing a more generic shortcut :

Everywhere you did <?php print $var; ?>, you'd now do <?php p($var); ?>

Thus we don't really 'add more PHP for themers', since p() would be the one and only template print construct.

I'm not *extremely* happy about something like p(), but it's the only solution I can see that allows us to add more intelligence without putting a big burden on themers (managing security themselves, etc). There are some tricky subtle bits, too: would this make it possible to create the following?

Or does the variable unpacking process break references? Hmmmm.

Or does the variable unpacking process break references? Hmmmm.

Yes it does.

Option 2 also does not let us do:

Because 'bar' will already have been printed. In fact, there is no way to do this with PHPTemplate at all because it is a single pass system. And yet, we're going to have a lot of problems with this. You'll end up having to do stuff like this:

Which I guess is OK but won't make templatemakers happy.

Yes, that's the last 'drawback' I mentioned in the OP. I see no workaround for this, it's the price to pay for the 'print the rest (including the fields I don't want to know about or that were not there yet when I wrote the template)' feature - which is an essential facilitator.
And I still think that it should make templatemakers much happier than what they currently have to do to have 'granular themeing' : manual theming, that require them to play dangerous games with raw data (or make them depend on a theme-aware PHP dev who writes safe preprocessors for them)

#3 seems like a pretty small price to pay in the scheme of things and better than what we have now.

I quite like p($var); - easy to remember, makes templates very concise.

It also brings us closer to a situation where we might be able to avoid waste like #451156: Duplicate calls to format_date() in node rendering maybe?

The only drawback with p() is that it needs to take its argument by reference, so that $content['foo']['#printed'] == TRUE after p($content['foo']).
But then you can't do p('string literal'); which defeats the "forget 'print', use 'p()' everywhere" part.

Very interesting.

/me tinkers

Forgot to note what I mentioned in IRC: We want both p() and r(), because you often want to render a single field and use that inside l() or whatever.

References would become a nonissue if we used objects instead of arrays. But saying that might get me lynched 'round these parts.

Yep, moving all render structures to nested ArrayObject objects feels daunting.

Attached patch takes a minimal approach, by moving the $content variable to ArrayObject($node->content) in node_preprocess. This only adds an object wrapper around the toplevel array. drupal_render() still operates on an array. Not too elegant, but does the job.

Just for testing purpose, the patch adds two $node->content['test'] and $node->content['test2'] elements, converts garland's node.tpl.php to use p() and renders the two test elements separately.

Without witespace fixes.

The last submitted patch failed testing.

yched, what actually happens with this:

The idea is that if we want a p() function that doesn't take its arg by reference (and thus allows p('literal')), then we always need to provide the full $content ArrayObject, not $content['foo'] because then the #printed = TRUE bits wouldn't bubble up in $content.

The test code in the node template produces:

The test failure was caused by the original modules/node/node.tpl.php not converted - broke stark theme.
Attached patch fixes this.

Subscribe. Very intersting stuff. Dealing with drupal_render() in the theme context is an important task as we're moving more and more towards using drupal_render() as much as possible, which is a great thing. I was thinking about these issues quite a bit, and I generally agree with the approaches taken here - both regarding a p() or r() function to be used in templates, and about using ArrayObjects to always copy on reference. Will review more closely as soon as time permits.

Heh, took me 15 followups to realize that the issue with p() as a richer variant of print is that it specifically forbids the code merlinofchaos posted in #3 :

which is the only way to display "everything but $content['bar'], then $content['bar'] after that".

Two proposals :

1) If we want to keep p() (which does have a nice unifying feeling), it needs an optional 'return instead of print' boolean param, so the above would become :

Question : are we OK with exposing those subtleties for themers ? As far as I'm concerned, why not.

2) Forget p() and just introduce r(), as a simple alias for drupal_render(). That's going back to the proposal 2) in my OP.
r() doesn't need to accept string literals, so it can take its arg by ref, no more ArrayObject dance. The example becomes:

This allows more advanced stuff like r($content['bar']['baz']), while the p($content, $key) syntax doesn't let you access nested elements.
Attached patch implements this second proposal. If we chose to go that way, keeping both drupal_render() and r() functions or only one of them can be debated...

Note that in both cases, the 'prerender' part can still be done in the theme preprocessor for cleaner templates, which slightly mitigates the 'more PHP for themers' drawback of solution 1).

Both directions are valid IMO, and both are far better than what we currently have, so it's up to us to decide...

How about solving the theme-me-that-multiple-times and i-want-to-output-in-weird-order issues this way?

Aside from that, we are talking about micro-optimizations of a perfect world. People need much more PHP code in their templates to achieve a customized output currently. A simple r() function that themes entity elements properly is a big step towards that perfect world.

I really like the most recent proposal for r($content['bar']['baz']). More flexibility for themers is the primary motivation here. Also attractive is the performance gain. We will only be theming the bits that the template actually uses. This is pretty much the holy grail "pull system" that we've been needing. If you benchmark D7 now, you see that drupal_render() and theme() are tops in terms of costliness. This patch can reduce calls to both significantly.

I would like to go even further and have templates inform the nodeapi('view') functions about their needed vars so that only the needed parts are built. But thats a much larger project. And I have no great idea how to do it. Maybe we extend hook_theme() for templates so they advertise their needed vars.

I think we should rename drupal_render() to r(), but don't care too much.

Given yched's explanation, r() seems like a really good option. Also happy with renaming drupal_render() (unless sun's $force param turns out to be a theme specific need which we wouldn't want there otherwise).

After IRC chat with sun, it seems that 'output some piece multiple times by adding a $force_print_even_if_#printed_is_TRUE param for drupal_render()' would require some changes in drupal_render() to use the existing $element['#children'] property if one is already there, instead of recomputing it all the way down. Maybe it's trivial, maybe it has nasty consequences, I'm not that familiar with the D7 drupal_render refactors.

Also, note that the current approach in #17 option 2) lets you 'save' $foo = r($content['foo']); and print it in several places.
What it doesn't let you do is : display $content['foo'] both at it's 'regular' position inside the all-inclusive r($content) *and also* some place else.
I can imagine use cases for that, but it seems a bit specific.

So +1 for the idea, but I would vote for keeping it for a later patch.

Well, maybe it could simply be added to the r() wrapper :

I'm not sure right now whether it's a fine solution or an ugly hack ;-)

[edit: as catch points, I guess the answer depends on whether we think that this $force feature belongs to drupal_render() itself. It seems the general opinion is that we shouldn't keep both r() and drupal_render() - unless this gives us some reason to so]

On further thought, I see no reason why the 'force reprint' feature discussed in #18 and #21-22, if deemed useful, shouldn't belong to the regular drupal_render() function. At worst, it's an additional $force param defaulting to FALSE, pretty harmless. So :
- we don't need to decide whether we want it or not before we can merge r() and drupal_render()
- I'd still vote to discuss and code it in a followup patch

I won't embark in renaming drupal_render() to r() just yet. Would make a huge patch that gets outdated every 2 hours. I'll do so when either Dries or Angie approve the patch and the rename, and state something like 'this patch will get in as soon as it includes the drupal_render() / r() rename".

Similarly, the nice cleanup that this patch allows around field API's field_attach_preprocess() and $field['exclude'] will be done in a followup patch, to keep this one clean and simple.

Attached patch removes the 'demonstration' test code, and updates the theming of the other fieldable core entity: user-profile.tpl.php and preprocessor (which used its own special trick to allow finer theming control in the template).
Still TODO: update the variables docs in node.tpl.php and user-profile.tpl.php, no time or braincells for this tonight.

Sigh. Without whitespace fixes.

This could be quite relevant for adding RDFa to Drupal elements in a consistent and clean manner.

The last submitted patch failed testing.

Rerolled after #363905: move template_preproces_node/block out of theme.inc.

To summarize: This now needs an approval from Dries or webchick before we can move forward.
I'm tempted to mark this as 'critical', since it's a requirement to allow flexible theming of nodes and of any other 'compound' entity (fieldable or not)
It will also (in a followup patch) let us get rid of some ugly Field API code and concepts, inherited from CCK's workarounds to allow flexible theming of fields while being contrib.

What the patch does (see OP, #17, #19 for rationale) :
- defer drupal_render($node->content) and drupal_render($user->content) to templates instead of preprocessors, allowing granular rendering and layout of individual bits and of 'the rest'.
- introduce an easy-for-themers r() function (currently an alias of drupal_render) and use it so that default node and user-profile templates keep their current out-of-the-box behavior.

Still TODO when we get a green light:
- PHPdoc: update the description of the variables in templates
- actually rename drupal_render() to r(). When we start doing so, the patch will inflate and get outdated twice a day, so I'd prefer to get a formal approval first.

The last submitted patch failed testing.

Reroll, + bump.

Wow - finally looked at the diff and this is really straightforward. I think it is the way forward. I'm going to mark this RTBC so we can get some committer feedback. Committers should mark this CNW when replying. The two remaining TODOs at bottom of #27 are known issues.

Spotted one micro bug:

1. typo: descritption

So in this issue I see a lot of ycheds, moshes, and catches, but I don't see a lot of John Albins, geerlingguys, and emmajanes.

If we're trying to make this easier for themers, let's ask some themers for their thoughts and make sure one of them marks this RTBC. ;)

Tagging.

Seeing as my name has been invoked, I would just like to say that I like my bike sheds blue. ;) Seriously though: this is a really interesting patch. Incredibly simple and yet it does seem to put a lot more control into the hands of themers. Here are my comments:

(1) print r() and print_r() are too "close." I would never want to have to teach a class where I have to explain the difference between the two. I understand that "render" is a scary word, but "r" on its own is not descriptive enough to easily distinguish it from its unrelated cousin print_r. I can hardly recommend the word "extract" being shortened to e() without looking like the world's most vain person, but I would like to encourage people to do a little thinking about the potential problem of using print r().

(2) Will I really have to wrap all my variables in yarrr()? That means TWO functions (print and r()) to print a variable. Ugh. Annoying. On the other hand: the ability to print out "leftovers" is awesome. On this point I think the control that I get as a themer outweighs the twitch it's giving me. Especially if it allows me to extract the "fields" I want to customize the display of and then print the remainder of the fields.

If #1 can be "fixed" then I can ignore #2 and will approve of the change.

print r() and print_r() being very close was my initial thought, but I can't think of a better alternative, and I'll get over it. ;) I barely use print_r() anymore since I can use dsm() with Devel anyway.

Being able to print leftovers in $content without jumping through hoops would be a very exciting and much appreciated improvement.

thanks @emmajane.

I had a similar reaction to print r() - I kept seeing print_r(). So, lets get back to the motivation of r(). We are using single letter function name in order to avoid clutterring the templates with too much php. I think thats pretty important, so lets keep the brevity. I could also live with dr() which we keep as a wrapper for drupal_render(). That meets the brevity goal and is dissimilar enough with print_r(). The attached patch uses dr().

Attached also adds and polishes the docs in the tpl.php files. Specifically, there are instructions for viewing the available variables in the template and instructions specific to 'print the rest'. I think these are improvements, though we could drop them if others disagree.

@webchick - yes we do want to help themers with this patch. in addition, there are significant performance improvements here which will be realized as people customize their nodes and profiles (less rendering/theming). and code cleanliness improvements will come with follow-up fields/cck patches.

print r() vs. print_r() is a good reason not to use r(). I can live with dr() too.

Agree, just a couple of things:

1. It must be possible for end users not to have to do anything.
2. Since the $node content will be available on the page template, there is a slight risk that people new to Drupal theming will try to theme the node in the page template, thus blurring the difference. But since a lot of theming work over the years has been dedicated to getting at the structure in the already rendered $content, the exposed r() is wonderful.

Victor Kane
http://awebfactory.com.ar
http://projectflowandtracker.com

Instead of only wrapping drupal_render – considering that the short function is meant for use in templates – shouldn't we include the print statement right in there?

We could even go back to r(), or use d() or p(), and grab that coveted one-letter-function real estate, heh.

The use in a template would be very simple:

Very exciting patch overall.
I second the change from r() to dr(), r really would line us up for some frustration.
There does seem to be a lot more visual clutter in

as opposed to

But then a lot less clutter when you start pulling apart $content to theme individual fields :)

Benjamin's suggestion would cure that, if it's workable.

Ben's suggestion takes us back to yched's #1 - which was p($var) - except calling it dr($var) and not using it for printing strings (which would save messing about references. The issue there is the example in #17 where you want to pre-render something first then print it later. I'm not sure why we could just use drupal_render() directly for that case though (and probably in a pre-process).

Very exciting.
Don't have much of a clue of the technical side.

But second any other naming than print r
Make it print dr print vr print cr whatever.

Especially not-so-experienced coders would think: "ah, I forgot the underscore, let's put that in..."

As already mentioned in #8 and #21, themers often want to use the output for something else. Maybe an example clarifies it better:

Maybe this example isn't so good. Anyway, I hope you get the point.

This patch converts page.tpl.php as well.

So... keep dr() as a wrapper around drupal_render(), possibly renaming drupal_render() to dr() later. Then add p() which prints the output of drupal_render() so that in 90% of cases you do

Something like that?

The last submitted patch failed testing.

Wow, great to see so much feedback overnight :-). Thanks moshe for the updates.

About print dr($var) being tedious and the need for p($var) as a shortcut : I'm personally not really convinced. I found p() interesting as a generic, one-fits-all replacement for print, but the discussion above (#6, #17) showed it is difficult to attain.
If we cannot do this, then I think p() as proposed in #45 gives minor syntactic sugar but mainly brings confusion. I'm not sure having themers figure out 'when should I use an explicit print, when should I not' (as in the code sample in #45) is a real gain.

[edit: PS - autocompletion test fails ?? WTF ?]

I agree with yched. I would like to keep the number of new Drupalisms to learn here to a minimum. #45 makes me cry out in agony as it's not at all clear what's going on, and I forsee people getting terribly confused between p() and r() and when to use them vs calling a theme function directly.

RADICAL idea: Use a short, descriptive actual word rather than an abbreviation. Perhaps something like output() and render() (but maybe better words than that). Then it would be a lot more clear from reading the code what's going on. Not sure if the "pain to type" factor would outweigh the "not making Drupal theming harder to learn that it already is" factor, but I'm open to suggestions/critique.

I am a themer who only chimes in on these discussions occasionally.

"References would become a nonissue if we used objects instead of arrays. But saying that might get me lynched 'round these parts." - merlinofchaos

What would it be like to create a parallel "track" of object based theming along side arrays. The hardest part of learning theming for me was to learn to use nested arrays and pick the right "fields" out of a nested array:

The first time I had to do

I had a hard time getting my head around it. I think this is the most difficult part of theming at this point (assuming you bring decent CSS and JS skills to the table). I'm used to nested arrays now but the are still really hairy at times - especially with node reference fields. Node reference arrays get nested arrays in nested arrays - not real themer friendly.

I remember back in the day when I did some Cold Fusion they had a release where you could use use alternate procedural and object oriented techniques to accomplish the same tasks. What would it be like to do something similar in Drupal 7 theming. It may be too big of a task to accomplish by 9/1/2009 but what would it take? Would it be an improvement?

Regardless, the combination of fields in core and the ability to make $content work more like the form api where we can pick out fields and then just get the rest without duplication is a great step forward.

How about display(). I think that's more descriptive.

That autocomplete test failure was very helpful! We can't convert page.tpl.php in this patch. The problem is that $scripts, $styles and $head are all created during preprocess but rendering of $content happens later, during page.tpl.php. The problem is that #autocomplete_path and #attached_css have yet to add their css and js. This is a nasty chicken egg problem.

So, lets proceed with #36 and forget about the most recent patch in #44.

re #48 / #50 : output() / render() / display() / other_name()...
I guess I'd vote for render(). A little more technical than the other suggestions maybe, but :
- at least it shows there's something special about $content. As a themer, why should I 'display' $content but not 'display' $title ?
- it keeps a better connection to the actual internal structure and the way it's referred to in all our comments, docs, etc ('render', 'a render array'...), and thus makes an easier transition when themers need to take a few steps in drupal API territory (let's face it, it will still happen even after this patch...)
- having render() either as an alias or a replacement of drupal_render() both keep consistency in our APIs. IMO any other arbitrary name will be wrong on that regard : output() as an alias for drupal_render() feels awkward, and renaming drupal_render() to output() breaks all our docs (we don't want to have to talk about 'output arrays', do we ?)

r() would have been the perfect match next to t(), l(), but agreed that print_r() / print r() is unfortunate.

So, I'm noticing that with this current patch, we would still have the problem that in page.tpl.php some variables would require print dr($content); while other variables would just be print $title;. On the face of it, that seems likely to confuse the hell out of themers.

However, perhaps I have a solution to the above problem, the what-the-heck-do-we-name-this-function issue, and to the chicken-egg rendering problem. #306358: Add $classes to templates with new theme_process_hook functions was committed 5 seconds ago (seriously! http://drupal.org/cvs?commit=217598) and just added a new THEME_process_HOOK phase to template variable processing.

So to briefly describe what that issue introduced, the order of operations for variables on templates in D7 now is: (I'm using node as an example and skipping some possible hooks, like MODULE hooks, to reduce noise; see #306358 for full details.)

1. template_preprocess_node()
2. THEME_preprocess_node()
3. template_process_node()
4. THEME_process_node()
5. node.tpl.php

Right now, most variables are rendered in the template_preprocess_node phase (or earlier). The current patch idea above would move the rendering to the node.tpl.php via a dr() function (any body else see that as a “Doctor” function?).

But how about we move the rendering to template_process_node()? That's actually the explicit purpose of the new "process" variables phase; right now, we only render a $classes variable for use in the wrapping div of the template, but I fully intended to expand that idea to other places. And, to me, this seems like a really good place to use it.

Let me give a concrete example (I'm going to use a generic render() function; we can talk naming later):

1. pass an unrendered content array to template_preprocess_node() and call it $vars['node'].
2. in THEME_preprocess_node(), allow themes to render any pieces of $vars['node'] via:
   <?php
$vars['my_new_var'] = render($vars['node']->the_piece_i_want);
?>
3. in template_process_node(), render the rest of the content via:
   <?php
$vars['content'] = render($vars['node']);
?>
   [edit: oh! and in template_process_page(), $messages, $scripts, etc would need to be rendered after $content which neatly solves Moshe's issue, no?]
4. in node.tpl.php, simply:
   <?php
print $content;
print $vars['my_new_var'];
?>

This also gets rid of the ugly print r(); issue emmajane mentioned too. So we could go back to a simple r(), but I actually prefer render():

But I could live with r() since it would make Talk Like A Pirate Day more fun.

win, win, win?

I wonder if implementing some kind of lazy rendering would not solve the issue. If the variable is an object, we can implement the __toString() magic method to do "on demand" rendering, directly in the theme template.

Of course it doesn't work when the template / theme function expects the child variable to be fully rendered for some reason (two examples in core: the page template expects everything else to be rendered, and the simpletest_test_table theme function expects the child checkboxes to be fully rendered too).

Here is a dirty proof of concept. Really dirty, I only wanted to see if it could actually work.

It took me a few readings to grok JohnAlbin's suggestion. I really like it. I'll wait for some more feedback, and then I will reroll the patch as proposed.

You mean once $vars['my_new_var'] is defined in template_process_node(), in node.tpl.php you can simply use:

Right?

And yes, I agree that we should either do all use of rendering functions in preprocess functions or provide a function that includes the print (say, print_render).

I favor JohnAlbin's solution of a best practice of print $var (rather than print function()) in template files and define and render parts of the variables in pre-process.

Hm. The more I think and read about this issue, the more I'm inclined to say that we need to support all possible rendering ways. That is:

1. Render an element in $content. Don't output it when $content is rendered.
2. Render an element in $content. And output it again when $content is rendered.
3. Render and output elements in $content. And allow to output an element again afterwards.

If we don't do and provide all three variants, we are introducing a major WTF for Drupal themers. They have to deal with and study the internals of drupal_render() to find out how they can output the content in the way they want to. Not looking up any documentation will (of course) make them use the unsafe values of an element in $content again.

The current implementation seems to cover only 1) (the default).

A previous patch (#22) seemed to cover only 1) and 3).

But whenever you render an element individually, you have no chance to output it again within the regular $content.

FWIW, on recent follow-ups: While technically more clean, I wouldn't want to be forced to create yet another THEME_process_THING() function, (in general) detached from the template that is using it, just to shuffle some content elements around in the template/output.

Given Damien's idea, the above might translate into 1) $content->extract(), 2) $content->render(), 3) $content->output().

EDIT: Clarified 3)

- JohnAlbin's suggestion in #53 loops back to option 1) in my OP, except that the brand new template_process_node() step makes it actually viable (back then, it prevented CSS-only themes).

That's a way to do it. As sun points out, it means the logic moves from the template to (pre)-processors. What you want to do in there is in many cases specific to a node type (dependiing on node types fields or pseudo-fields), which is more easily done in templates (node-TYPE.tpl.php variants) than in processors (giant switch($node->type)).
On the plus side, it merges the points 2) and 3) in sun's #57 : if partial renders are prepared in processors, it doesn't matter in which order the template outputs them.

I think I'd personally favor the ability to work directly in templates (patch in #36), but I guess I could go with either approach.

- sun's 1) 2) 3) in #57 sound like optional flags for (drupal_)render()
1) is the default:

2) is a RENDER_ALLOW_REPRINT flag ("do not mark #printed = TRUE"):

3) is a RENDER_FORCE_REPRINT flag ("do not check for #printed"):

As I said above, this last case is not needed of we go with JohnAlbin's processors.

Those flags are a) features for drupal_render b) can be added in a separate patch c) will most likely raise controversial discussions and whatnot.
Providing only 1) without 2) and 3) is already worlds ahead of what we currently have, and I strongly disagree about it being a "major drupal WTF". *Please* let's not scope creep this to death.

- I'm not sure I see how partial render of $content['foo'] would work with Damien's approach in #54

I'm really warming up to JohnAlbin's proposal of using the new process phase. I'm a bit concerned about the phased complexity of preprocess, then process, then output via tpl.php, but I'm not sure that we can avoid it.

Also, my original idea for a dr() or render() or r() or p() function (whatever it would be named) was to have it support structured data OR strings. ie., its implementation would be something like:

The downside is that the person working with the tpl.php file still may need to understand that $content['field_blah'] is something meaningful, rather than $content. But training people to use p() or dr() or what not, regardless of the KIND of variable they're spitting out, would let them drill deep into the variable, or spit out simple strings, with the same function.

If people do their work in a process hook, that would leave us with simple 'just print out a string' tpl.php files again, and people splitting out chunks of $content would have to write PHP functions, right? And preprocess functions would be about 'assembling raw data for theming' rather than prepping strings?

OK - for clarity, let's name :

option 1 : JohnAlbin's solution - partial rendering of $content['foo'] is prepared in theme_preprocess, 'the rest of $content' is rendered in template_process [edit : fixed typo about process / preprocess steps]
Templates do regular 'print $prepared_var', and dont have to understand that some variables are strings, some others are render arrays.
Granular themeing has to be prepared in preprocessors, thus forcing you to maintain per node-type templates and preprocessors in parallel.

option 2 : we introduce "some" themer-friendly version of drupal_render() : render(), p()..., so that granular theming can happen directly in templates (patch #36).
Templaters have to be aware that some variables have to be treated special (render() - unless we can come out with a generic p() function, that can eat strings and arrays - see below).
You don't *have to* maintain preprocessors in parallel - but you can go that way if you want, option 2 "includes" option 1.

(+ option 3 : Damien's approach in #54 - still unclear to me)

About eaton's generic p() function in #59 : the discussion above raised two issues
- p('litteral string') breaks (because of by-ref param). Sad.
- it prints out of the box, so if you want to print $content['foo'] *after* 'the rest of $content', you need the flags mentioned in #58.

Some additional notes that will prove I either (1) understand or (2) need more help understanding the issues.

1. I like the idea of doing the work in the preprocess function. Conceptually this function is "really hard" for new themers, so I'm fine if it gets more complicated with things like case/if/else statements.

2. Now that I know it is possible to do it this way, I do NOT like the idea of having anything except
print $variable
in tpl.php files.
JohnAlbin is right: it will be hard to explain why some variables need it, but others don't.
eaton is also right: I would like it if the rendered $variable contained merely a string that could be printed in a template file.

3. I am neutral and leaning to "not-liking" the optional parameter to retain the extracted field in $content. If I'm extracting something but still want all content, I can create a custom $variable which contains the additional/repeated/redundant field (as I do now). I really liked having the option initially, but now I just think it's too much choice that will end up being more confusing than helpful.

I think those are all of my opinions. :)

emmajane, nick lewis on IRC, and obvioulsy JohnAlbin prefer the preprocess approach, so I suggest we call it a wrap.

re emmajane #61-3) : the current ways to reprint an already printed piece of $content (print $content['foo']['#voodoo_key'];) are not really ideal, so I'd still agree with sun about the need for a RENDER_ALLOW_REPRINT flag for drupal_render() - in a separate patch :-)

Subscribing

Here is the Albin method. Node system only. You can test the patch by applying it and clearing theme registry. Comments? I will proceed with profiles and pages soon.

To give an example of what #64 means for themers:
if you want to separate $content['field_foo'] and $content['field_bar'] for story nodes, you'd do

Small gotchas:

- patch #64 means that $vars['content'] is the $node->content array in THEME_preprocess() (because of the "// Flatten the node object's member fields." line in template_preprocess), but is the rendered content string after template_preprocess(). Could lead to hair-pulling wtfs, we might want to take $node->content out of the flattened vars.

- the most natural thing you'll do is probably name your 'partial render' variable "$field_foo", thus smashing the existing $field_foo which contains the raw $node->fied_foo data. Themers "shouldn't" need those raw data anyway, but that could still lead to annoying debug sessions...

Also, going for the 'preprocess' approach means that this new, recommended way of doing granular rendering has a chance to remain unnoticed by themers with a pre-D7 background, which will continue doing the ugly, unsafe manual themeing from raw data they were used to. I'm not sure people read the "D6 -> D7 theme upgrade" page if they don't upgrade an existing theme but start a new one.
The other option ("print $content" becomes "print render($content)") was more disruptive, and thus couldn't go unnoticed.

Actually, I liked the first approach taken much more (using render() or r() or whatever directly in templates). With the approach in the latest patch, a themer that wants to put one cck field in a different place still has to create or edit template.php, write an actual php function with a special name, modify the $variables array by reference... not really any easier than currently.

With render() inside templates, though, the average themer just has to learn to use one php function, and can then just do what he wants to do, without ever touching template.php. So to print out a cck field at a specific location, it's just print render($content['field_foo']); and then print render($content); for the rest. In my opinion this is really straightforward. Also, we can then for example add two more utility functions, show() and hide() that simply modify the #printed attribute of a renderable array element. So you could do the following in a template:

The three functions here would be very simple, nothing more than:

In my opinion, this is much easier than having to implement a preprocess function whenever you want to render the node content (or everything else that uses renderable arrays) slightly different. Remember that we are using renderable arrays more and more. I really think the average themer can easily learn to deal with these three functions and renderable arrays. This saves us lots of preprocess functions and doesn't force themers to write php functions in many places.

About the concerns raised in #51 regarding #attached_css making it impossible to completely late-render the parts of the $page array in page.tpl.php: We just contemplated in IRC about a seperate iteration over the $page array prior to passing it into the theme layer (but after hook_page_alter) that would handle #attached_css and #attached_js and could also handle more stuff in the future like caching (http://cyrve.com/node/7) or a #render_callback that allows late-loading of data.

Then, even page.tpl.php could just receive a $page variable and the regions and other elements are printed via render($page['left']), render($page['content']), render($page['breadcrumb']) etc (as I proposed in #66).

Fixing tag. Other RDF-related core issues seem to be under "RDF" rather than "RDFa"

I'm swinging back and forth like a spineless cheerleader. I could easily do the Albin method or the yched method (use render() in the tpl.php itself).

I really want some solution for #67. I tried to add a drupal_prepare_page() after hook_page_alter() that handles #attached_js and #attached_css. But my recursion isn't looking clean or even working properly. We need some solution for this before I can get fully behind the render() proposal. help wanted.

I've just spent 30 minutes thinking about this, and I'm also swinging back and forth. It might mean we haven't cracked the code yet.

I think I'm leaning towards render(). The process function are rather complex.

Maybe it would be nice to be able to do: render($content['bar'], 'process_bar') where 'project_bar' would be an explicit process function. ;-)

Will using the render() method change how we work with variables in pre/process functions?

I've just spent 30 minutes thinking about this, and I'm also swinging back and forth. It might mean we haven't cracked the code yet.

Indeed. Long-term, I think it's one of the reasons that making renderable things objects, and overriding their toString() method to facilitate rendering, is an excellent approach. We could use print, and hide any behaviors we want to under the hood.

In the near term, though, it feels like we are choosing between the straightforward cleanliness of the tpl.php's Earl championed in D6, and the flexibility of render(). While the cleanliness is important, if we had to choose I would probably lean towards a render() approach that can take strings or arrays; yched's note about string literals doesn't bother me much, render('foo') doesn't concern me too much. are there many folks out there using print for literals in tpl.php's, rather than just leaving the literals in the tpl.php's un-parsed markup?

Will using the render() method change how we work with variables in pre/process functions?

Well, there would definitely be a stronger emphasis on leaving some things 'unrendered' before they hit the tpl.php files, rather than pre-rendering them into strings. Right now preprocess is used to turn data into rendered strings. That's an important philosophical shift.

So I tried out what problems it would bring if even the page regions would be rendered as late as possible, that is, in the template file.

The basic problem is, as outlined in #51, that we are then printing $styles and $scripts in page.tpl.php before the main page content is rendered.

In the attached very WIP patch I splitted drupal_render() in two parts: drupal_process_elements() which processes the elements (merging in default values, calling #pre_render, handling #attached_css and #attached_js) and drupal_render(), which really only renders the element and calls #post_render.
The two functions can be called independently. drupal_render will call drupal_process_element on elements that haven't been processed, though. This required moving block contents out of the #block->content property into an actual renderable element so that the block contents get caught by drupal_process_elements().

This means that #attached_css and #attached_js are handled fine now. What remains, though, is that it is not possible to call drupal_add_css and drupal_add_js in theme functions that are called from drupal_render via an element's #theme property, as that would happen too late (at least for menu callbacks that return unrendered arrays, which is the recommended way).

So - if we want to move towards completely rendering the page as late as possible, we have to move drupal_add_css / drupal_add_js out of theme functions and into #attached_css and #attached_js properties.

Also in this patch I converted node.tpl.php towards render(), show() and hide() to give an example on what I outlined in #66.

Note that these two parts of the patch are independent. We could move towards render(), show(), hide() without problems by leaving page.tpl.php alone (that means, rendering the regions in preprocess_page instead of page.tpl.php) if we think that rendering the main content in the template involves too much struggling. I would be completely fine with this.

Attached is a patch that uses render(), show() and hide() as explained in #66 without trying to convert page.tpl.php (and therefore not having to deal with css and js addition issues). See node.tpl.php on how one could use show() and hide() and also how we can then remove code from the preprocess function without making things harder for the themer.

Edit: Now that is cool, 100% passes for #73! Didn't expect that.

The last submitted patch failed testing.

Also: I agree with eaton and others that in the long term, moving our renderable structures towards objects is the way to go. This makes many things easier, the syntax can basically stay the same thanks to ArrayObjects, but for example we could get rid of #properties (by using -> syntax for them), we wouldn't have problems with losing references and we could use __toString for final rendering.

Reminds me of the "FAPI4" brainstorming code I did some two years ago (the code is in my sandbox, I've extracted some doxygen documentation. See especially the documentation of the DrupalElement class. The biggest problem with this code was that it heavily relied on circular references. These are quite a pain in PHP (regarding memory consumption), which I didn't know back then. And, of course, that it tried to rewrite half of Drupal, which usually is quite a problem by itself ;)
I still think though that there are some quite interesting and useful concepts in that code.

As I said a couple times before, I'm also in favor of 'render() in templates' vs 'preprocessor only'. It's far more flexible, easy to customize node-type by node-type (which is the most common use case), and you can still pre-render your variables in preprocessors if that's how you want to work with your templaters.

The one thing I don't want is see D7 punt because we couldn't pick between two solutions that are both immensely better than the status-quo :-). We need something in.

I was a bit skeptical with Frando's hide() and show() methods at first. Flags for drupal_render (as I proposed in #58) seem more inline with the usual drupal way. Then I figured that templates are a different world addressing a different audience, for which hide() and show() are probably easier to grasp and remember. So +1 from me.

I'm also not sure page.tpl.php *needs* to be converted to late in-template render() if that means important refactoring of the render process. Enforcing 100% consistency is not really important IMO, just use the best tool for the job. Are there actual use cases for late rendering of page regions ? If not, leaving them as flat, pre-rendered variables in the page template is not an issue.

I frankly dislike, however, the removal of *all* preprocessed node content (terms, links, comments) in #74. The consequence in node templates is awful :

I'd vote to keep comments, links and terms pre-rendered in the preprocessor, as was done in the earlier 'template based' patches.

@Frando - I think drupal_process_elements() is too much struggling. Let's use the Albin pattern for page while other templates use the yched's render() within the tpl.php technique.

I like how the patch leaves rendering of terms/links/comments to the template. Many sites disable these and there is no point in generating HTML that will never be used. These are nice examples for template authors about how to use the new functions. I agree it looks a little odd at first, but in the end I think it is empowering for themers to see this in action.

I'm certain we need hide()/show() in addition to render().

If noone gets to this sooner, I can probably roll a patch on Saturday.

OK. Here is a happy hybrid approach. As mentioned, page uses Albin approach since we can't use yched approach there. node and profile use yched apprach, and the templates principally use a new render() function which is an mostly alias for drupal_render().

I also brought over hide() and show() from Frando's patch. Those are used in the node template, as he proposed in #74. They might look a bit odd at first (#75) but they are very empowering for the themer and I like having usage examples in core.

I am getting tons of exceptions and a login failure running tests on my machine. Lets see if testbot can do better. I am also not seeing taxonomy and comment node links for some reason. Hopefully the tests will flush it out.

Some debug left in.

Not marking to CNW since there's plenty still to review.

Without that debug line.

I like the patch a lot.

Attached patch only removes some more debug code that was left and improves documentation a tiny bit, no functional changes.

IMO this is RTBC. This hybrid approach seems to be the best and easiest for the moment. It is a lot better to what we have now. Using render(), show() and hide() in templates gives a lot of flexibility to themers, and it's a lot easier to learn than learning to use process functions and stuff. So if JohnAlbin or some other themer is OK with the patch, go ahead and RTBC it.

Fine for me too, but I obviously can't RTBC myself.

/me drools and subscribes

One too many new lines here?
Everything else looks fine.

Removed the extra newline.

The last submitted patch failed testing.

re-roll of the patch from #86

frando and yched and myself are all in agreement that this is RTBC. I'll be the one to mark it so.

I've been looking this over and ran it past a number of the folks who showed up at the Drupal For Designers event. General consensus (at least among those I talked to) is that it's really, really awesome. Moving towards these kinds of structures allows us to treat something like $content as a standalone entity, OR dip in for a tiny piece and render the rest later, AND preserve the safety of drupal_render's output scrubbing. While preprocess functions allow this too, the leap from 'manipulating a template' to 'writing a preprocess function' requires juggling a lot of extra conceptual clutter. I'm cool with it as a developer, but in conversations with them, I think I've come to the conclusion that this feature smooths the "learning ledge" a lot.

I'd like to bikeshed about the use of render() as the function name -- p() or r() might be nice shortcuts, but that's beside the point.

Patch looks good to me, so let's go for it!

Yay! Awesome. A couple of questions.

Should we put this into something like $variables['page']['regions']['left'] ?

['page']

Yuck. :( Why do we have to do this again? It's not clear from the code, and I tried reading through the issue to spot it and only saw the reason that it's nice to have usage examples in core.... well. maybe. but not if it makes the code 10x harder to understand. At the very least we should comment this in the default template.

Looking at the new template output, we do introduce inconsistency (sometimes we use print $thing, sometimes we use print render($thing)), which may become problematic. I guess we'll find out as more themers start working with Drupal 7. But one nice benefit of this inconsistency is it's clear when you are dealing with a 'complex' data structure vs. just a regular ol' string. Once themers learn what render() can do for them, they can view any template and go nuts with switching things around.

So please answer my two inquiries, fix the comment typo, and resolve the last item either through documentation or by removing it, and then this is good to go, unless someone chimes in and says otherwise.

1. I don't think so. The only renderable entities in $page are regions so I see no need to nest them inside a regions element. Anyway, that's not strictly part of this issue. The $page array structure should be decided before this code runs.

2. Fixed

3. The goal of this issue is to let themers render parts of the node/profile and keep their presentation working when they install new presentation elements such as fivestar. This goal can be accomplished be doing rendering only in preprocess. Thats the 'Albin' method. The yched/frando method is a superset of the Albin method. Themers can alternatively choose to do rendering in a tpl.php file. Thats what this patch does for node and profile templates. I added a mention of hide() in the $content section of node.tpl.php. We already discuss render() there. Further discussion of how to write a template belongs in the theme guide, IMO.

This looks really really really cool!!

Is the idea then that the themer could go in and do somethink like: hide($content['links']['read_more']); to remove that or am i going down the wrong path?

small pieces of that kinda logic would be pretty awesome to have. I know its logic and should not go into the theming layer and yadi yadi yadi ;) ... but please ... pretty please ...

imho it will not be a problem for a themer to understand render($foo); its just telling 'em "hey this is the way drupal does it baby!" just as long its gonna be the same for every in the tpls (i had a huge problem understanding that $node was the array i had to grap to split up the $content

The problem themers still have (again imho) is: So why is drupal putting: FOO around my foo i only asked for the render($content['foo'])
hmm maybe a render($content[foo][#clean]) to get out and around the druaplistic markup? - and make both sides of the theming way happy ?

@mortendk and i cross-posted. resetting status.

yes, hide() how you would suppress the read more link. you could alternatively use hook_page_alter() but thats perhaps out of reach of template focused themers.

this issue is not strictly about markup. we are just discussing how to give more control about what prints where while keeping the "print everything else' feature.

The thing I don't like about this patch is that we're adding more PHP to the template file. In node.tpl.php, you see:

Don't we want the tpl.php files to be just minimal HTML with only outputting PHP variables? The Drupal NYC June meetup was yesterday and we talked about what went on at D4D and I was really impressed with the Studio theme did it. It forces you to just register the variables in the preprocess functions, and then gives a granular folder hierarchy for the overrides.

My thoughts are have the render() and hide() calls in preprocess functions, and then have the .tpl.php files just look something like this:

There is nothing *forced* about this approach. You can do your templates with just print statements if you wish. if you read the issue, we discussed the tradeoffs at length and decided to give the template author more power in the template *if he wants it*.

I love studio's approach too but it isn't really related to this issue. This is about more granular rendering, not injecting attributes into the template.

Removed an unrelated hunk in node.install.

re Rob Loach #96: [edit: crossposted with moshe]
"The thing I don't like about this patch is that we're adding more PHP to the template file / Don't we want the tpl.php files to be just minimal HTML with only outputting PHP variables"
We've been through this. This is #60 'option 1' vs 'option 2'. Since then, it seems a clear majority raised in favor of option 2.
Sticking to the (IMO short-sighted) statement that 'templates should just output php variables' means any flexibility needs to go in preprocessors, which requires *much* more drupal and PHP knowledge from themers.
The PHP added in the templates has been deemed simple and straightforward enough, even by theme-oriented people, as shown in several followups above.
*And* this approach still lets you work in preprocessors and keep straight prepared variables in templates if that's how you prefer to work.
So I see no reason to loop on this question.

re webchick #92:

I initially thought 'yuck' too, but I now think that templaters will more easily grasp how they can do granular templating if they find an actual working example already in place.

I really like the preprocess/process changes I see for page.tpl. :-)

The old template_preprocess_user_profile() used to uasort by weight. It looks like this was duplicate work since element_children() already sorts by weight. So nice optimization!

I updated the phpdoc blocks for render, show, and hide. Since it wasn't obvious to me why render()ing an element after hide()ing it worked, i.e. without first show()ing it.

I added a comment to the node.tpl.php to briefly explain why we are hide()ing stuff.

Also it looks like in modules/node/node.tpl.php, we were doing a print render($content['links']); which since render already prints seems redundant. I've removed the print bits; it seems to still work. Note: you need to test the default node.tpl.php using the Stark theme, since Garland uses a different node.tpl.

And, I removed the // print '<pre>' . check_plain(print_r(get_defined_vars(), TRUE)) . '</pre>'; comments from the tpls. The idea is very good, but the implementation is too scary for a beginner themer. If you uncomment them, it looks like a debugger threw up on the page. :-)

The 2 slight fixes (node_update_7004() mod removal and print removal from node.tpl) need review. Otherwise, this patch looks RTBC to me. :-)

Nice tweaks, JohnAlbin.

Excellent.

Render experts approve
#drupal-design approves
Testing bot approves
And both core committers approve!

I found one last thing when I was reading through:

it's should be 'its'

So fixed that, and committed to HEAD!! :) Woohoo!!! Marking needs work until it's documented in the module(?) and theme upgrade guides.

So. Cool. Many thanks to the many people that helped.

We can now remove one ugly bit of the Field API : #367215: Remove 'exclude from $content' display setting

Quick fix : print render() -> render() in user-profile.tpl.php

I somehow assumed that the omitted "print" before render() was left out by mistake in previous follow-ups. Why does render() print instead of return now? I've read back, but could not find the reason for this change.

For reasons to have render() return (not print), see #43 and earlier.

I committed #103 so that this is consistent with the other templates. I thought this was a failing in our test coverage, but for whatever reason the user profile in stark seems to show up fine even without the patch. Curious.

If we go sun's way (return rather than print), it adds more PHP to templates which themers seemed not to like (print render($foo) as opposed to render($foo)). You could also argue that if you're going to be sticking things into variables and calling other theme functions, you're into "that ought to go in template.php" mode, where render() is not necessarily intended to be used. But we can discuss it. Could some of the #d4d people chime in with their thoughts?

back to needs work for docs.

Less PHP in the template files = more design pleasure, for me at least.

To my OCD-ified self, seeing all this PHP stuck in the middle of my wonderfully-laid-out and formatted markup makes my heart skip a beat or two. I can live with a little (it's what makes a dynamic website possible), but the more code you put in, the less 'template-y' it feels, and the more WordPress designers feel justified in saying "Drupal is built for programmers." :-/

However, if the code is well-commented, and if themers and beginning Drupal designers can find out what all this stuff means easily and quickly, then I'm all for it.

That appears to be an argument for sticking with render() as opposed to print render() (slightly less PHP).

As discussed in #drupal, although this definitely does stick more PHP in themers' faces, once they learn the hide()/render() pattern, there is far, far less monkeying about with theme/variable overrides in template.php, or trying to find every single element of the $content array because you're forced to print them all out manually if you want to customize it in any way. These alternatives are all as "PHP in your face" as you can get.

We tried to reach a compromise of not running *all* variables through render(); for example, page.tpl.php remains mostly print $var statements. But for something like node.tpl.php, where teasing apart individual pieces and sticking them at random places is a pretty common occurrence, render()/hide() pass along a ton of power.

re geerlingguy: please re-read #98.

re sun; webchick: It's true that the current use of render() is kind of "in between".
IMO we have two directions:

1. Since render() currently takes care of print and accepts both flat $var strings and $content arrays, we could:
- rename it p()
- make it the generic, consistent way to print stuff in templates: print $title; -> p($title); everywhere.
We don't need to explain '$content is a bit special, you have to use render()", but instead "oh, with $content you can *additionally* do cool things using show() and hide()", which is a little less imperative.
Drawback: p('litteral') or p($count % 2 ? 'odd' : 'even') cause fatal error, which is a bit tough - node templates used to have some for some CSS classes until recently. Field templates currently have a few.

examples:

2. If the inconsistency with literals is problematic, we can decide we don't need a generic print function, and get the call to print out of render().

Personally, I'm in favor of p()/render()/whatever() our generic utility printing function. It handles renderable arrays, it handles strings, and if we wanted to be crafty, we could even check is_object() and handle that differently as we migrate portions of Druapl to OOP code. p() or render() could stay a consistent interface for 'outputting stuff'.

That is not really my point. Whether it's called p(), render(), or whatever(), in all cases, you can do:

Just returning and not printing in render() might also avoid confusion:

sun: which functions print and which don't is exactly the point of comment #109:
option 1 removes the ambiguity by stating that all prints are done by a p() function (currently named render()). p is for 'print', so it should be clear that this function prints and the others don't.
option 2 removes the ambiguity by stating that no function prints, all printing needs an explicit print. No p() function, render() stays render() and doesn't print.

Your last example, though (use the output of a render to build something else), is more problematic with option 1. You raised this use case several times in this thread, I have to confess it slipped my mind during the latest evolutions of the patch, sorry.

Updated options to account for this:

Our current code is:

1.
- remove the 'print' from render().
- introduce function p(&$var) {print (render($var));
- make p() the generic, consistent way to print stuff in templates: print $title; -> p($title); everywhere.
We don't need to explain '$content is a bit special, you have to use render()", but instead "oh, with $content you can *additionally* do cool things using show() and hide()", which is a little less imperative.

2. Rename render() to p(), keep the 'print' in there, add an optional $return param (just like print_r() has)

3. We don't need a generic print function, and simply remove the 'print' out of render().

re: yched/#109 / #98:

I did read that comment (and re-read it)... one thing that I've noticed about this whole discussion is the use of the term 'themer,' and how themers have come to accept the idea of extra PHP in the name of helping them more easily theme things...

I come more from the perspective of 'designer,' and I believe there's a bit of a difference between a designer and a themer. A themer, in my opinion, would be one who is more of a mix of programmer and designer, whereas a designer is someone who designs, and doesn't do a ton of 'theming' in Drupal-speak. For a lot of Drupal shops and design firms, a programmer-themer is not hard to find. For a one-man operation, or for an individual coming from a Dreamweaver/WYSIWYG editor who attempts to design a Drupal site (but has little programming experience), not so much...

My point is not to say that the render() stuff here is a bad idea. I just want the main Drupal developers to remember that designers are not necessarily Drupal themers, and in all these kinds of decisions, make sure you explain things and do them in a way that is accessible to non-programmers.

Especially in these .tpl.php files, which many designers (like myself) will be touching, make sure you leave a comment in if there's something weird going on inside. Or at least put a link to a node explaining things on drupal.org.

Plus one for yched's Option 1 in #112.

If we put print in any function, it should be a wrapper (of sorts, at least) for the original function.

Making p() our generic printing function, with render() still out there without a print, is ideal. People who want the .tpl.php files to be only 'print $var' style can put rendering in preprocess functions. So whatever we do, let's not embed 'print' in a larger render() function (even with a print_r style return parameter).

An unscientific poll of themers and designers at Agaric suggests that p() for intelligent printing could become a much-loved Drupalism.

^{benjamin, Agaric Design Collective}

Well, #2 is #1 plus the ability to fulfill's sun's l() example without resorting to output bufferring. I like #2.

I am working on docs for this issue.

Docs posted to http://drupal.org/node/254940#granular. Feel free to improve them. I think we should add that p() function so leaving this at CNW.

Also, #1 (function p(&$var) {print (render($var));) adds a nested function call for every 'non delayed print' operation, which remains the most common case. For flat variables, this means two nested function calls around a simple print $title...
So I also think #2 is more reasonable.

My vote is for #2 as well -- defaulting to print, with the option to return, gives a lot of flexibility with minimal overhead and avoids yet one more layer of call stack (p() to render()).

OK, here's the beginnings of a patch for #112-option2 : p($var, $return = FALSE)

This outlines the occurrences that cannot be moved to p() - some denote code that could go in preprocessors, but not all.

Moving print to p() in all templates is a bit tedious, so I did the first few (up to book.module) for now, and punt for confirmation that this is where we want to go.

The last submitted patch failed testing.

Forgot to update render() to p() in garland's node.tpl.php. Failure flood ensues.
Other than that, see comment in #119.

Hrm. I'm having a war right now between my profound desire for consistency everywhere, and the fact that this change would mean shoving Drupalisms on each and every designer/themer.

print $var;, though a tiny bit longer to type, at least is standard PHP, recognizable from anywhere, and portable anywhere. p() is a Drupalism, which is going to require a trip to the API documentation for every single person who tries to theme Drupal. (p()? What is that? Browse, search, read... Huh. Why not just print then?)

render() is of course yet another Drupalism, but it currently only affects certain, select templates (node, user, etc.). I think it's conceptually simple to say "If the thing you're trying to print out is complex, such as a node or a user profile, as opposed to a page title, then you use the render() function." Themers already know they can't print $var when $var is an array or object. Those things are "special." So would be be that much of a stretch to tell them to use render() on things they can't print? I don't really know.

I will ask some designers/themers to take a look at this patch once again. Thanks for the nice start, yched.

webchick: personally I think that consistency is more valuable. Inconsistency and a myriad of special cases is the bane of a Drupal newcomer's existence. As developers we've spent years frightened of showing themers even a smidgen of PHP, and working to eliminate everything but print() from our tpl files. While that is sometimes helpful, a far bigger problem is our deeply, massively nested tree of templates and variables.

Folks talks about designers favoring wordpress; it has nothing *but* custom one-off functions for every kind of data, and templates don't contain calls to print() at all. The difference isn't the lack or presence of PHP, but the complexity of the mental model necessary to build a working theme. If using p() or render() allows us to (even in some cases) reduce the amount of deeply nested drilling that people have to do to exercise control over the output, it will be a net win IMO.

Feedback from real designers who do real theming is absolutely desired, but I think we need to remember that complexity is a matter of mental models. Catering to those who don't know what a function is, but understand that they need to drill down to content-field--image-0-picture.tpl.php to change the ID of a wrapper div, seems like the 'Designer Experience' version of misplaced optimization.

Seeing the options laid out in #112 really has me torn. I keep reading it over and over and was debating over whether or not to post but here goes...

I understand the reasons behind having this all in a template file, and that goal is to hide the complicated nature of $content, but that fact is that it IS complicated and this patch makes it VERY cool at the same time, so why hide it or dumb it down?

I know this is going to sound weird, but the more I think about p() all over the place, the more I think it's a little insulting in a way. If I were just learning, I'd feel like I was being tricked into thinking that I need to do <?php p($title); ?> when really there is no reason. I'd also feel like drupal developers thought I was too dumb to understand the reasons behind it. It's very obvious what 'print' is in <?php print $title; ?>. People are used to it, code editors know what it means and highlight it so it's easy to read, snippets all over the place have it, and if someone really cares they can look it up on php.net. Also, since print still exists in a bunch of places as noted in #119, it's not consistent, which is confusing and hard to explain.

The point is, I think explaining what's going on and not dumbing it down is better, so I'm opting for #3. I'm not sure if there are plans to change this where theming forms are concerned, but at least this method seems most consistent with theming forms. If there aren't plans for that, I'd rather see <?php print drupal_render($content['field']; ?> and <?php unset($content['field'); ?> for consistency. OMG, did I just say that out loud?

/me dodges bullets, hides and hopes I didn't miss anything obvious.

Well, technically unset($content['field']) isn't what's going on under the hood of the functions in question. It's really setting $content['field']['#printed'] = TRUE, otherwise it would be impossible to 'show' it again in the future.

Also, it's important to keep in mind that p() is intended to maintain consistency across all the variables that are hended off to a tpl.php file -- whether they're strings, structured arrays, or what not. If that sounds like it isn't worth pursuing, so be it. But it will require that we educate people in a lot more depth about the difference between "a variable you have to render" and "A variable you can print."

Or, go back to the D6 approach and hard-render everything in a preprocess function.

I didn't realize about the #printed = TRUE part, though I'm quite used to doing <?php $vars['field'] = drupal_render($field_whatever); ?> with forms, so I can show it later so it seems familiar.

Personally, I'd prefer the education, but I will also be doing all of this type of stuff in preprocess functions anyway, so I'm probably not the targeted audience for feedback here. For me, that's the "right" way of doing it. If I was asked, that's how I would tell someone to do it (preprocess), but I wouldn't want to push that on people :)

I guess comments from newbie themers would be more helpful... Where are they?

Just subscribing for now. I need to look into this further.

I actually find option 3 in #112 the most appealing: render shouldn't print... print should print, and render should only render. In cases where you want to generate but NOT print the rendered item, you should be able to trust that nothing was printed, and the entire item is capturable.

print is standard php, p() is going to end up a confusing drupalism. Why add another bump to an already steep learning curve?

As someone who has spent the last 2 years doing not much but editing themes, I'd like to see something that is simple to use, but allows me to extend it as necessary.

The idea of using a p() to call a render() seems the most logical.

I don't care whether I have to use p() or print, as long as I don't have to go and check the format of the function every time I use it (rules out 2 from #112). I also don't want to have to remember something arcane like print render() for every field (rules out 3). Additionally, having to use something like p(l(p($foo,TRUE),node,node-id) is ridiculous and will trip every themer who deals with it.

So +1 for Option 1.

I can use p($foo) whenever I like.
I can use render($foo) if i need it.

This is far and away the easiest suggestion for a themer, even if it does break the upgrade path a little (but when has that ever stopped Drupal in the past).

I read through this whole issue when I should have been working. Oh well.. I have coffee. :) Will stay up a little later.

Jacine pointed out yched's #112 comment but I wasn't aware that all the big pieces where in already. I don't completely agree with the direction it took but it's my fault for not spotting this sooner.

On to #112:

I don't like option #2 at all. It should either print or not. It seems really strange to my eyes passing in TRUE which is very non-descript. TRUE what? Anyone who works with this will eventually get it but it looks so generic which contributes to that initial hurdle when learning how to theme.

?>

The above example posted by yched will be a common thing? $content['field_url_title'] is information that would never be printed on its own without any surrounding markup but themers will be expected to wrap it in a p() function anyway? I understand it's for consistency but that double p() wrapping is not easy to get. "p" stands for what now? Print? Or print when it doesn't get the secret message.? Now that doesn't seem consistent.

I like concise code but if we're going to have exceptions like the above example, I'll vote for no shortcuts just to prevent a "print" statement. So #3 has my vote.

The last submitted patch failed testing.

As someone who's less adept at theming Drupal, I think #2 is quite confusing, with the nested single-letter functions and the obscure TRUE parameter. I see drawbacks to both #1 & #3, but overall, I think I favor #3, because print is standard PHP.

There doesn't seem to be much support here for the p() wrapper. I'm leaning toward either leaving things as they currently stand (render does implicit print, for consistency with hide/show) or returning the value from render() and using print render(...); The designers/themers who've commented here seem slightly in favour of print render(...) from what I can see.

I'm probably the wrong person to comment because I think they are all great and personally quite like #2, but the inconsistency make me a little weary and I'd punt for #3 and ease of learning.

A large majority of templating systems out there (like Smarty or ETS) restrict the use of PHP within the templates and limit you to just outputing variables. This is because it forces a reduced amount of business logic within the templates, as well as helps with security. Due to this, if we were going to port Smarty theme engine or ETS theme engine to Drupal 7, we'd have to put preprocessing functions within smarty.engine to render the $content to obtain the rendered content rather then just having the content array.

Having known that, what are your thoughts on having a Drupal core theme engine (maybe named Starkly or something), that provides the themed variables as fully rendered elements rather then the element content arrays?

@Rob - yes, I would support such a theme, though i think it should be phptemplate based. see studio theme for a fine example.

Note that with #112 option 1, people who don't want to use p() don't have to. The reaction against it is from being made to use it / being talked down to.

Whatever happens, -1 for render secretly printing.

I agree with Jacine that p($title) is not necessary, and there is no need to hide the difference between complex and simple variables in core templates. But I think we should use

for and if someone does want to use p() everywhere for consistency (or, conversely, put render() in pre-process functions and always use print $var) they most certainly can.

Nothing too hard to understand, and if we want Drupal to claim every single-letter function in PHP we could rename render to r making a dead-simple render/return function usable everywhere. The complex use case becomes:

All in all I think we're not ready for p().
- it's (arguably) less visible in templates than a print that usually gets highlighted by IDEs
- We cannot make it generic: p('literal'); p(function_call()); p($conditional ? 'foo' : 'bar');p($var . 'baz'); break and still need to go through an old-school print, which really adds confusion.

Thus here's a patch for #112 option 3 (remove the print out of render(), everything that's printed in a template goes through an explicit print).

I kept the fact that render() accepts flat variables (simply returns them as-is), as a safe guard against render($title). "Don't baby sit broken code" applies differently when it's templater-code, I guess.

I think this is the best approach for now as it still leaves open the ability to create a p() if someone wants to champion the cause. I looked over the patch and it looks valid. I'll leave it to someone more vocal to set it as RTBC.

Today I needed to poke a stick at user-profile.tpl.php for unrelated reasons. I was greeted with:
<?php render($user_profile); ?>
and was *instantly* confused by the lack of "print."

With that in mind I am +1 for this latest patch which removes print from render(). Removing print also makes render() consistent with drupal_render() and drupal_render_form() and consistency is good.

Where possible I would like to see themers encouraged to move render() to template.php so that we can have print $rendered_var, which I think is the easiest to deal with. In other words, I don't want tpl.php files to end up looking like this again:
print theme('links', $primary_links, array('class' => 'links primary-links'))
(yes, this is already gone in D7, I'm just giving it as an example of something that is confusing)

Committed #138 to HEAD. Thanks! We can possibly revisit this later in the cycle, but for now agreed that this is more consistent.

Still needs upgrades afaik.

Fixed docs http://drupal.org/node/254940#granular

I believe that was the last issue for now. If p() is revisited it probably should be a new issue.

Yay, thanks redndahead !

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

Just read through the entire thread and as some who does a little of everything, code, theme, design, I can say that I understand (more or less) and support the final solution. I think it will be a great improvement!