Hook Docs clean up

[DrewAPicture]

We'll use this ticket as a clearing house fixes to the list of files found on make/docs: ​http://make.wordpress.org/docs/closed-hook-tickets-review/

Changes may include cleaning up any discrepancies in terms of spacing, code that was unnecessarily moved (such as in the case of filterable arrays, in-line @ tags, etc.

[DrewAPicture]

wp-comments-post.php [25249]

[DrewAPicture]

In 27144:

Fixes for inline documentation for hooks in wp-comments-post.php.

Adds missing @since versions, spacing, and language tweaks.

See #26869, #25229, [25249].

[DrewAPicture]

wp-admin/admin-footer.php [25252]

[DrewAPicture]

In 27145:

Fixes for hooks documentation in wp-admin/admin-footer.php.

Adds some new lines, some hook-line spacing, docs-specific variables, and other language tweaks.

See #26869, #25229, [25252].

[DrewAPicture]

xmlrpc.php [25281]

[DrewAPicture]

In 27146:

Fixes for hooks documentation on xmlrpc.php.

Adds spacing, changes an @see to @link, adds a docs-specific variable for a parameter.

See #26869, #25229 and [25281].

[DrewAPicture]

wp-includes/wp-db.php [25284]

[DrewAPicture]

In 27147:

Fixes for hooks documentation in wp-includes/wp-db.php.

See #26869, #25229 and [25284].

[SergeyBiryukov]

In 27192:

Correct @since value. see #26869.

[DrewAPicture]

wp-includes/nav-menu-template.php [25410]

[DrewAPicture]

In 27201:

Fixes for hooks documentation wp-includes/nav-menu-template.php.

See #26869, #25229 and [25410].

[kpdesign]

Hook docs audit recommended fixes

[DrewAPicture]

props kpdesign for most of this

[nacin]

audit patch is good.

[DrewAPicture]

In 28083:

Priority fixes for various existing hook documentation.

Props kpdesign.
See #26869

[GaryJ]

Small hook docs tweaks for wp-admin/includes/misc.php - order of tags, spacing between tags, and clearer English.

[DrewAPicture]

Clarify comment for back-compat load-* hooks.

[DrewAPicture]

Added load-back-compat.diff​ which clarifies a comment on back-compat hooks of the load-* variety.

Notes and todo items from @kpdesign's audit(s):

Missing or incomplete docs for the following hooks:

• wp_header_image_attachment_metadata
• wp_create_file_in_uploads (duplicate)
• Long description for do_mu_upgrade filter - "it is automatically fired" - what is fired?
• get_attached_media

Suggested changes by file:

wp-admin/includes/menu.php:

• Fix erroneous phpdoc tag for 3 hooks: _network_admin_menu, _user_admin_menu, _admin_menu.

wp-admin/includes/plugin-install.php:

• Add missing docs variable for plugins_api hook.

wp-admin/network/admin.php:

• Fix incorrect docs variable for redirect_network_admin_request hook (copy/pasta error).

wp-admin/options-discussion.php:

• Change docs variable for thread_comments_depth_max hook to indicate what that number represents $max_depth.

wp-admin/options-writing.php:

• Fix short description and add missing docs variable for enable_update_services_configuration hook.

wp-admin/options.php:

• Fix short description and add missing docs variable for enable_post_by_email_configuration hook.

wp-admin/user/admin.php:

• Fix short description, remove long description for redirect_user_admin_request hook.

wp-includes/class-wp-embed.php:

• Add missing @since value for embed_oembed_discover hook.

wp-includes/class-wp-xmlrpc-server.php:

• Document http_headers_useragent hook, added in r27872 (other hook docs originally added in r27730).

wp-includes/comment-template.php:

• get_comment_excerpt missing docs
• pings_open missing docs
• the_permalink (twice as duplicates)
• Add missing @since values for comments_open and get_comments_link hooks.

wp-includes/l10n.php:

• Fix duplicate hook lines for locale and plugin_locale hooks.
• Fix typo in short description for override_unload_textdomain hook.
• Add missing docs variables for override_load_textdomain and override_unload_textdomain hooks, fix alignment due to change.

[DrewAPicture]

In 28207:

Ensure the nav_menu_meta_box_object filter hook is only documented once.

See #26869.

[DrewAPicture]

In 28209:

Ensure the style_loader_tag filter hook is only documented once.

See #26869.

[DrewAPicture]

In 28210:

Ensure the submitlink_box action hook is only documented once.

See #26869.

[DrewAPicture]

In 28211:

Ensure adjacency of a duplicate hook comment for a the_category filter call.

See #26869.

[DrewAPicture]

In 28212:

Ensure all duplicate calls of the wp_create_file_in_uploads filter are labeled as such.

See #26869.

[DrewAPicture]

In 28213:

Ensure the wp_edit_nav_menu_walker filter is only documented once.

See #26869.

[DrewAPicture]

In 28214:

Ensure the wp_set_comment_status action hook is only documented once.

See #26869.

[DrewAPicture]

In 28215:

Clean up duplicate hook notations and adjacency for calls to the wp_signup_location filter.

Also adds braces missed in [25535].

See #26869.

[DrewAPicture]

$action

[DrewAPicture]

$hook

[DrewAPicture]

@nacin:

• admin_post_actions.diff​ is a proposed attempt to handle the dynamic hook currently designated as $action
• do_feed.diff​ is a proposed attempt to handle (one of several) dynamic hook(s) currently designated as $hook

[GaryJ]

Replying to DrewAPicture:

• admin_post_actions.diff​ is a proposed attempt to handle the dynamic hook currently designated as $action

You've changed behaviour there I think. The old code allowed hooks of:

• action_post (logged in, request action empty)
• action_post_nopriv (not logged in, request action empty)
• action_post_{$action} (logged in, request action populated)
• action_post_nopriv_{$action} (not logged in, request action populated)

The new code appears to allow (re-ordered to match above):

• admin_post_ (logged in, request action empty - different)
• action_post_nopriv_ (not logged in, request action empty - different)
• action_post_{$action} (logged in, request action populated)
• action_post_nopriv_{$action} (not logged in, request action populated)

Now I've written it out, it looks like it's just the underscore before an empty ${action} that needs to be handled better.

[Otto42]

For the admin_post one, I suggest something more like this:

if ( !empty($_REQUEST['action']) )
	$request_action = '_' . $_REQUEST['action'];

and then have both:

do_action( "admin_post_nopriv{$request_action}" );
do_action( "admin_post{$request_action}" );

In your if/else.

[Otto42]

$per_page

[Otto42]

manage_{$screen->id}_columns

[DrewAPicture]

Replying to Otto42:

For the admin_post one, I suggest something more like this:

if ( !empty($_REQUEST['action']) )
	$request_action = '_' . $_REQUEST['action'];

and then have both:

do_action( "admin_post_nopriv{$request_action}" );
do_action( "admin_post{$request_action}" );

In your if/else.

Yup, works for me. I was unsure about whether there would ever be a case of "admin_post_" (invalid or empty action), and whether that would actually constitute a separate, valid hook.

[DrewAPicture]

edit_{$post_type}_per_page round 2.

[DrewAPicture]

screenid.diff​ and perpage.2.diff​ look good to me. Working on a new patch for the admin_post_ actions.

[DrewAPicture]

admin_post_* actions round two.

[DrewAPicture]

In 28348:

Use interpolation instead of concatenation for the manage_{$screen->id}_columns hook name.

Props Otto42.
See #26869.

[DrewAPicture]

For anyone interested, the reason we're converting several (possibly many) dynamic hook names to use interpolation instead of concatenation is due a need for consistency with the new code reference. Concatenated hook names are tougher to grok.

[DrewAPicture]

In 28349:

Convert and rename the $action hook in wp-admin/admin-post to two dynamic hook sets.

• admin_post_nopriv{$request_action} is fired for logged-out users
• admin_post{$request_action} is fired for logged-in users

Props Otto42, DrewAPicture.
See #26869.

Edit: s/$request_action/$action

[DrewAPicture]

In the [28349] message, should be $action instead of $request_action in the hook names. Oops.

[DrewAPicture]

In 28350:

Revert [28349] in favor of retaining the single dynamic admin_post* hook.

See #26869.

[DrewAPicture]

In 28351:

Rename the $action hook in wp-admin/admin-post.php to admin_post{$action}.

Also, clarify documentation of priv vs nopriv prefixing of the hook name.

See #26869.

[DrewAPicture]

In 28352:

Minor phpDoc fixes for the got_rewrite, got_url_rewrite, and documentation_ignore_functions hooks.

Props GaryJ.
See #26869.

[GaryJ]

I'm not in favour of r28349 as it stands.

Looking at just one of them:

/** 
 * Fires the requested handler action for logged-out users. 
 * 
 * The dynamic portion of the hook name, $action, refers to the handler action. 
 * 
 * @since 2.6.0 
 */ 
do_action( "admin_post_nopriv{$action}" ); 

Imagine that is taken out of context and shown on devhub. That now looks like I can provide a handler action of foobar and I can hook into the admin_post_noprivfoobar hook. There's no mention that $action is given an underscore prefix in the hook doc, or perhaps worse, the hook name doesn't reflect that the (missing) underscore is static and always present.

I think here I'd rather see four explicit hooks that are clearly named and unambiguous, than two that are misnamed and don't appear to follow the naming consistency of other dynamic hooks.

[DrewAPicture]

Four hooks for admin_post

[DrewAPicture]

In 28375:

Fix incomplete inline documentation for the wp_header_image_attachment_metadata filter.

See #26869.

[DrewAPicture]

In 28376:

Clarify documentation for *what* gets "automatically fired" when the do_mu_upgrade filter evaluates to true in Multisite.

See #26869.

[DrewAPicture]

In 28377:

Add a complete short description to the get_attached_media filter documentation.

See #26869.

[DrewAPicture]

In 28378:

Use a proper docs-specific variable for the first parameter passed to the embed_oembed_discover filter.

See #26869.

[DrewAPicture]

In 28393:

Clarify inline documentation for back-compat load-* action hooks.

See #26869.

[DrewAPicture]

In 28394:

Properly split and document the admin_post* actions into the following four hooks:

• admin_post_nopriv – for logged-out requests lacking a supplied action
• `admin_post_nopriv_$action – for logged-out requests with a supplied action
• admin_post – for logged-in requests lacking a supplied action
• admin_post_$action – for logged-in requests with a supplied action

See [28349], [28350], [28351].
See #26869.

[DrewAPicture]

That should cover it for this ticket. Let's do new tickets for new issues.