WordPress.org

Make WordPress Core

Opened 19 months ago

Closed 16 months ago

Last modified 14 months ago

#25533 closed defect (bug) (fixed)

Hook Docs (44): wp-includes/user.php

Reported by: stephenharris Owned by: DrewAPicture
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch commit
Focuses: docs Cc:

Description

Adds missing in-line documentation to wp-includes/user.php.

Attachments (6)

25533.diff (15.2 KB) - added by stephenharris 19 months ago.
Adds in-line documentation
25533.2.diff (16.6 KB) - added by stephenharris 18 months ago.
25533.3.diff (16.5 KB) - added by DrewAPicture 17 months ago.
3rd pass
25533.4.diff (16.7 KB) - added by kpdesign 17 months ago.
Fourth pass
25533.5.diff (16.6 KB) - added by DrewAPicture 16 months ago.
Final pass
25533.6.diff (819 bytes) - added by SergeyBiryukov 16 months ago.

Download all attachments as: .zip

Change History (25)

@stephenharris19 months ago

Adds in-line documentation

comment:1 @DrewAPicture19 months ago

  • Owner set to DrewAPicture
  • Status changed from new to reviewing

comment:2 @DrewAPicture19 months ago

  • Keywords needs-patch added; has-patch removed

Hi, thanks for the patch. Some notes on 25533.diff:
Overall:

  • Space out the specific filter or hook links per coding standards
  • s/Triggered/Fires
  • s/Filters/Filter

wp_authenticate hook:

  • Change "Triggered before the user is logged in." to "Fires before the user is authenticated."
  • Need a blank line after the long description
  • Parameters need descriptions with periods

secure_signon_cookie filter:

  • Comma after "User credentials" should be a period.
  • Add the boolean default to the $remember doc line. Wrap the line to align with the beginning of the parameter description.

wp_login hook:

  • Assign $user->user_login to $user_login just above the doc block. Whenever possible, try to avoid documenting variables that don't exist.
  • Parameter descriptions need periods.
  • "usersname" is misspelled

wp_authenticate_user filter:

  • The first part of the description is redundant as that information belongs in the first parameter description. Maybe just use "Filter whether the given user can be authenticated with the provided $password."
  • Wrap the long description at "filtered" and try to keep lines at ~80 character width, expanding up to ~120 if necessary
  • The $user parameter variable is missing from the @param line. The description here can also be shortened a bit, something like "WP_User object or WP_Error object if a previous callback failed authentication."

check_is_user_spammed filter:

  • Assign is_user_spammy() to $spammed

get_user_option_{$option} filter:

  • Describe what the $option portion of the dynamic hook name refers to in a long description.
  • Add the $result variable to the first parameter line.

user_search_columns filter:

  • Specify the default search columns in either the parameter description or a long description for the filter. They appear to be 'ID', 'user_login', 'user_email', 'user_url', 'user_nicename'.
  • $this refers to the current WP_User_Query instance

pre_user_query hook:

  • Remove the first sentence in the long description, and instead add ", passed by reference" to the end of the parameter description.
  • Change "It contains" to "The query contains"

found_users_query filter:

  • Assign 'SELECT FOUND_ROWS()' to $sql, add a parameter description with a period.

get_blogs_of_user filter:

  • Add $blogs variable to the parameter doc.
  • Wrap the third parameter description to align with the beginning of the description.

wp_dropdown_users filter:

  • Add $output variable to the parameter doc.

edit_{$field} filter:

  • Just mark it //duplicate_hook (it's already documented in another file)

edit_user_{$field} filter:

  • Add a proper short description
  • Lines need to be spaced out per the inline docs template
  • Use a long description to describe what the $field portion of the dynamic hook name refers to.
  • Needs parameter docs for $value and $userid
  • Remove the @see line

pre_{$field} filter:

  • //duplicate_hook

pre_user_{$field} filter:

  • Add a proper short description
  • Lines need to be spaced out per the inline docs template
  • Use a long description to describe what the $field portion of the dynamic hook name refers to.
  • Needs parameter docs for $value
  • Remove the @see line

$field filter:

  • //duplicate_hook

user_{$field} filter:

  • Add a proper short description
  • Lines need to be spaced out per the inline docs template
  • Use a long description to describe what the $field portion of the dynamic hook name refers to.
  • Needs parameter docs for $value, $user_id, and $context
  • Remove the @see line

validate_username filter:

  • Add $valid to the first parameter doc
  • Add $username to the second parameter doc

pre_user_nicename, pre_user_url, pre_user_email, pre_user_first_name, pre_user_last_name, pre_user_display_name, pre_user_description filters:

  • Parameters need a description with a period.

profile_update, user_registered, password_reset hooks:

  • Parameter descriptions need periods.

registration_errors filter:

  • Wrap the long description at "invalid"` and align new lines with the beginning of the description using spaces

comment:3 follow-up: @stephenharris19 months ago

Thanks, meant to ask about situations where a variable isn't being directly filtered (e.g. where a function call, or object property is being filtered).

Regarding wrapping lines, in general, where should I be wrapping at?

comment:4 in reply to: ↑ 3 @DrewAPicture19 months ago

Replying to stephenharris:

Thanks, meant to ask about situations where a variable isn't being directly filtered (e.g. where a function call, or object property is being filtered).

Regarding wrapping lines, in general, where should I be wrapping at?

We generally aim for about ~80 character lines whenever possible. Going a little long isn't a problem, just interrupts the flow of reading less.

comment:5 @DrewAPicture18 months ago

This still needs a new patch incorporating the suggested changes in comment:2.

comment:6 follow-up: @stephenharris18 months ago

Ah yes, apologies :/. I've implemented the above changes except for some where I need some clarification:

secure_signon_cookie filter:
Comma after "User credentials" should be a period. Add the boolean default to the $remember doc line. Wrap the line to align with the beginning of the parameter description

I may have misunderstood this, but we don't know the default value?

user_search_columns filter:
Specify the default search columns in either the parameter description or a long description for the filter. They appear to be 'ID', 'user_login', 'user_email', 'user_url', 'user_nicename'.

The default columns depend on the search term. I've noted this and listed the above columns as the possible default columns.

s/Triggered/Fires
s/Filters/Filter

To clarify you want me to change 'Triggered' to 'Fires' and 'Filters' to 'Filter'?

I'll attach the corrections I've done so far below.

@stephenharris18 months ago

comment:7 in reply to: ↑ 6 @SergeyBiryukov18 months ago

Replying to stephenharris:

secure_signon_cookie filter:
Comma after "User credentials" should be a period. Add the boolean default to the $remember doc line. Wrap the line to align with the beginning of the parameter description

I may have misunderstood this, but we don't know the default value?

Default value for remember parameter is false: tags/3.7.1/src/wp-includes/user.php#L35.

@DrewAPicture17 months ago

3rd pass

comment:8 @DrewAPicture17 months ago

  • Keywords has-patch added; needs-patch removed

25533.3.diff tightens up quite a few descriptions, and has some other fixes for consistency.

comment:9 @DrewAPicture17 months ago

  • Summary changed from Hooks Docs: wp-includes/user.php to Hooks Docs (44): wp-includes/user.php

comment:10 @DrewAPicture17 months ago

  • Summary changed from Hooks Docs (44): wp-includes/user.php to Hook Docs (44): wp-includes/user.php

@kpdesign17 months ago

Fourth pass

comment:11 @kpdesign17 months ago

25533.4.diff contains formatting, description, and @since fixes. Should be pretty close with this one.

Needs a review and a recommendation.

@DrewAPicture16 months ago

Final pass

comment:12 @DrewAPicture16 months ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 3.9

25533.5.diff looks good.

comment:13 @DrewAPicture16 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 26901:

Inline documentation for hooks in wp-includes/user.php.

Props stephenharris, kpdesign.
Fixes #25533.

@SergeyBiryukov16 months ago

comment:14 follow-up: @SergeyBiryukov16 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

"Filter SELECT_FOUND_ROWS value" appears to be inaccurate: it's a query, not a value.

25533.6.diff is my take.

comment:15 in reply to: ↑ 14 @DrewAPicture16 months ago

Replying to SergeyBiryukov:

"Filter SELECT_FOUND_ROWS value" appears to be inaccurate: it's a query, not a value.

25533.6.diff is my take.

Good call. I think for some reason I viewed the filter as evaluating to the result instead of the query to get the result. Works for me.

comment:16 @SergeyBiryukov16 months ago

In 26904:

Correct 'found_users_query' filter description. see #25533.

comment:17 @SergeyBiryukov16 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

comment:18 @DrewAPicture14 months ago

  • Focuses docs added

comment:19 @DrewAPicture14 months ago

  • Component changed from Inline Docs to Users
Note: See TracTickets for help on using tickets.