Add a render filter to twig

What do you think @Cottser?

Attached "test only" patch demonstrates the need for this with a test failure.

Patch from #1 is RTBC from me, just a small addition and includes test coverage. Updating beta evaluation in the issue summary.

Related: It would be nice to unify the existing "render_var" function and call that "render" as well so this is available as both a filter and function. But for backwards compatibility maybe we can just add the following line and tests (or modify existing any tests that use render_var), but that should probably happen in a separate issue unless we agree it makes sense to do it all here. I don't see a strong reason to hold this one up though.

new \Twig_SimpleFunction('render', 'twig_render_var'),

Draft CR created: https://www.drupal.org/node/2448219

I think it's confusing you only want |render if you're doing things like |length or |trim. Because rendering of render arrays actually happens automatically, except in case you're directly applying a filter.

Isn't this therefore a TX/DX regression?

Can't we make TwigNodeVisitor not wrap the existing expression (and thus make the "outer" call one to twig_render_var()) but instead make it smarter to become the first invocation/filter so that it's the "inner" call?

(I don't know Twig terminology well enough.)

Assigning to Fabian to get his feedback.

Generally I am +1 to this change as its sometimes needed to render something and we already have a function, so its okay to also define a filter.

On render being smarter, there is a chicken-egg problem:

You might want to apply a filter to the render arrays, like the |exclude() filter, which is the show/hide, so you don't want to render it.

- You want to get a count, then you _do_ want to render it first.
- You also generally would like to not render things twice.

There is a way out by probably extending the API of twigs filters.

If |exclude() could specify that it needs render_arrays or raw values (similar to how they specify which escaping strategy is supported and if the result is safe), then it would be possible to indeed be smarter about the filtering, though some things like |join remain, that are impossible, because:

There is no way to know if something is a render array or not, #type and #theme are indications, but it can just be children with arbitrary keys ...

e.g.

[ 'london', 'paris', 'barcelona' ] is not a valid render array.

[ 'london' => ['#markup' => 'foo'], 'paris' => [...], 'barcelona' => '...' ] is.

This is related in ways to the visibility issue, which I still would love to discuss in IRC ;).

Below is a cross-post, but I fundamentally agree with what @Fabianx said - except it's |without(), not |exclude() ;)

Twig can deal with both arrays and strings. If we try to automatically render a render array every time it goes through a filter, AFAIK we not only kill our own |without filter, but also use cases for built-in array filters: join, first, last, keys, length, and more…

The failing test in #3 isn't because {{ render.array|length }} returns NULL or an exception, it's because |length returns the length of the array or string depending on what it's given - in that case 1.

So I don't think there are any fundamental changes needed here. Of course having objects with __toString methods as @scor said would be ideal but I don't see that happening for D8. And actually I'm not sure how you would distinguish in a Twig template from whether you wanted the "array" part of the object vs. the string part, you'd probably have to drill down to a method call or something to return the array. So DX/TX-wise not sure that is actually an improvement in this particular case.

Ok, just wanted to make sure we know what we're signing up for! Looks like we have sound justifications for doing things this way.

Indeed, thanks for the feedback!

Committed 9e13c54 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation.

The followup I mentioned in #3: #2455233: Add a render function to Twig (same as existing render_var function but a nicer name)