API page: http://api.drupal.org/api/drupal/developer--example.profile/function/exa...

Describe the problem you have found:
It appears that the function signature should be

example_form($form, &$form_state) { ... }

I found while running PHP 5.3.2 that the current signature throws the following warning:

<em>Warning:</em> Parameter 1 to MODULE_form_settings() expected to be a reference, value given in drupal_retrieve_form() (line 771 of /PATH/TO/WEBROOT/includes/form.inc).

Line 755 appears to merge a non-referenced variable in the 0 position of the $args array.

/includes/file.inc

751   $form = array();
752   // We need to pass $form_state by reference in order for forms to modify it,
753   // since call_user_func_array() requires that referenced variables are passed
754   // explicitly.
755   $args = array_merge(array($form, &$form_state), $args);
756
757   // When the passed $form_state (not using drupal_get_form()) defines a
758   // 'wrapper_callback', then it requests to invoke a separate (wrapping) form
759   // builder function to pre-populate the $form array with form elements, which
760   // the actual form builder function ($callback) expects. This allows for
761   // pre-populating a form with common elements for certain forms, such as
762   // back/next/save buttons in multi-step form wizards. See drupal_build_form().
763   if (isset($form_state['wrapper_callback']) && function_exists($form_state['wrapper_callback'])) {
764     $form = call_user_func_array($form_state['wrapper_callback'], $args);
765     // Put the prepopulated $form into $args.
766     $args[0] = $form;
767   }
768
769   // If $callback was returned by a hook_forms() implementation, call it.
770   // Otherwise, call the function named after the form id.
771   $form = call_user_func_array(isset($callback) ? $callback : $form_id, $args);
772   $form['#form_id'] = $form_id;
773
774   return $form;
775 }
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Title: Documentation problem with example_form » Example profile is totally wrong for Drupal 7
Project: Drupal core » Documentation
Version: 7.x-dev »
Component: documentation » API documentation files

I was going to move this to the examples project... But this is from the example profile, which is in the Doc project and should probably be removed. Didn't we have an issue about that? Let's see.
Oh yes, here it is:
#715176: Adopt example.profile - won't fix. OK, can't do that.

But this profile example is just totally wrong for D7, so we should probably fix it or remove it. Meanwhile, moving to the Documentation project, since that is (for better or worse) where this file is.

See also
#712870: hook_profile_tasks api documentation nonexistant?

Rok Žlender’s picture

Assigned: Unassigned » Rok Žlender
Status: Active » Needs work
FileSize
0 bytes

Initial work on porting example profile to D7. Things not done yet or I'm not sure if my solution is correct.

  • Code commnets need a complete run through
  • Language is not selected automatically
  • Old profile task needs to be migrated completely
  • Old example_form_submit needs to be removed and comments moved to example_task1_form_submit
  • In example_task1_form submit handler I pass entered text to $form_state['build_info']['args'][0] which according to http://api.drupal.org/api/drupal/includes--install.core.inc/function/ins... is where $install_state is. Looks very hacky and burried.
  • in example_task2 I manually construct url with install.php and parameters passed in $install_state

There is probably more that I missed will continue to work on it and also appreciate any feedback.

Rok Žlender’s picture

Whoops previous patch was no good. Trying again.

jhodgdon’s picture

Thanks for taking this on!

I took a look at the code comments. I think it looks generally good -- found a few style/grammar/punctuation/spelling types of issues:

a)

+; These two strings will be displayed on the initial profile-selecion

selection is misspelled, and probably profile-selection should not be hyphenated (two separate words would be better)

b)

+; if the profile is focused to install Drupal in some other language,

should be "if the profile is focused on installing..."

c)

+; that there's just one profile available, by deleting the default
+; profile.

There are several profiles provided with Drupal 7 actually -- two currently, but there's another one proposed.... So this should probably say "by deleting all other profiles".

d)

+; This example profile enables the same optional modules as standard profile.
+; plus the 'locale' module. But however, any available modules may be added
+; to the list, including contributed modules, which will be then reqired by
+; the installer. Configuration of these modules may be handled later by tasks.

- should be "as the standard profile" in first line
- first line should end in , not .
- "But however" in 2nd line should just be "However"
- required is misspelled in 3rd line

e) Generally, try to wrap comments as close to 80 character lines as possible, without going over.

f)

+  // only focused to a single language. We only need to list tasks,

"focused to" => "focused on"

g)

+  // only focused to a single language. We only need to list tasks,
+  // for which a page will be displayed; internally, unlisted keys
+  // may be well used too. It's also possible to return dynamic data
+  // here, adding/removing tasks on-the-fly depending on previous
+  // steps.

The sentence starting with "We only..." ... I have no idea what this is talking about or what it means. Maybe it could be rewritten so that I could understand something from it?

h)

+    // Our second custom task shows a simple page, summarizing the previous
+    'example_task2' => array(

Our coding standards require that all comments be sentences (so it should end in . and maybe some words were missing at the end too?)

i)

+/**
+ * Form API array definition for the example form.
+ */
+function example_task1_form($install_state) {

See http://drupal.org/node/1354 for standards on how to document form-generation functions. This applies to the submit function too.

j) All functions need documentation headers. There are a few missing.

Rok Žlender’s picture

New patch attached. Fixed from previous one:

  • Fixed comments made by jhodgdon
  • Language selection tested and works
  • example_form_submit removed and comments moved appropriately

Still needs work

  • Need to go through code comments again see if they make sense after all these code changes
  • Old profile task still need some work or just testing
  • In example_task1_form submit handler I pass entered text to $form_state['build_info']['args'][0] which according to http://api.drupal.org/api/drupal/includes--install.core.inc/function/ins... is where $install_state is. Looks very hacky and burried.
  • in example_task2 I manually construct url with install.php and parameters passed in $install_state
jhodgdon’s picture

Issue tags: +valid issue

I just marked #1377526: Fix up example profile as a duplicate of this issue.

Also tagging so that it doesn't get picked up by #1421874: [meta] Documentation Issue Queue Cleanup

apaderno’s picture

Assigned: Rok Žlender » Unassigned
Issue tags: -