WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#25588 closed defect (bug) (fixed)

Hook Docs: wp-includes/theme.php

Reported by: ShinichiN Owned by: DrewAPicture
Milestone: 3.8 Priority: normal
Severity: normal Version:
Component: Inline Docs Keywords: has-patch commit
Focuses: Cc:

Description

I added inline documentations to all the filter and action hooks in wp-includes/theme.php

Attachments (3)

25588.diff (9.1 KB) - added by ShinichiN 7 years ago.
25588-2.diff (9.5 KB) - added by ShinichiN 7 years ago.
'after_switch_theme' action fires multiple times and has different parameter(s) depending on the context. I didn't know if I should document about the parameter "$stylesheet" for the second one. In the patch, I wrote it in the long description field of the first one.
25588.2.diff (11.2 KB) - added by DrewAPicture 7 years ago.
3rd pass

Download all attachments as: .zip

Change History (11)

@ShinichiN
7 years ago

#1 @DrewAPicture
7 years ago

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

#2 follow-up: @DrewAPicture
7 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to 3.8
  • Type changed from enhancement to defect (bug)

Hey, thanks for the patch. This is a really great start. Here are some notes on 25588.diff:
Overall:

  • Replace 'Filters' with 'Filter'
  • We can probably remove the abundant use of "The " at the beginning of most parameter descriptions. For instance, "The stylesheet directory URI" can become "Stylesheet directory URI"
  • Space out the specific action or filter lines per coding standards
  • Several parameter types are missing

template_directory_uri filter:

  • Having the example URL for $theme_root_url is nice, but it might fit better in a long description for the filter

wp_cache_themes_persistently filter:

  • Documentation for the second parameter is missing

theme_root_uri filter:

  • Typo in the short description

switch_theme filter:

  • The $new_theme parameter should have a type of WP_Theme

validate_current_theme filter:

  • "Filter whether to validate the current theme."
  • There's no need to store the boolean value in a variable first. Just use a doc-specific variable in the parameter documentation. (if you're confused, we changed our approach to this a couple of weeks ago)
  • Specify the default boolean value for the parameter at the end of the description. Ex: Default true.

theme_mod_$name filter:

  • Use a docs-specific variable in the parameter docs for the parameter. Maybe $current_mod
  • Use a long description to explain what the dynamic portion of the hook, $name, refers to

The second use of the theme_mod_$name filter should be marked a duplicate as follows:

/** This filter is documented in wp-includes/theme.php */

current_theme_supports-$feature filter:

  • Maybe change the short description to something like "Filter whether the theme supports a specific feature
  • Use a long description to explain what the dynamic portion of the hook, $feature, refers to

after_switch_theme action:

  • "Fires on the first WP load after a theme switch."
  • Since this action is fired multiple times and the second one will be marked as a duplicate, use a long description to describe the different contexts the action is used in, e.g. whether the old theme exists.

The second use of the after_switch_theme action should be marked a duplicate as follows:

/** This action is documented in wp-includes/theme.php */

#3 @SergeyBiryukov
7 years ago

stylesheet, template, theme_root, switch_theme, and some other filters were added in 1.5.0, not in 1.5.2. Minor releases generally don't contain new hooks.

Looks like the database on http://adambrown.info/p/wp_hooks doesn't include 1.5.0 for some reason.

Last edited 7 years ago by SergeyBiryukov (previous) (diff)

#4 @ShinichiN
7 years ago

Thank you for the reviews!

I'm working on it now.

@ShinichiN
7 years ago

'after_switch_theme' action fires multiple times and has different parameter(s) depending on the context. I didn't know if I should document about the parameter "$stylesheet" for the second one. In the patch, I wrote it in the long description field of the first one.

#5 in reply to: ↑ 2 @ShinichiN
7 years ago

Replying to DrewAPicture:

(if you're confused, we changed our approach to this a couple of weeks ago)

Thank you for the notification.
Where can I read the discussion about this? Should I resend patches for the old ones which I've sent for the WP version 3.7?

#6 @ShinichiN
7 years ago

  • Keywords has-patch added; needs-patch removed

@DrewAPicture
7 years ago

3rd pass

#7 @DrewAPicture
7 years ago

  • Keywords commit added

25588.2.diff is a comprehensive third pass. Notably, it minimizes changed code wherever possible. Also adds a bunch of missing parameter types, and fixes some descriptions.

#8 @DrewAPicture
7 years ago

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

In 26501:

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

Props ShinichiN.
Fixes #25588.

Note: See TracTickets for help on using tickets.