All #process callbacks get called with three parameters: the element to be modified and returned, the form_state (by reference), and the complete form.
Date very conveniently provides an alter hook for 4 of the 6 callbacks it uses, but only 1 of them gets all of those values.

In addition, hook_date_combo_process_alter passes the field info and field instance as context, which is useful, but the other 5 cannot do that.

Instead of having 5 hooks with $form as the third param and one with array('form' => $form), all 6 should just have a context.

CommentFileSizeAuthor
#1 date-1424656-1.patch8.14 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new date-1424656-1.patch8.14 KB

See attached.

karens’s picture

karens commented

I agree. I started creating these and found more and more different ways to use them, but never went back to make them more consistent. It turns out that drupal_alter is limited to 3 arguments, so the third one can only be $context to carry whatever other values are available. I think I originally need to have a reference to the third item someplace which is why one of them is missing the $context array (since there was no way to pass in a fourth item). Now I can't find that, maybe it is no longer needed. So long as we check that everything that is needed by reference in all the functions that use these hooks is still available by reference I am fine with this change.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

The only difference between the $contexts right now is that date_combo_process_alter gets access to $field and $instance. But the others shouldn't need that, and the process functions that call the alter don't have it or need it either.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: Unassigned » karens

I think this can go in as is.

karens’s picture

karens commented
Status: Needs review » Fixed

I found a few more changes needed where the Date All Day module implements some of these hooks. Committed with those changes. Thanks!

http://drupalcode.org/project/date.git/commit/dde9f0d

Status: Fixed » Closed (fixed)

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