It would be very useful to have a mention that the extra parameters passed here are accessibly to hook_form_alter() implementations inside $form['#parameters'].

CommentFileSizeAuthor
#13 942718-d6-2.drupal.docs-drupal_get_form-params-stash.patch798 bytesjoachim
#11 942718-d6.drupal.docs-drupal_get_form-params-stash.patch811 bytesjoachim
#11 942718-d6.docs_.drupal_get_form-params-stash.patch1.28 KBjoachim
#5 942718.drupal.docs-drupal_get_form-params-stash.patch2.23 KBjoachim

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Title: Documentation problem with drupal_get_form » drupal_get_form and hook_form_alter should mention #parameters
Version: 6.x-dev » 7.x-dev
Category: feature » bug

Is that actually true? Can you point to where in the code this is set up so a reviewer can verify?

If it is true, it should be mentioned in the hook_form_alter() and hook_form_FORM_ID_alter() docs, rather than or in addtion to drupal_get_form(). And it should be done in D7 first, then backported to D6, unless it is only true in D6.

You file a lot of issues, which is helpful... Can you please also when you file issues, change the title to something more descriptive than "documentation problem with ..."? Thanks.

joachim’s picture

joachim commented

The relevant code and comment is in drupal_retrieve_form():

  // We store the original function arguments, rather than the final $arg
  // value, so that form_alter functions can see what was originally
  // passed to drupal_retrieve_form(). This allows the contents of #parameters
  // to be saved and passed in at a later date to recreate the form.
  $form['#parameters'] = $saved_args;

I had no idea this existed until I went hunting for a way to do this today!

> Can you please also when you file issues, change the title to something more descriptive than "documentation problem with ..."? Thanks.

Yup, will do!

joachim’s picture

joachim commented

Things have changed for D7: the args get put into $form_state['build_info']['args'] in drupal_get_form(). Though there's no mention of it in the docs there either.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Sounds like a good thing to document then. Just need a patch...

joachim’s picture

joachim commented
StatusFileSize
new 942718.drupal.docs-drupal_get_form-params-stash.patch2.23 KB

Here's a patch for D7.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Needs review

Need to set to "needs review" status in order to launch the testing bot, and alert humans that you have a patch. :)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Looks like a good patch to me, thanks! I've confirmed what joachim said in #3, that's where the args are put in D7, and it is a good idea to document them.

When committed, please pass to D6 for porting, thanks.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

joachim’s picture

joachim commented
Status: Fixed » Patch (to be ported)

Needs some changes for D6.

Where does the equivalent of modules/system/system.api.php live on 6?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

The hook docs for D6 are in the contrib repository. You can see the file name on the page on api.drupal.org.

joachim’s picture

joachim commented
Version: 7.x-dev » 6.x-dev
Status: Patch (to be ported) » Needs review
StatusFileSize
new 942718-d6.docs_.drupal_get_form-params-stash.patch1.28 KB
new 942718-d6.drupal.docs-drupal_get_form-params-stash.patch811 bytes

In the docs repository, you mean?

I assume this one: http://drupalcode.org/viewvc/drupal/contributions/docs/developer/hooks/c...

(Meta: are there any docs on how to get a checkout of this? I've got far too used to the CVS instructions tab on project pages :/)

Here are two patches, one on D6 core for drupal_get_form() and one on the docs for the hooks.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Docs on checking out contrib repository: http://drupal.org/node/321

Regarding these patches, I think the form.inc patch is using the D7 location of $args, not the D6 location?

I've committed the other patch to the contrib repository.

joachim’s picture

joachim commented
Status: Needs work » Needs review
StatusFileSize
new 942718-d6-2.drupal.docs-drupal_get_form-params-stash.patch798 bytes

Oops.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

That looks better. It applies cleanly, and should be committed to the D6 docs. Thanks.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 942718-d6-2.drupal.docs-drupal_get_form-params-stash.patch, failed testing.

joachim’s picture

joachim commented

Sounds like the bot is on crack again:


Setup environment
Detect an invalid file URL
Detect invalid repository information
Detect invalid patch format
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

I had at least 5 failures like this in my queue this morning. The details:

[10:09:06] Command [patch -p0 -i /var/lib/drupaltestbot/sites/default/files/review/942718-d6-2.drupal.docs-drupal_get_form-params-stash.patch 2>&1] failed with status [1] and output:
can't find file to patch at input line 8
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: includes/form.inc
|===================================================================
|RCS file: /cvs/drupal/drupal/includes/form.inc,v
|retrieving revision 1.265.2.34
|diff -u -p -r1.265.2.34 form.inc
|--- includes/form.inc	6 Aug 2010 11:02:49 -0000	1.265.2.34
|+++ includes/form.inc	20 Oct 2010 17:35:08 -0000
--------------------------

Thta's bogus. It should be able to find includes/form.inc. These patches apply fine.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks.

Status: Fixed » Closed (fixed)

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