WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 3 months ago

#44397 accepted enhancement

Argument type does not match: theme.php

Reported by: subrataemfluence Owned by: williampatton
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch
Focuses: docs Cc:

Description

In the function doc of get_theme_root_uri(), the argument types of $stylesheet_or_template and $theme_root do not match. Default values of arguments are boolean but in function doc they are stated as strings.

File name: wp-admin/theme.php
Function: get_theme_root_uri
<?php
/**
 * Retrieve URI for themes directory.
 *
 * Does not have trailing slash.
 *
 * @since 1.5.0
 *
 * @global array $wp_theme_directories
 *
 * @param string $stylesheet_or_template Optional. The stylesheet or template name of the theme.
 *                                       Default is to leverage the main theme root.
 * @param string $theme_root             Optional. The theme root for which calculations will be based, preventing
 *                                       the need for a get_raw_theme_root() call.
 * @return string Themes URI.
 */
function get_theme_root_uri( $stylesheet_or_template = false, $theme_root = false ) {
   ...
}

Can we re-write the parameter descriptions in the following way?

<?php
 * @param string | bool $stylesheet_or_template Optional. The stylesheet or template name of the theme.
 *                                       Default is to leverage the main theme root. Default value false.
 * @param string | bool $theme_root             Optional. The theme root for which calculations will be based, preventing
 *                                       the need for a get_raw_theme_root() call. Default value false.

since we are checking for boolean values of these two parameters inside the function body like:

<?php
if ( $stylesheet_or_template && ! $theme_root ) {
   $theme_root = get_raw_theme_root( $stylesheet_or_template );
}

if ( $stylesheet_or_template && $theme_root ) {
   ...
}

and on the other hand we are passing string values to theses parameters when calling this function like this:

<?php
$this->theme_root_uri = get_theme_root_uri( $this->stylesheet, $this->theme_root );

Attachments (2)

44397.diff (1.2 KB) - added by subrataemfluence 12 months ago.
44397-2.diff (1.2 KB) - added by subrataemfluence 3 months ago.
Updated patch

Download all attachments as: .zip

Change History (8)

#1 @subrataemfluence
12 months ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

#2 @mukesh27
10 months ago

  • Focuses docs added; coding-standards removed
  • Type changed from defect (bug) to enhancement

#3 @pento
5 months ago

  • Version trunk deleted

#4 @williampatton
3 months ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to williampatton
  • Status changed from new to accepted

To me this looks like a good improvement to the documentation that is better at showing how the function works at a glance. The spaces look off to me though and I don't think they fit within coding styles. There should be no spaces between the types like string|bool.

Also could we maybe go the other way around like bool|string since the Boolean is the default?

#5 @subrataemfluence
3 months ago

@williampatton Thank you for accepting the patch. I am uploading a refreshed patch with changes made according to your suggestion. Please let me know if this works.

@subrataemfluence
3 months ago

Updated patch

#6 @williampatton
3 months ago

Hey @subrataemfluence, thank you for submitting the original patch and flagging the documentation change that is needed. The newly uploaded patch suits me fine :)

I plan to try get this into 5.3 release and between then and now I will get some more eyes on it if anyone else wanted to give some input before it was added.

Note: See TracTickets for help on using tickets.