Hook Docs: wp-login.php

[ShinichiN]

Adding inline documentation to wp-login.php

[ShinichiN]

I'm making another patch file. I'm sorry for bothering...

wp-login.diff above was from wp 3.6.1 and was old.

[ShinichiN]

wp-login-2.diff is a svn diff file.

I didn't know how to research the WP version of the do_action( 'login_form_' . $action ).

action hooks

login_enqueue_scripts
login_head
login_footer
lostpassword_post
retrieve_password
retrieve_password_key
login_init
'login_form_' . $action
lost_password
lostpassword_form
validate_password_reset
register_form
login_footer
login_form

filter hooks

shake_error_codes
login_headerurl
login_headertitle
login_body_class
login_message
login_errors
login_messages
allow_password_reset
retrieve_password_title
retrieve_password_message
post_password_expires
lostpassword_redirect
register
wp_signup_location
registration_redirect
login_redirect
wp_login_errors

[kpdesign]

Second pass

[DrewAPicture]

wp-login-3.diff​ is getting there. Unfortunately, there's still some work to do yet. Here are my notes:

• "password retrieving process." should be "password retrieval process."
• In my opinion, // Misspelled and deprecated should be in a doc block even if the action is deprecated.
• allow_password_reset filter: "Filter password reset restriction boolean." is pretty vague. Can you better clarify what is filterable here? Also, "True as default." should be a short description of what the parameter is. It's fine to append the default to the end, but we should know what it signifies
• "Fires before inserting the new md5 key into database." should be clearer. What md5 key are we talking about? The activation key?
• login_init: what does "the main action of wp-login.php" refer to exactly?
• lostpassword_redirect: I suggest splitting the follow out to a variable first, then using the variable in the filter and documenting it, respectively: !empty( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : ''. Perhaps $lostpassword_redirect or something
• lostpassword_form: The short description, "Fires in the lostpassword form to add extra hidden input.", makes an assumption about what specific type of action will happen here. Maybe something like this instead? "Fires in the lostpassword form before the hidden fields."
• register: I suggest splitting the following out to a variable above the filter: printf( '<a href="%s">%s</a>', esc_url( wp_registration_url() ), __( 'Register' ) )
• register_form: "form to add extra input." should be plural: "form to add extra inputs."
• login_redirect: Split the following out to a variable above the filter, same as lostpassword_redirect: isset( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : ''.

If you have questions about any of the suggestions I listed or need help, let me know. Happy to help :)

[ShinichiN]

I'm glad to reveive your review, thank you!

I'm going to work on this tonight, japan time:)

[ShinichiN]

I modified wp-login.php following DrewAPicture's review.

[ShinichiN]

New patch including the Changeset r25619

[ShinichiN]

wp-login-5.diff includes

• patch wp-login-3.diff
• DrewAPicture's review
• helen's patch r25619

Thank you!

[DrewAPicture]

Iterating on .5

[DrewAPicture]

Final

[DrewAPicture]

In 25701:

Inline documentation for hooks in wp-login.php.

Props ShinichiN, kpdesign.
Fixes #25393.

[tivnet]

Replying to DrewAPicture:

This one is wrong

* @param int  $user_data->ID The ID of the user attempting to reset a password.

because it re-declares $user_data as int (->ID is ignored)

[DrewAPicture]

Replying to tivnet:

Replying to DrewAPicture:

This one is wrong

* @param int  $user_data->ID The ID of the user attempting to reset a password.

because it re-declares $user_data as int (->ID is ignored)

Not sure I'm following your logic. The docblock redeclares $user_data as int or code redeclares it as int somewhere I'm not seeing? If it's the former, we could always store in variable before the filter is evaluated.

[tivnet]

Replying to DrewAPicture:

The docblock applies @param int to the $user_data for the code following it.

In general, this use of docblocks is illegal, according to

​http://www.phpdoc.org/docs/latest/for-users/phpdoc/basic-syntax.html#which-elements-can-have-a-docblock

So, why not to keep is as just comments, without docblock @-isms? Using @s will only disturb phpdocumentor, IMO.

[DrewAPicture]

Replying to tivnet:

Replying to DrewAPicture:

The docblock applies @param int to the $user_data for the code following it.

In general, this use of docblocks is illegal, according to

​http://www.phpdoc.org/docs/latest/for-users/phpdoc/basic-syntax.html#which-elements-can-have-a-docblock

So, why not to keep is as just comments, without docblock @-isms? Using @s will only disturb phpdocumentor, IMO.

We use a ​customized documentation schema that draws inspiration from phpdoc but doesn't follow it to a tee. A notable difference, for instance, is that phpdoc doesn't provide any support for an action and filter-driven architecture, such as WordPress is.

The style of documentation you see here for actions and filters is consistent with, and adheres to, the standards outlined in the document linked above.

If you'd like to join the discussion, the inline-docs team meet weekly on Wednesdays in #wordpress-sfd on Freenode, at 18:00 UTC.

[tivnet]

Replying to DrewAPicture:

Do what you want, but remember that this makes only trouble for those who use good IDEs (which you have listed in the Standards page). As I said, non-standard use of docblocks breaks the code validation.

[DrewAPicture]

Replying to tivnet:

Replying to DrewAPicture:

Do what you want, but remember that this makes only trouble for those who use good IDEs (which you have listed in the Standards page). As I said, non-standard use of docblocks breaks the code validation.

FWIW, I use PhpStorm and I haven't had a single problem.

Again, if you have concerns, you're more than welcome to voice them at the weekly chat. Having this discussion in one of a multitude of hook docs tickets isn't really going to be the most effective way to do that. I appreciate the input though :)

[tivnet]

Replying to DrewAPicture:

Sorry, I won't be able to join the chat this Wed...

Some of the phpStorm inspections for wp-login.php:

• PHPDoc comment does not match function or method signature (at line 25)
• Field 'ID' not found in class (at line 325)

• Method 'update' not found in class (at line 351) - this one because there is no /** @var wpdb $wpdb */

Now, this one concerns me even more:
@param bool $is_admin in vars.php
There is no such var here. This is all wrong...

[tmtoy]

Replying to tivnet:

Now, this one concerns me even more:
@param bool $is_admin in vars.php
There is no such var here. This is all wrong...

Excuse me for cutting in...
This issue has been resolved already.
​http://core.trac.wordpress.org/changeset/25718

[tivnet]

Replying to tmtoy:
Yes, saw that yesterday, thanks