drupal_alter() documentation describe $data as a structured array

Correct, drupal_alter('profile', $account); is an example where an object is passed to drupal_alter(), not an array. Patch attached for review.

Removed a trailing whitespace.

We should make 6 and 7 read the same here, which means fixing this in D7 first.

D7's description atm, btw, is "&$data The primary data to be altered."

@webchick: I'd left the D7 version alone since contrary to the D6 version, it's not buggy. I do agree it makes sense to reword it so the D6 and D7 versions are more similar.
@kiamlaluno: good point, I'll address that once the D7 patch is in and we're back to D6.

D7 patch attached, made some additional (minor) edits:

• "A string describing the data type of the alterable $data." => slightly altered to read "A string describing the type of the alterable $data.", as "data type" is misleading (we are not talking int, string and array here, but things like "form", "profile", "user", etc.
• "…this should be a keyed array as described above…" => changed to use "associative array" as this is more widely used in core docs.

Dear d.o., stop eating my patches...

Committed to CVS HEAD. Moving to Drupal 6.

Patch for D6.

I am not too happy with this:

  * @param ...
- *   Any additional params will be passed on to the called
- *   hook_$type_alter functions.
+ *   In case more arguments need to be passed and alterable, modules may provide
+ *   additional variables by using an array containing the index
+ *   '__drupal_alter_by_ref' to pass two by-reference arguments to the
+ *   hook implementation.

I think the wording is very confusing, and it doesn't explain how to use the __drupal_alter_by_ref or how to use this in general. Also, this seems to imply that all ... args must be alterable, which isn't the case. And the alter_by_ref thing doesn't make sense to me, and implies there can only be two by-reference args -- I'm not seeing that in the code.

Yeah, struggled w/that ...how about this?
@param ....
drupal_alter() normally supports just one by reference parameter. Additional params that need to be passed by reference should be stored within the $data array using the key '__drupal_alter_by_ref' . They will then be passed on to the called hook_$type_alter functions.

The @param for ... needs to say that it's for additional arguments that are not passed by reference, which is still not mentioned in your new proposal.

Probably the stuff about __drupal_alter_by_ref should go in the @param for $data, since that's where you pass it in?

Agreed.

Revised patch.

OK, this is much better.

The only little thing I would change now is that some places, it says hook_TYPE_alter() and some places, it says hook_$type_alter. I think the first form is better?

Hmmm...
" Additional params that need to be passed by reference should be stored using an array with the key '__drupal_alter_by_ref'."

Maybe this would be better as:

If you need to pass additional parameters by reference to the hook_TYPE_alter() functions, include them as an array in $data['__drupal_alter_by_ref']. They will be unpacked and passed to the hook_TYPE_alter() functions, before the additional ... parameters (see below).

Also, I think we should say "parameters" not "params" in the "..." section. How about:

Any additional parameters will be passed on to the hook_TYPE_alter() functions (not by reference), after any by-reference parameters included in $data (see above).

Thoughts?

Thoughts are: consistency is key, and, by merging in proposed changes, this patch now clearly communicates the handling of the different(additional) parameters. Thank you for the suggestions.

This is fine. Ignore the "test" failure - the bot is not working right now for D6.
Thanks!

Hum, it does not look like that $type can be an array.

You are right -- in D6 $type cannot be an array. Sorry for not noticing that in my review -- I must have been looking at the D7 code by mistake. That does need to be removed from the patch.

New patch per Gabor's comment:

augh! d.o. grabbed it twice somehow and I don't even know if there is a way to "unsend", sorry!

Thanks, looks good! And ignore it if the bot says it doesn't apply - the patch is fine.

Great, thanks committed.