Clean up API docs for field_ui module

Do you plan to patch this module? If so can you assign the issue to yourself and also add your name to the summary issue? Thanks!

Since there hasn't been any word from Lars, I claim this one for now.

Patch attached.

I opened #1347888: @param / @return docs missing in much of Field UI module, as there are @param/@return missing almost everywhere (I assigned that issue to myself, but won't roll a patch until this patch here is in --- as conflicts are virtually guaranteed).

I had to get creative, as many of the paths of page callbacks and form constructors vary by entity --- and this is not handled via %-placeholders. So I went with the PLACEHOLDER_IN_CAPS convention. Downside: The meaning of the placeholder has to be explained in every function. Alternative ideas welcome.

Test failed because of #1346772: StatisticsLoggingTestCase->testLogging() fails with clean URLs in some environments. I requeued it.

Looks pretty good! A few things to fix:

a)

 /**
- * Pre-render callback for field_ui_table elements.
+ * Pre-render callback: Performs pre-render tasks on field_ui_table elements.

field_ui_table should have () after it I think, to make a link to this function (or should be changed to the actual function name if that is not correct)?

Other places need () too, such as:

+ * Form validation handler for field_ui_field_overview_form.

b)

+ * Path: BUNDLE_ADMIN_PATH/fields
+ *
+ * where BUNDLE_ADMIN_PATH is the path stored in the ['admin']['info'] property
+ * in the return value of hook_entity_info().

Hmmm... You had asked about whether this was a good structure... I think it's OK, except I guess I would move the "where BUNDLE..." up onto the same line as Path:. It shouldn't be a new paragraph? In the case of the ones where the paths are a list, I would just remove the blank line between the paths list and the "where..." sentence.

c) Typo (spelling of "row"):

+ * Validates the 'add existing field' roew of field_ui_field_overview_form().

d) Not following standards:

 /**
- * Form submit handler for multistep buttons on the 'Manage display' screen.
+ * Form submission handler for multistep buttons on the 'Manage display' screen.

Should include the form function name rather than "on the manage display screen". Probably the following doc block for the Ajax handler should too?

e) Typo (not your fault) field -> fields

+ * Returns an array of existing field to be added to a bundle.

f) Should follow the form submission template:

 /**
- * Save a field's settings after editing.
+ * Saves a field's settings after editing.
+ *
+ * @see field_ui_field_settings_form()
  */
 function field_ui_field_settings_form_submit($form, &$form_state) {

This one too:

 /**
  * Form submit handler for field instance settings form.

g)

+ *
+ * @return
+ *   The instance array.
  */
 function field_ui_menu_load($field_name, $entity_type, $bundle_name, $bundle_pos, $map) {

Normally we don't need return value docs in menu callbacks... but I could be convinced that it is OK here. Maybe it should say "the field instance array" rather than "the instance array" though?

h) Missing period at the end:

+ * Title callback: Returns the name of a given instance

i) Duplicated word "formatter":

   /**
-   * Test formatter formatter settings.
+   * Tests formatter formatter settings.

Arg, I should have caught at least some of these things.

Addressed all of the, one way or another. Comments:

(a) 'field_ui_table' is an element #type. I went with the format we agreed on in #1347890: Clean up API docs for file module, and changed the summary to "Render API callback: Performs pre-render tasks on field_ui_table elements." and then included a line in the docblock saying "This function is assigned as a #pre_render callback in field_ui_element_info()."

Made the parallel change for field_ui_field_edit_instance_pre_render() for consistency.

(g) I really think it makes sense to document the return value here, as this is a menu loader function---and these do not have a common return value. Rather, it varies by loader what they return. I did change the return to to "The field instance array." and also added a @ingroup field (though perhaps an @see or a link would be better?). The docgroup page of the field group is where the keys of instance arrays are documented.

This is looking really good! I only found this:

a)

+ * Populates display settings for a new  view mode from the default view mode.

(extra space between new and view).

b)

+ * Proves common functionality for the Field UI test classes.
  */
 class FieldUITestCase extends DrupalWebTestCase {

Proves -> Provides

c)

+   *   Admin path of the bunde that the field is to be attached to.
 

bunde -> bundle

Since these were all simple typos, I just edited the patch file directly and re-uploaded it (after verifying I only made those three changes). Unless the testbot objects, I think we can call this done!

Looks good to me. Committed/pushed to 8.x.

Attached removes the menu callback changes, plus a hunk for field_ui_field_edit_form_delete_submit(), which does not exist in D7.

These all look like good changes to get into D7. Thanks!

My personal favorite piece of this patch:

-  // Pick the elements that need ro receive the ajax-new-content effect.
+  // Pick the elements that need to receive the ajax-new-content effect.

ro ro ro your boat!

Bot goof.

Committed and pushed "ro" 7.x! ;) Thanks.

The only thing I found in glancing through was:

+/**
+ * @file
+ *   Right-to-left specific stylesheet for the Field UI module.
+ */

That extra indentation shouldn't be there, so I removed it, both in D7 and D8.

Hmmm. Are you saying that CSS files should not have @file comments at the top?

It looks like right now some do and some don't. My opinion is that all files should have them... thoughts?

There's nothing specific on
http://drupal.org/node/302199
about @file blocks, but it does basically say to follow the PHP block doc standards when documenting sections of your CSS file...

Should we open a separate issue to discuss that?

I think webchick simply meant that

+/**
+ * @file
+ *   Right-to-left specific stylesheet for the Field UI module.
+ */

should have been

+/**
+ * @file
+ * Right-to-left specific stylesheet for the Field UI module.
+ */

And not that there should not be any @file header.

Yep, that's right.

Duh. I can't read. :)