Now we have moved to psr-0 and make heavy use of classes/namespaces etc... it would make sense to allow the '#pre_render' and '#post_render' callback arrays to process any php callable and not just a function. This would promote better code organisation. For a good example of this, see views_handler_field_custom_pre_render_move_text in views.module.

Files: 
CommentFileSizeAuthor
#15 1993312-15.patch3.7 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,637 pass(es).
[ View ]
#15 interdiff-1993312-15.txt1.15 KBdamiankloip
#14 callables-1993312-14.patch3.42 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 56,307 pass(es).
[ View ]
#14 interdiff.txt1.56 KBtim.plunkett
#11 1993312-11.patch3.28 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,407 pass(es), 17 fail(s), and 36 exception(s).
[ View ]
#11 interdiff-1993312-11.txt625 bytesdamiankloip
#7 1993312-7.patch3.27 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,899 pass(es), 17 fail(s), and 36 exception(s).
[ View ]
#7 interdiff-1993312-7.txt1.35 KBdamiankloip
d8.render-callables.patch1.92 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,952 pass(es).
[ View ]

Comments

damiankloip’s picture

Issue tags:+VDC

.

aspilicious’s picture

Status:Needs review» Reviewed & tested by the community

Works for me :)

tim.plunkett’s picture

yched’s picture

+1 for this, of course.

But more generally, rather than keeping with one-off "this callback" and "oh, this one too" patches, it might be a good idea to check all remaining render and form callbacks and make sure they consistently accept callables ?

damiankloip’s picture

That sounds good to me, let's get this in now so we can continue with our views and field patches though. If none else does first, I will create an issue in the morning.

tim.plunkett’s picture

Title:Change pre_render and post_render callbacks to accept callables» Change pre_render, post_render, and after_build callbacks to accept callables
Status:Reviewed & tested by the community» Needs work

It seems the only remaining one is #after_build, let's just finish this off right.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new1.35 KB
new3.27 KB
FAILED: [[SimpleTest]]: [MySQL] 55,899 pass(es), 17 fail(s), and 36 exception(s).
[ View ]

Yeah, why not.

tim.plunkett’s picture

Status:Needs review» Reviewed & tested by the community

Beautiful, thanks!

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 1993312-7.patch, failed testing.

tim.plunkett’s picture

+++ b/core/includes/form.incundefined
@@ -1939,8 +1939,8 @@ function form_builder($form_id, &$element, &$form_state) {
+      $element = call_user_func($callable, $element, $form_state);

Apparently this will need to be call_user_func_array()? call_user_func() doesn't work with references

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new625 bytes
new3.28 KB
FAILED: [[SimpleTest]]: [MySQL] 55,407 pass(es), 17 fail(s), and 36 exception(s).
[ View ]

Ah, that is good to know.

Status:Needs review» Needs work

The last submitted patch, 1993312-11.patch, failed testing.

dawehner’s picture

Just in general: do we have to convert all of them to cufa?

+++ b/core/modules/system/system.api.phpundefined
@@ -282,7 +282,7 @@ function hook_queue_info_alter(&$queues) {
- *  - "#pre_render": array of callback functions taking $element and $form_state.
+ *  - "#pre_render": array of callables taking $element and $form_state.

Let's also change after_build, post_render

tim.plunkett’s picture

Status:Needs work» Needs review
StatusFileSize
new1.56 KB
new3.42 KB
PASSED: [[SimpleTest]]: [MySQL] 56,307 pass(es).
[ View ]

I looked into it, those docs are wrong.
And this is what was needed for the references to work. cufa is exempt from the call time pass-by-reference rules.

damiankloip’s picture

StatusFileSize
new1.15 KB
new3.7 KB
PASSED: [[SimpleTest]]: [MySQL] 55,637 pass(es).
[ View ]

Yep, let's do that.

aspilicious’s picture

Status:Needs review» Reviewed & tested by the community

Ok we should be good now. Docs and code look good.

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 523d64a and pushed to 8.x. Thanks!

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