WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#20546 closed defect (bug) (fixed)

WP_Theme cleanups: get_files, scandir, is_child_theme, multiple screenshot support

Reported by: nacin Owned by:
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: Themes Keywords:
Focuses: Cc:

Description

After looking closely at the WP_Theme API in the weeks that have followed #20103, implementing it in a few areas, and seeing how it settled, I have a few final changes to make to the public API.

Remove the is_child_theme() method

is_child_theme() simply checks if template !== stylesheet. However, it is possible for it to be a child theme under this definition, but to not have a valid parent. This means code like this: if ( $theme->is_child_theme() ) echo $theme->parent()->display('Name'); can potentially fatal error. I made this mistake at least once, which doesn't bode well for others.

Thus, we can eliminate is_child_theme() as a direct counterpart to the global function. That is fine, as you still have $this->parent() to check, a method that came later in my development and definitely makes more sense. Otherwise you have two very, very similar methods, and mixing them up can cause fatal errors in edge cases. Yuck.

Remove multiple screenshot support

api.wordpress.org and WP_Theme are both ready for multiple screenshots, but they aren't displayed anywhere in the UI. We should remove get_screenshots() and get_screenshot_count() until or if we decide to implement them in the UI.

Generalize the get_files method

get_files() was originally designed to cache and return a theme's CSS and PHP files. This gets complicated quickly due to child/parent and depth differences. get_themes() looked one directory deep for PHP files in the parent theme, top-level only for CSS and PHP files in a child theme, and avoided looking for CSS files in the parent theme all together. This is pretty inconsistent, something I tried to standardize, especially given our needs with page templates (we want all PHP files from a consistent depth in both themes) and the theme editor (we no longer show inherited files from a parent theme).

get_files() also has an odd return value, in that it can return an array of 'css' and 'php' keys, with an array of matching files as the values. I wrote this when I hadn't yet ported things over to the theme editor. It seems unnecessary. Likewise, caching here seems unnecessary, and makes things inflexible. Right now, a plugin would need to implement their own method to scan a theme for, say, .png or .js files, and that's lame.

So. get_files() can become a public function with the following definition:

get_files( $type = null, $depth = 0, $search_parent = false )

By default, return all files, flat search, of that theme only (don't include the parent).

Core would use it like so:

// Theme editor:
$theme->get_files( 'php', 1 );
$theme->get_files( 'css' );
// back compat for 'Template Files' and 'Stylesheet Files'
get_files( 'php', 1, true );
get_files( 'css' );
// getting page templates (avoids parent files, we get those separately, so they can both be cached)
$this->get_files( 'php', 1 );

The $search_parent arg is purely for plugins to use.

Change History (7)

comment:1 Otto422 years ago

  • Cc Otto42 added

comment:2 nacin2 years ago

In [20586]:

Documentation and visibility cleanups in WP_Theme.

  • Declare __toString() as public.
  • Use parent() and error() internally, rather than the properties.
  • Add and correct inline documentation throughout.
  • Attempt to clarify that display() is superior to get().

see #20546, see #20103.

comment:3 DH-Shredder2 years ago

  • Cc mike.schroder@… added

comment:4 nacin2 years ago

In [20588]:

Make WP_Theme::get_files() a general method for retrieving any file types, at any depth, optionally to include parent files. see #20546.

comment:5 nacin2 years ago

In [20589]:

Remove WP_Theme::is_child_theme() in favor of WP_Theme::parent(). see #20546.

comment:6 sabreuse2 years ago

  • Cc sabreuse@… added

comment:7 nacin2 years ago

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

[20590]

Remove support from WP_Theme for multiple screenshots until we bring it to the UI. fixes #20546.

Note: See TracTickets for help on using tickets.