Document $form_state and form builder function in form_api group

I suggested to rfay on IRC that $form_state should be documented on
http://api.drupal.org/api/group/form_api/7

Which is a rather lame page anyway. It could use some expansion... such as documentation of what $form and $form_state are, and maybe a suggestion of what the key functions are, such as drupal_get_form() or something?

subscribe

subscribing

This is a VAST improvement and is almost ready to be RTBC... A few things need changing (most minor):

a) Between "Its keys are:" and the beginning of the list of keys, there should not be a blank line.

b) There are some line wrapping problems, such as:

+ *   determines whether the values are
+ *   a flat array or an array whose structure parallels the $form array.)

Some of the words on the 2nd line should be moved up.

c) English grammar/usage:
"... a URL which ..." => "a URL that"

d) I don't think I'd include information about deprecated parts of $form_state, and this has a redundant "element that caused submission" in it:

+ * - 'triggering_element': (read-only) The form element that triggered
+ *   submission. This is the same as the deprecated
+ *   $form_state['clicked_button']. It is the element which caused submission,
+ *   which may or may not be a button (in the case of AJAX forms.)

e) Shouldn't start this with "This is":

+ * - 'input': (internal) This is the array of values in the $POST. For security
+ *   reasons, it must not be referred to.

f) Persist is not a transitive verb, so nothing can "be persisted". Maybe use persistent?

+ * Note that all $form_state keys not listed in form_state_keys_no_cache() are
+ * persisted, so are available in the validation, submission, and the form
+ * builder function (after first submission). In addition, they're available
+ * to a #ajax['callback'] function, if any. By convention, user values that
+ * are intended to be persisted are stored in $form_state['storage'] though.

g) Another "which" problem:

+ * In addition, there is a set of Form API tutorials in
+ * @link form_example_tutorial.inc the Form Example Tutorial @endlink which
+ * provide basics all the way up to multistep forms.

In this case, I think I would reword this to:

In addition, the @link form_example_tutorial.inc Form Example Tutorial @endlink provides many form examples from basic to multistep forms.

h) I think I would move the information about where to learn about $form to above the whole discussion about $form_state. And it shouldn't say "more" information about $form, because there is basically zero information about $form in this doc... Actually, maybe there should be a short paragraph just before the $form_state section, something like this:

The $form argument to form-related functions is a structured array containing the elements and properties of the form. For information on the array components and format, and more detailed explanations of the Form API workflow, see the @link http://api.drupal.org/api/file/developer/topics/forms_api_reference.html Forms API reference @endlink and the @link http://drupal.org/node/37775 Form API section of the handbook. @endlink

(I don't personally think that referring people to the quick start guide is useful -- and didn't that move out to the Handbook anyway?)

This is looking good! A few minor cleanup things, and I would mark it RTBC:

a) Pre-eminent means "superior" according to dictionary.com -- maybe we should just say primary? This is near the top.

b) When I tried the link to the form API reference, it worked, but redirected to another URL:
http://api.drupal.org/api/drupal/developer--topics--forms_api_reference....
So maybe we should put that URL in there? You never know if redirects are going to continue working forever.

c) I'm also not sure about the link to form_example_tutorial.inc -- I think it needs to be stand-alone rather than inside @link/@endlink in order to make a link to a non-URL. Did you test this with the API module? I'm not sure whether it would work this way or not. Could be fine...

d) "An associative array of values which were submitted to the form." which -> that, or omit the word entirely.

e) " their decisionmaking. " two words: decision making

f) See #817190: HTML formatting problems in forms API reference - found some formatting problems on the Form API reference while checking a link. Separate issue, just mentioning here...

g) "It is the element which caused submission," which -> that [note that the following line "which" is correct, though.]

h) "forces FAPI to turn on persistent storage for the $form."... I don't think the FAPI acronym was previously used/defined anywhere, so probably this should be written out as "forces the Form API to turn on...". Also, FAPI could also refer to field API, etc., so I would prefer to avoid using the acronym entirely.

i) "persistent, so are available in the validation, submission, and the form builder function (after first submission)."
I think I would either add 'the' before submission, or remove 'the' before form builder, for smoother flow.

Here's a new patch, with the doc cleanup described above.

Excellent work and I like the initiative! This is some challenging stuff though (hence the need for this issue), so here's some feedback:

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * programmatically without any user input using the drupal_execute() function.

s/drupal_execute()/drupal_form_submit()/

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * HTML form for modules automatically. drupal_get_form() takes as arguments
+ * $form, $form_state, and any number of optional additional user-defined
+ * arguments.

No. drupal_get_form() takes $form_id plus extra args only. I'm not sure if the point of this paragraph is to explain the args of drupal_get_form(), or to explain the args of form builder and other form-related functions.

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * @link form_example_tutorial.inc the Form Example Tutorial @endlink which

What exactly is this linking to? A file in the Examples module? How can that be conveyed in the @link structure?

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * $form = drupal_get_form('some_example_form');
+ * ...
+ * function some_example_form($form, &$form_state) {
+ *   $form['submit'] = array('#type' => 'submit', '#value' => t('Submit'));
+ * }
+ * function some_example_form_validate($form, &$form_state) {
+ *   // Validation logic.
+ * }
+ * function some_example_form_submit($form, &$form_state) {
+ *   // Submission logic.
+ * }

- Can we make it clear here that forms should be prefixed by module name? For example, "mymodule_form" or "MODULE_form"? If we also want to make it clear that a module may implement multiple forms, then perhaps "MODULE_example_form".
- Can we change the submit button to have each property on its own line? I know sometimes we put short arrays on one line, but since this is being used as an example, let's stick to the canonical form.
- If you look at this patch in context, then the documentation in form.inc above where this patch starts instructs developers to use @ingroup and @see directives to link form builder, validation, and submission functions. Seems a shame to then not follow this in the first example. But, is it even possible to add mini-PHPDoc above each of these functions within the larger PHPDoc?

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * In the form builder, validation, submission, and #ajax['callback'] functions,
+ * $form_state has enormous influence on the processing of the form.
+ * Its keys are:

- Is it ok to introduce #ajax here like this without saying anything about it previously?
- Many of the $form_state keys are documented in the PHPDoc of drupal_build_form(), drupal_redirect_form(), and drupal_rebuild_form(). Do we want to mention that?
- "has enormous influence" is true, but do we want to get a little more precise here? In Drupal 6 we were pretty sloppy with what went into $form and what went into $form_state, but in Drupal 7, they're pretty clearly separated into $form (information about the form) and $form_state (information about the state of the form). Not sure yet how to explain that succinctly though, but understanding the distinction between the two would help developers tremendously. And if this is understood, then the statement "information about the state of the form influences its processing" seems too obvious to mention.

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * - 'rebuild': If the submit function sets $form_state['rebuild'] to TRUE,
+ *   submission is not completed and instead the form is rebuilt using any
+ *   information that the submit function has made available to the form builder
+ *   function via $form_state. This is commonly used for wizard-style
+ *   multistep forms, add-more buttons, and the like.

How about just linking to drupal_build_form() for this one, since that has an explanation for it?

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * - 'storage': $form_state['storage'] is not a special key, and no specific
+ *   support is provided for it in the Form API, but by convention it is
+ *   the location where application-specific data is stored for communication
+ *   between the submit, validation, and form builder functions, especially
+ *   in a multistep-style form.

I'm not sure how much of a convention this really is any more. For example, node editing forms persist information about the node being edited in $form_state['node']. I think the node form might be too complicated to use as an example here, but if we decide to, keep in mind that the details of $form_state['node'] are being changed in #735800: node form triggers form level submit functions on button level submits, without validation.

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * - 'cache': (internal) used typically with #ajax. $form_state['cache'] = TRUE
+ *   forces the Form API to turn on persistent storage for the $form.

Not really internal (at least not yet). This is a tough key to explain though (see #689084: Document $form_state['cache']), and I don't like the explanation for it in drupal_build_form(). Here's a shot:
- 'cache': The typical form workflow involves two page requests. During the first page request, a form is built and returned for the user to fill in. Then the user fills the form in and submits it, triggering a second page request in which the form must be built and processed. By default, $form and $form_state are built from scratch during each of these page requests. In some special use-cases, it is necessary or desired to persist the $form and $form_state variables from the initial page request to the one that processes the submission. A form builder function can set 'cache' to TRUE to do this. One example where this is needed is to handle AJAX submissions, so ajax_process_form() sets this for all forms that include an element with a #ajax property. Note that the persistence of $form and $form_state across successive submissions of a multistep form happens automatically regardless of the value for 'cache'.

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * - 'input': (internal) The array of values from the POST. Since the values
+ *   in $form_state['input'] are raw, unchecked POST values, these values
+ *   should not be used in most form code for security reasons.

Not internal. How about this:
- 'input': The array of values as they were submitted by the user. These are raw and unvalidated, so should not be used without a thorough understanding of security implications. In almost all cases, code should use the data in the 'values' array exclusively. The most common use of this key is for multistep forms that need to clear some of the user input when setting 'rebuild'.

+++ includes/form.inc	3 Jun 2010 17:02:06 -0000
@@ -29,21 +29,89 @@
+ * Note that all $form_state keys not listed in form_state_keys_no_cache() are
+ * persistent, so are available in the validation, submission, and form builder
+ * functions (after first submission). In addition, they're available
+ * to a #ajax['callback'] function, if any. By convention, however, user data
+ * in $form_state that is intended to be persistent (for multistep or similar
+ * forms) is placed under $form_state['storage'].

I don't think we need this. See my comments above for 'storage' and 'cache'.

Powered by Dreditor.

Typo:

+ *   support is provided for it in the Form API, but by tradition it iswas

Other than that, I think it looks good.

Oh. Also, as effulgentsia pointed out, $form_state doesn't get passed to form builder functions. So this seems to need revision:

+ * In the form builder, validation, submission, and other form functions,
+ * $form_state is the primary influence on the processing of the form and is

One other thing mentioned in #15 that I don't think has been addressed:

+ * function my_module_example_form($form, &$form_state) {
+ *   $form['submit'] = array('#type' => 'submit', '#value' => t('Submit'));
+ * }

Separate this array onto multiple lines, each property on one line, for readability.

Scratch that first comment on #18.

Looks good!

I think this looks great, but I have a couple of suggestions:

+++ includes/form.inc
@@ -29,21 +29,105 @@
+ * The primary function used with forms is drupal_get_form(), which is
+ * used for interactive forms. Forms can also be built and submitted
+ * programatically without any user input using the drupal_form_submit()
+ * function.

When I read 'interactive forms', I had to scratch my head. It became clear when reading 'programmatically'. Maybe instead of interactive, we should say 'forms presented and completed through a page'? Just a thought.

+++ includes/form.inc
@@ -29,21 +29,105 @@
+ * - 'triggering_element': (read-only) The form element that triggered
+ *   submission. This is the same as the deprecated
+ *   $form_state['clicked_button']. It is the element that caused submission,
+ *   which may or may not be a button (in the case of AJAX forms.)

It seems valuable to mention that this is handy when you have multiple buttons on the form.

+++ includes/form.inc
@@ -29,21 +29,105 @@
+ *   processes the submission. A form builder function can set 'cache' to TRUE
+ *   to do this. One example where this is needed is to handle AJAX submissions,
+ *   so ajax_process_form() sets this for all forms that include an element with
+ *   a #ajax property. Note that the persistence of $form and $form_state across

It would be good to say _why_ AJAX submission need to do this. Right now, the example doesn't really explain things.

These things aside, I want to re-iterate that this patch is a great improvement. :)

I think Dries' comments have been addressed too...

Looks great. Committed to CVS HEAD. Thanks.

Thanks! Minor follow-up.

Committed to CVS HEAD. Thanks.