Missing doc for a few form.inc functions, and form.inc not up to doc standards

So true! See http://api.drupal.org/api/function/theme_markup/7

Turns out there were a few functions in includes/form.inc that were missing documentation. This patch fixes all of them, at least adding a brief doc block for each. The issue with theme_markup was that there was a blank line between the doc and the function, by the way.

See http://api.drupal.org/api/drupal/includes--form.inc/7

Probably should wait until things calm down to get a patch of this scope approved/applied. Other patches to form.inc have apparently changed some functions...

It appears that theme_markup() no longer exists in Drupal 7. But there were still those two other functions in form.inc that were missing doc headers. This patch adds doc headers to them.

Looks good, minor tweak:

+++ includes/form.inc	17 Nov 2009 21:18:59 -0000
@@ -1405,7 +1405,7 @@
  * @param $form
  *   The form element whose value is being populated.
-*  @param $input
+ *  @param $input
  *   The incoming input to populate the form element. If this is FALSE,

The indentation on @param $input does not match the indentation on @param $form and @return

Powered by Dreditor.

Thanks -- must have been a tab in there instead of spaces, since it looked fine when I applied the patch.

Anyway, here's a new patch, with a reroll so it will apply and hopefully a fix of that one spot.

Duh. I was looking at a different function... And I noticed some other things. Bigger patch coming...

Here's a new patch. While I was fixing the right function, I noticed that it didn't have a blank line between the @param and @return sections, and then I noticed that a bunch of the functions in this file were not following the one-line-description-in-third-person guidelines, and that some doc headers were not wrapped at 80 characters. So I fixed up the file...

Some comments/questions, none of them major:

+++ includes/form.inc	13 Mar 2010 17:18:01 -0000
@@ -344,7 +347,7 @@
+ * Fetches a form from cache.

Since the next function is described as "Stores a form in the cache", this should probably be "Fetches a form from the cache"?

+++ includes/form.inc	13 Mar 2010 17:18:01 -0000
@@ -755,8 +759,10 @@
+ * Validates the data from the $form_state using the validate functions defined

@@ -868,9 +874,11 @@
+ * user-defined validators.

@@ -982,9 +990,10 @@
+ * Helper function: Executes validation and submission handlers.

Throughout the comments, we use "validate functions", "validator", "validation handler". Should we pick one and be consistent? Other places in core seem to use yet another form: "validation function"

+++ includes/form.inc	13 Mar 2010 17:18:01 -0000
@@ -1738,13 +1756,14 @@
+ * Helper function: determines the value for form's token value.

I've read this line three times, but I can't figure out what this function does. I can't find any place in core where it is used (deleting it doesn't even seem to break my site). http://drupal.org/node/447930 seems to indicate that this is an implementation of an undocumented hook, form_type_ELEMENT_value. http://drupal.org/node/169815 has more information in the value callback section.

I wonder if we can rephrase the description so it makes more sense stand-alone. I'm thinking of dropping the "helper function" bit (it's more of a callback or hook implementation, right?): "Value callback for a form's token element value."? Probably means updating the siblings too...

+++ includes/form.inc	13 Mar 2010 17:18:01 -0000
@@ -1805,12 +1827,18 @@
+ * Flattens an object or nested array into a depth 1 array.

into a single-dimension array?

+++ includes/form.inc	13 Mar 2010 17:18:01 -0000
@@ -2165,7 +2197,7 @@
+ * Helper function: displays month names drupal_map_assoc().

... for use with drupal_map_assoc()?

143 critical left. Go review some!

All good points. Needs update...

Regarding your comment on the "Validates..."... This function header:

/**
 * Validates user-submitted form data.
 *

is for function drupal_validate_form(). So it really is different from validation functions. But I have changed elsewhere in the patch so that the validation handlers are called validation handlers, hopefully everywhere, since I think that's our standard term.

Regarding the docblock for form_type_token_value() -- wow, I didn't know anything about that callback! I re-documented all similar functions to make them consistent, and also documented form_type_TYPE_value() in the API file.

I think I've addressed your other comments -- thanks for the review! Here's a new patch.

#13: 576082-more-cleanup.patch queued for re-testing.

Here's a new patch. Quite a few of the problems fixed by the previous patch had been fixed elsewhere...

#17: 576082-17.patch queued for re-testing.

#17: 576082-17.patch queued for re-testing.

New patch. Two more of the changes were fixed elsewhere.

Forgot status change for test bot.

Now that two of us have had a look at this patch, let's get it in before the file changes on us again...

This looks great, overall!

Small question/nitpick about this:

+ * group and each builder function should reference its validation and submit
+ * handlers using \@see. Conversely, validate and submit handlers should

It seems like you've settled on "validation handlers" as what these are called elsewhere. So shouldn't the "Conversely" thing say "validation" rather than "validate"?

Also, just throwing this out there. Although "validation handler" is more proper English, the nice thing about "validate handler" or "validate function" is that it reflects the naming convention of FORM_ID_validate(), just as "submit handler" does. And in fact, if we are going to change this, don't we need to change "submit" to "submission" everywhere as well, for consistency?

I think I'd rather just leave these as "validate", personally, for clarity's sake.

The current standard for form doc uses submission/validation. Although this standard has not been widely applied yet, it was agreed upon in an issue somewhere...
http://drupal.org/node/1354#forms

But I'm actually wondering why we even have this text in the @defgroup at all. We document standards on node/1354, not in group docblocks usually. We should probably just cut this paragraph out, and add to node/1354 a note that all form builders should be @ingroup whateveritiscalled.

Oh, we do have @ingroup forms in the sample doc header on 1354 already. I'll add a note below that highlighting why that is.

Returning to this issue.

We're doing the general cleanup on #1310084: [meta] API documentation cleanup sprint

The original subject of this issue was the theme_markup function, which has gone away in 7.x/8.x. So I'm just going to close this issue as a duplicate of #1310084: [meta] API documentation cleanup sprint