Documentation for hook_forms is incorrect

fgm - March 18, 2009 - 10:26
Project:Drupal
Version:7.x-dev
Component:documentation
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs review
Issue tags:API clean-up
Description

The current API doc for hook_forms show this function as not taking any parameters, which is wrong.

In D5, it takes any $args the caller of drupal_retrieve_form() chose to pass. See form.inc/drupal_retrieve_form().
In D6 and D7 until UNSTABLE-5, it takes a $form_id and the same $args as in D5.

#1

fgm - March 18, 2009 - 10:28

Note that the D6->D6 migration guide includes a bit of information about this change : http://drupal.org/node/144132#hook-forms

#2

add1sun - March 18, 2009 - 11:33
Project:Documentation» Drupal
Version:6.x-1.x-dev» 7.x-dev
Component:Correction/Clarification» documentation

Moving to correct queue.

#3

jhodgdon - July 13, 2009 - 17:54

Note that hook_forms() is called from http://api.drupal.org/api/function/drupal_retrieve_form , which in turn is called from drupal_get_form() (with an intermediate function drupal_build_form() in Drupal 7).

In Drupal 5, the input args to drupal_retrieve_form() are $form_id and flexible additional arguments passed into drupal_get_form(). The arguments passed to hook_forms() are the form ID and the additional flexible arguments. Documentation of hook_forms() doesn't make this clear, in my opinion.

In Drupal 6, the input args to drupal_retrieve_form() are $form_id, $form_state, and the flexible arguments from drupal_get_form(). $form_state is removed from the arg list before calling drupal_retrieve_form() in Drupal 6 -- so hook_forms() works like Drupal 5 (form ID plus flexible arguments). Documentation of hook_forms() doesn't make this clear, in my opinion.

In Drupal 7, the input args to drupal_retrieve_form() are form ID and form state. What is passed to hook_forms() in D7 is $form_id, plus the 'args' component of $form_state, which actually comes from the flexible arguments of drupal_get_form(). This is almost correctly documented in the hook_forms() documentation, except that it doesn't make clear that $form_id is not in the $args array.

Also, the Drupal 7 documentation for drupal_retrieve_form() incorrectly implies that drupal_retrieve_form() takes the additional flexible arguments, but this is incorrect -- drupal_get_form() puts these into $form_state['args'] and the flexible arguments are not passed in that form to drupal_retrieve_form(). So the item for "additional ... arguments" should be removed from the Drupal 7 documentation of drupal_retrieve_form().

So, in my opinion, D7 needs a small patch for hook_forms() and one for drupal_retrieve_form(), and D6/D5 need completely different patches for hook_forms().

#4

jhodgdon - July 13, 2009 - 20:54
Status:active» needs review

Here is a patch for Drupal 7 for review.

AttachmentSizeStatusTest resultOperations
405832.patch3.78 KBIdleFailed: Failed to apply patch.View details | Re-test

#5

System Message - August 21, 2009 - 19:40
Status:needs review» needs work

The last submitted patch failed testing.

#6

jhodgdon - August 21, 2009 - 19:47
Status:needs work» needs review

This is a doc-only patch. Submitting for retest, as there is no way this patch caused tests to fail.

#7

arianek - August 22, 2009 - 21:47

reuploading for bot

AttachmentSizeStatusTest resultOperations
405832.patch3.78 KBIdleFailed: Failed to apply patch.View details | Re-test

#8

c960657 - September 22, 2009 - 19:15

The lines are wrapped at considerably less than 80 characters. There is no reason to make them that short.

#9

jhodgdon - September 28, 2009 - 16:42

Here's a new patch, with some additional cleanup of wording, and wrapping at closer to 80 characters.

AttachmentSizeStatusTest resultOperations
405832.patch4.4 KBIdleFailed: Failed to apply patch.View details | Re-test

#10

sun - October 7, 2009 - 11:56

I'll review this after API freeze.

#11

System Message - November 4, 2009 - 15:30
Status:needs review» needs work

The last submitted patch failed testing.

#12

jhodgdon - November 6, 2009 - 17:52
Status:needs work» needs review

Here's a new patch, which should apply.

AttachmentSizeStatusTest resultOperations
405832.patch3.76 KBIdlePassed on all environments.View details | Re-test

#13

System Message - December 3, 2009 - 23:07

jhodgdon requested that failed test be re-tested.

#14

arianek - December 5, 2009 - 19:42

in the modules/system/system.api.php section, should "drupal-form_submit" be "drupal_form_submit"?

#15

jhodgdon - December 7, 2009 - 14:06
Status:needs review» needs work

Yes, needs a new patch to fix that. thanks!

#16

jhodgdon - December 7, 2009 - 14:20
Status:needs work» needs review

Here's a patch with that typo fixed.

AttachmentSizeStatusTest resultOperations
405832.patch3.76 KBIdlePassed on all environments.View details | Re-test
 
 

Drupal is a registered trademark of Dries Buytaert.