drupal_prepare_form() works hard to pass $form_state by reference to hook_form_alter() and hook_form_FORM_ID_alter()...


  // Normally, we would call drupal_alter($form_id, $form, $form_state).
  // However, drupal_alter() normally supports just one byref parameter. Using
  // the __drupal_alter_by_ref key, we can store any additional parameters
  // that need to be altered, and they'll be split out into additional params
  // for the hook_form_alter() implementations.
  // @todo: Remove this in Drupal 7.
  $data = &$form;
  $data['__drupal_alter_by_ref'] = array(&$form_state);
  drupal_alter('form_'. $form_id, $data);

This fact is documented for hook_form_FORM_ID_alter(), but not for hook_form_alter():

function hook_form_alter(&$form, $form_state, $form_id) { }
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lyaunzbe’s picture

First time contributor here and I'd like to attempt this if possible. If my understanding is correct, you simply want to mention, in a comment above the function hook_form_alter():, that modification for the form with the given form ID ($form_state) is passed to its parameters? I just want to make sure I'm understanding the bug correctly before proceeding. Thanks

Berdir’s picture

A comment might not even be necessary, if you replace $form_state with &$form_state, that should be enough to indicate that it is by reference. Feel free to add a explaining comment too, if it makes sense for you.

lyaunzbe’s picture

FileSize
896 bytes

My first attempt. Changed the form_state @param description and the parameter itself.

lyaunzbe’s picture

Assigned: Unassigned » lyaunzbe
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

lyaunzbe’s picture

Status: Needs work » Needs review
FileSize
898 bytes

Okay. Diff'd from the correct, root folder this time. Hopefully it passes now :)

Berdir’s picture

I am not so sure about the comment. The thing is that we usually don't document that something is by reference (as you can see it from the function definition) and neither does $form have such a comment. So, just from reading the apidocs, I'd assume that only $form_state is by reference..

Damien Tournoud’s picture

@lyaunzbe: Thanks for working on this! I agree with Berdir, we don't usually add in comments that a variable is passed by reference. Could you reroll the patch without this change?

lyaunzbe’s picture

FileSize
637 bytes

Sorry. Rerolled.

lyaunzbe’s picture

FileSize
867 bytes

Using cvs diff this time, rather than regular diff.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

Perfect, thanks!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Welcome lyaunzbe! I committed your patch to CVS HEAD. Thanks.

Looking forward to more patches and other contributions. :)

Status: Fixed » Closed (fixed)
Issue tags: -Novice

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