Breaking off from #720876: Third parameter of user_module_invoke() should be object but user_register_validate() calls it with an array instead. as requested by pwolanin on IRC.
[15:41] <pwolanin> pillarsdotnet: can you add to the doxygen of http://api.drupal.org/api/drupal/modules--user--user.module/function/user_module_invoke/6 [15:49] <pwolanin> pillarsdotnet: please add some doxygen for the function I linked to indicate it has to be an object [15:49] <pwolanin> pillarsdotnet: other than that it's rtbc
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | user_module_invoke-docs-1192178-18.patch | 1.53 KB | pillarsdotnet |
| #16 | user_module_invoke-docs-1192178-16.patch | 1.88 KB | pillarsdotnet |
| #8 | php-test-program.txt | 940 bytes | pillarsdotnet |
| #8 | test-program-output.txt | 866 bytes | pillarsdotnet |
| #6 | user_module_invoke-document_parameters-1192178-6.patch | 1.61 KB | pillarsdotnet |
Comments
Comment #1
pillarsdotnet commentedPatch adds documentation for parameters.
Comment #3
pillarsdotnet commented#1: user_module_invoke-document_parameters-1192178-1.patch queued for re-testing.
Comment #4
jhodgdonTo me, the description of $type is a bit confusing, since it's not really the hook name, but the hook name without 'user_'.
Other than that, it looks good.
Comment #5
jhodgdonOh. Also, in D7/8, the paragraph above where your patch is states that it invokes hook_user(), which is not the hook name any more in D7/8. That should also be fixed.
Comment #6
pillarsdotnet commentedCorrected as suggested, and also added an explicit list of valid choices for the
$typeparameter. Considered adding@see http://api.drupal.org/api/search/8/hook_user_, but that would be misleading, as there are 17 API functions whose names begin with"hook_user_"but only 5 that have signatures compatible withuser_module_invoke().Comment #7
jhodgdonSo this is interesting. On another issue yesterday, I traced down module_invoke_* and realized that actually the args *are* passed by reference (see http://drupal.org/node/1193514#comment-4629398). So that line that says:
* We cannot use module_invoke() for this, because the arguments need to
* be passed by reference.
is actually incorrect. Maybe there is no good reason for user_invoke, and maybe there is a good reason, but that isn't the reason.
Any insight?
The rest of the doc in the patch looks really good to me...
Comment #8
pillarsdotnet commentedI wrote a test program which I believe shows that the comment is indeed correct, at least for my version of PHP.
Comment #9
jhodgdonyou are right. I was confused. Within user_module_invoke or module_invoke, everything is pass by reference via PHP's call_user_func functions, but module_invoke itself doesn't take its args by reference.
In which case, the patch in #6 is fine. D8/d7, and then mark as to be ported for d6 please.
Comment #10
pillarsdotnet commentedAt least for some versions of PHP < 5.3.0
Comment #11
jhodgdonWhatever. In any case, the patch is fine, so let's get it in.
Comment #12
webchickCommitted to 8.x and 7.x. Thanks!
Marking to be ported to D6.
Comment #13
webchickComment #14
pillarsdotnet commented#6: user_module_invoke-document_parameters-1192178-6.patch queued for re-testing.
Comment #16
pillarsdotnet commentedPatch backported.
Comment #17
jhodgdonIn Drupal 6, the hooks are all hook_user($op = 'whatever'), not hook_user_cancel() etc. So the patch cannot be simply copied from Drupal 7.
Comment #18
pillarsdotnet commentedCorrected.
Comment #19
jhodgdonThat looks quite reasonable to me. Thanks!
Comment #20
gábor hojtsyThanks, committed, pushed.