Clean up API docs for contact module

Thanks -- mostly looks OK - a couple of things to fix:

a)

 /**
- * Categories/list tab.
+ * Page callback: List contact categories.

List -> Lists

b)

+ * Form submit handler for contact_category_delete_form().

submit -> submission -- see http://drupal.org/node/1354#forms

c)

 + * Page page callbacks for the contact module.

Page - word is repeated.

d)

+ * Tests site-wide contact form.
...
+   * Tests configuration options and site-wide contact form.

site-wide --> the site-wide
Other places that could use "the":

+   * Submits contact form.

e)
<code>
+   * Gets list category ids.

ids -> IDs. "id" is a psychological term. "ID" is short for identifier/identification.

f)

+   * Fills out a user's personal contact form and submit.

submit -> submits

This still could use a few more "the"s:
+ * Form constructor for category edit form.
+ * Form constructor for contact category deletion form.
+ * Tests site-wide contact form.
+ * Tests configuration options and site-wide contact form.

And maybe a few less too:
+ * Enables the use of personal and the site-wide contact forms.
... This one should either have two "the" or no "the". I think no "the" is probably better, since forms is plural.

One other thing I didn't notice last review:
+ * Page callbacks for the contact module.
Refer to modules either as "the Contact module" or "contact.module" (no the)

And this @return needs a newline in the middle:

-   * @return array Category ids.
+   * @return array Category IDs.

Should probably be:

@return array
   A list of the category IDs.

Other than that, looking pretty close...

I reviewed #4 and checked the module with the patch applied, and I found a few things. Many have to do with the new menu callback standard.

1. +++ b/core/modules/contact/contact.admin.incundefined
   @@ -2,11 +2,13 @@
    /**
   - * Categories/list tab.
   + * Page callback: Lists contact categories.
   + *
   + * Path: admin/structure/contact
     */
   

   I think we want an @see to the hook_menu() implementation that registers this path and callback, correct?

2. +++ b/core/modules/contact/contact.testundefined
   @@ -267,9 +270,10 @@ class ContactSitewideTestCase extends DrupalWebTestCase {
   +   * @return array
   +   *   A list of the Category IDs.
   

   Minor point but I'd de-capitalize "Category" here. It's not a title in this instance, just a noun. So: "A list of the category IDs." Edit: Looks like @jhodgdon mentioned that same point as well.

3. The docblock for _contact_personal_tab_access() probably needs to be adjusted to the menu callback standard: http://drupal.org/node/1354#menu-callback. There might be others as well.
4. Maybe we want form alters to have @see references to relevant form constructors? E.g. contact_form_user_admin_settings_alter().
5. contact.test has some @param that are malformatted. Not sure if we should clean that up here or not.
6. For form constructors that are also menu callbacks, we've been doing a hybrid-y thing for the docblocks. (See the latest patch in #1313980: Clean up API docs for node module.) In most cases the form constructor is actually a callback for drupal_get_form(), which in turn is the page callback. What do you think?

Thanks!

I replied to #1337282: Lacking doc header standard for forms used as page callbacks. I think you're right about that.

+++ b/core/modules/contact/contact.admin.incundefined
@@ -188,9 +207,7 @@ function contact_category_delete_form($form, &$form_state, array $contact) {
 function contact_category_delete_form_submit($form, &$form_state) {

+++ b/core/modules/contact/contact.pages.incundefined
@@ -2,14 +2,15 @@
 /**
- * Form builder; the site-wide contact form.
+ * Form constructor for the site-wide contact form.
  *
  * @see contact_site_form_validate()
  * @see contact_site_form_submit()
+ * @ingroup forms
  */
 function contact_site_form($form, &$form_state) {

@@ -166,10 +171,11 @@ function contact_site_form_submit($form, &$form_state) {
 /**
- * Form builder; the personal contact form.
+ * Form constructor for the personal contact form.
  *
  * @see contact_personal_form_validate()
  * @see contact_personal_form_submit()
+ * @ingroup forms

Should we add the menu paths and @see to contact_menu() for these, then?

I suppose we'd better postpone this until we arrive at a standard...

Thanks @sven.lauer. I noticed a few more things when I read through it this time. (I just read the patch; I didn't apply it.)

+++ b/core/modules/contact/contact.admin.incundefined
@@ -53,7 +57,20 @@ function contact_category_list() {
+ *   - admin/structure/contact/add
+ *   - admin/structure/contact/edit/%contact

I think this has extra indentation? The hyphens should line up with the previous line. Reference: http://drupal.org/node/1354#lists

+++ b/core/modules/contact/contact.admin.incundefined
@@ -53,7 +57,20 @@ function contact_category_list() {
+ * @param $category
+ *   An array describing the category to be edited. May be empty for new
+ *   categories.

@@ -167,8 +188,15 @@ function contact_category_edit_form_submit($form, &$form_state) {
+ * @param $contact
+ *   Array describing the contact category to be deleted.

Maybe we should define allowed keys here? First few lines of the function show what they are. They get used as default values for the form elements. Also, the second should probably have an article.

I checked contact_category_edit_form_submit() to see if there was an API function we should reference, but it just runs DB queries directly rather than using any API functions. Maybe we should open a followup issue to decouple the submit handler from the save functionality.

+++ b/core/modules/contact/contact.testundefined
@@ -1,10 +1,13 @@
  * Tests for contact.module.

This should be "Tests for the Contact module," correct?

+++ b/core/modules/contact/contact.testundefined
@@ -200,12 +203,16 @@ class ContactSitewideTestCase extends DrupalWebTestCase {
+   * @param string $category
+   *   Name of category.
+   * @param string $recipients
+   *   List of recipient e-mail addresses.
+   * @param string $reply
+   *   Auto-reply text.
+   * @param boolean $selected
+   *   Defautly selected.

@@ -217,12 +224,16 @@ class ContactSitewideTestCase extends DrupalWebTestCase {
+   * @param string $category
+   *   Name of category.
+   * @param string $recipients
+   *   List of recipient e-mail addresses.
+   * @param string $reply
+   *   Auto-reply text.
+   * @param boolean $selected
+   *   Defautly selected.

@@ -236,13 +247,18 @@ class ContactSitewideTestCase extends DrupalWebTestCase {
+   * @param string $name
+   *   Name.
+   * @param string $mail
+   *   E-mail address.
+   * @param string $subject
+   *   Subject.
+   * @param integer $cid
+   *   Category id.
+   * @param string $message
+   *   Message.

We could probably fix these up a little while we're reformatting them. E.g., "The category name," "The category ID," etc., and maybe add a bit more information about how each parameter is used. "Defaultly selected" is especially creative; perhaps "A boolean indicating whether the category should be selected by default," or something. (I didn't check the method to see what it actually does.)

+++ b/core/modules/contact/contact.testundefined
@@ -306,7 +323,7 @@ class ContactPersonalTestCase extends DrupalWebTestCase {
+   * Tests personal contact form access.

Maybe "Tests access to the personal contact form" for less ambiguity? Debatable, I guess.

Yeah, I guess it's kinda borderline to fix them, but since the patch is a manageable size and we're changing the lines anyway it seems worthwhile to me. The new text is far more useful.

Couple typos:

+++ b/core/modules/contact/contact.admin.incundefined
@@ -167,8 +195,16 @@ function contact_category_edit_form_submit($form, &$form_state) {
+ *

Looks like an extra blank line here? Edit: Whoops, meant to select more context than that. I'll add another comment with the whole hunk.

+++ b/core/modules/contact/contact.testundefined
@@ -236,13 +248,18 @@ class ContactSitewideTestCase extends DrupalWebTestCase {
+   *   The-mail address of the sender.

Typo here, unless The-mail is a new mail service to replace all others. :)

+++ b/core/modules/contact/contact.admin.incundefined
@@ -167,8 +195,16 @@ function contact_category_edit_form_submit($form, &$form_state) {
 /**
- * Form builder for deleting a contact category.
+ * Form constructor for the contact category deletion form.
+ *
+ *
+ * Path: admin/structure/contact/delete/%contact
  *

The extra blank line.

9 days to next Drupal core point release.

+++ b/core/modules/contact/contact.pages.incundefined
@@ -2,14 +2,18 @@
- * User page callbacks for the contact module.
+ * Page callbacks for the contact module.

Contact module is lowercased here. Sorry, not sure how I missed that in #15.

status update per previous comment

I found two more small things:

+++ b/core/modules/contact/contact.admin.incundefined
@@ -53,7 +57,27 @@ function contact_category_list() {
+ *   - weight: Thw weight of the category.

Typo here ("thw").

+++ b/core/modules/contact/contact.testundefined
@@ -236,13 +248,18 @@ class ContactSitewideTestCase extends DrupalWebTestCase {
+   *   The E-mail address of the sender.

I don't think "E-mail" should be capitalized here. (Elsewhere it's lowercased.)

Alright, everything looks great to me now.

Committed to 8.x. Thanks for the continuous efforts on making our documentation better.

With new menu standard stuff removed for the backport.

This all looks good for d7, thanks!

Committed and pushed to 7.x. Thanks!

For reference, I created #1811628: Further clean up of API docs for Contact module to resolve some further documentation issues.