Make WordPress Core

Opened 9 years ago

Last modified 11 hours ago

#6531 assigned defect (bug)

Recursively search for files in theme and plugin editors

Reported by: torbens Owned by: chsxf
Milestone: 4.9 Priority: high
Severity: minor Version: 2.5
Component: Themes Keywords: theme-editor needs-patch
Focuses: administration Cc:


Themes (like Subtle: http://gluedideas.com/downloads/subtle/) might contain numerous CSS files. The theme editor, however, does not recognize any other CSS files other than style.css.

The files might be located some levels deeper in sub directories like '/wp-content/themes/glued-ideas-subtle-01/assets/css/print.css'.

Attachments (8)

patch-recursive-editors.diff (10.5 KB) - added by chsxf 6 years ago.
24050.diff (2.4 KB) - added by MikeHansenMe 4 years ago.
patch from 24050 that was a duplicate of this ticket.
24050.2.diff (2.4 KB) - added by MikeHansenMe 4 years ago.
should fix the bug mentioned in comment 19
6531.diff (3.6 KB) - added by MikeHansenMe 4 years ago.
Show path for plugins and themes
6531.2.diff (3.7 KB) - added by MikeHansenMe 4 years ago.
Does not show if description and relative file are the same
6531.3.diff (7.6 KB) - added by valendesigns 3 years ago.
6531.4.diff (7.6 KB) - added by valendesigns 3 years ago.
6531-theme.diff (2.2 KB) - added by WraithKenny 2 weeks ago.
Adds filters for theme editor

Download all attachments as: .zip

Change History (50)

#1 @lloydbudd
9 years ago

  • Milestone changed from 2.5.1 to 2.6

#2 @mrmist
9 years ago

  • Keywords needs-patch added

Could be lumped in with #6632 maybe. No point in having 2 generic editors. Not sure how common buried stylesheets are.

#3 @mrmist
9 years ago

See also #4131. This is possibly a dupe.

#4 @Denis-de-Bernardy
8 years ago

  • Type changed from defect (bug) to enhancement

It's not common, but it occurs. The patch from #6632 partially sorts this if the files are in a direct subfolder. If you've a file any deeper, it won't do any good. example:


#5 @Denis-de-Bernardy
8 years ago

  • Summary changed from CSS files are not shown in Theme Editor to Recursively search for files in theme and plugin editors

the same may also apply to plugins.

#6 @Denis-de-Bernardy
8 years ago

  • Milestone changed from 2.9 to Future Release

#7 @chsxf
6 years ago

  • Cc christophe@… added
  • Keywords has-patch needs-testing added; needs-patch removed
  • Owner changed from anonymous to chsxf
  • Status changed from new to assigned

Patch is attached.

It includes :

  • recursive search for files in both Theme and Plugin editors
  • sorting of files by path and name
  • inclusion of JavaScript files in the Theme editor

#8 @hm2k
6 years ago

  • Cc hm2k added
  • Type changed from enhancement to defect (bug)

It seem it will already recursively search for PHP files.

Is there any logical reason why you would not display all of the theme's CSS files from subdirectories in the theme editor?

Changing to a bug because expected behaviour is that all CSS files in the theme are shown.

Last edited 6 years ago by hm2k (previous) (diff)

#9 @chsxf
6 years ago

The patch i've submitted two months ago brings this fonctionnality for CSS, JS and PHP files.

#10 @hm2k
6 years ago

Is there any reason why you would NOT want to apply this patch?

Is there any reason you would NOT want this feature?

This bug has been open for 4 years, is there any reason why this bug cannot be closed?


#11 @WraithKenny
6 years ago

  • Cc Ken@… added

#12 @prionkor
5 years ago

  • Type changed from defect (bug) to enhancement

I would like to see this on future release. Perhaps the directory structure with js/jq implement.

#14 @SergeyBiryukov
5 years ago

#23500 was marked as a duplicate.

#16 @SergeyBiryukov
4 years ago

#24050 was marked as a duplicate.

4 years ago

patch from 24050 that was a duplicate of this ticket.

#17 @Daedalon
4 years ago

  • Cc daedalon@… added
  • Type changed from enhancement to defect (bug)

Thanks Mike for the patch! Verified, works. Also improves the coding style to match http://make.wordpress.org/core/handbook/coding-standards/php/.

I'd like to see this included in 3.6 after two improvements:

  1. Use curly braces with all ifs. Avoids hassle and bugs in later edits. A decent percentage of bugs are caused by editing code like this:
} else {
	if ( plugin_basename("$dir/$file") != $plugin )
		$plugin_files[] = plugin_basename("$dir/$file");
  1. Theme editor could use the same functionality.

Marking ticket as defect (bug) instead of enhancement. More appropriate for the file list not displaying all files of a plugin or theme. Milestone, anyone?

#18 @Daedalon
4 years ago

  1. In order to improve performance, instead of calling this function twice inside a loop:

Split it into these parts, the first of which is to be called only once in the beginning of the function:

$plugin_basedir = plugin_basename($dir);

The patch in #24049 also applies this change.

#19 @Daedalon
4 years ago

Encountered a bug:

  1. After opening a file from within a subdirectory, the Plugin Files lists only files and directories under that subdirectory.

4 years ago

should fix the bug mentioned in comment 19

#20 @MikeHansenMe
4 years ago

  • Cc mdhansen@… added

#21 @MikeHansenMe
4 years ago

Turns out the bug in comment 19 affects core now. Should it be a separate ticket, even though 24050.2.diff fixes it? Also I still think there needs to be some ui improvements for the long list.

Last edited 4 years ago by MikeHansenMe (previous) (diff)

#22 @a.hoereth
4 years ago

  • Cc a.hoereth@… added

#23 @MikeHansenMe
4 years ago

  • Keywords needs-ui added

I think if we want to get this into core we need to get a UI that supports a long list of files and file tree structure.

#24 @helen
4 years ago

  • Keywords needs-patch added; has-patch needs-testing needs-ui removed

Let's not let this ticket get hung up on UI - see #24048 for that instead.

As for this, what if we go with displaying the path for some clarity in themes, I guess the way plugins do now? And could we get a unified patch addressing all of the above? Can't apply the existing ones together, but one only deals with plugins :)

Last edited 4 years ago by helen (previous) (diff)

#25 @Daedalon
4 years ago

Agreed with Helen. It's crucial to be able to see all the files in subfolders, and secondary to prettify it. Making all of the files visible is also pretty much a prerequisite for making the list neat. The sooner we display the full list in any ugly way, the sooner we can start to work on the chrome and polish for it.

Regarding Mike's excellent patches above, my opinion is that if there's one patch that fixes all of them for both plugins and themes, that'd be just perfect.

4 years ago

Show path for plugins and themes

#26 @MikeHansenMe
4 years ago

  • Keywords has-patch added; needs-patch removed

4 years ago

Does not show if description and relative file are the same

#27 @nacin
4 years ago

  • Component changed from Template to Themes
  • Focuses administration added

3 years ago

#28 @valendesigns
3 years ago

  • Keywords dev-feedback added

The 6531.3.diff patch includes unit tests to prove that get_plugin_files will now return paths beyond two directory levels deep. This new patch subtly updates the UI for both plugins and themes. The previous patch didn't change the UI for plugins, but did for themes. I've updated the code to improve performance according to comment #18. I also solved an issue where the select dropdown was not showing the correct selected plugin when editing a file that wasn't the main plugin file.

C'mon Guys. Let's get this ticket closed! It's been open for 7 years.

3 years ago

#29 @valendesigns
3 years ago

I guess I didn't test the theme code thoroughly enough from the previous patch. When I was showing someone how to apply and create a patch I noticed a bug when viewing the tree with a non default theme. Some of the files had empty strings in the nonessential span element which meant you just saw "()" under the file without text inside. I've fixed it in the latest patch.

Last edited 3 years ago by valendesigns (previous) (diff)

#30 @obenland
3 years ago

  • Keywords theme-editor added

#31 follow-up: @wonderboymusic
2 years ago

  • Keywords needs-patch added; has-patch dev-feedback removed

we need a better solution for recursion here

#32 @DrewAPicture
2 years ago

#24050 was marked as a duplicate.

#33 @Maikuolan
14 months ago

Any news on this?

#34 @WraithKenny
7 months ago

The patches on the ticket address the scanning of plugin files. For Themes, it's easier, and if we can't just bump up the depth (for whatever reason), maybe we could put filters and let themes "grant access" to its own deeper files.

foreach ( $file_types as $type ) {
	switch ( $type ) {
		case 'php':
			$allowed_files += $theme->get_files( 'php', apply_filters('wp_theme_scandir_depth', 1, $type, $stylesheet ) );
			$has_templates = ! empty( $allowed_files );
		case 'css':
			$style_files = $theme->get_files( 'css', apply_filters('wp_theme_scandir_depth', 0, $type, $stylesheet ) );
			$allowed_files['style.css'] = $style_files['style.css'];
			$allowed_files += $style_files;
			$allowed_files += $theme->get_files( $type, apply_filters('wp_theme_scandir_depth', 0, $type, $stylesheet ) );

This would at least allow themes (TwentyEighteen, for example) to do

add_filter( 'wp_theme_scandir_depth', function( $depth, $type, $theme ) {
	if ('twentyeighteen' == $theme && 'css' == $type)
		return 2;
	return $depth;
},10,3 );

This would be useful for plugins that enhance the theme editor, such that they no longer have to completely rewrite the entire theme-editor.php file. (example: https://github.com/unFocus/Scripts-n-Styles/blob/17e3c1b522ef7aa349b417041f12ccd4853f80e6/dist/includes/theme-editor-page.php)

I've also rewrote the entire plugin-editor.php to use the patch on this ticket https://github.com/unFocus/Scripts-n-Styles/blob/17e3c1b522ef7aa349b417041f12ccd4853f80e6/dist/includes/plugin-editor-page.php (Currently in 4.0.0-alpha, not in the public release.)

Last edited 11 hours ago by WraithKenny (previous) (diff)

This ticket was mentioned in Slack in #core by paaljoachim. View the logs.

7 weeks ago

#36 @SergeyBiryukov
4 weeks ago

#38470 was marked as a duplicate.

#37 @melchoyce
4 weeks ago

  • Milestone changed from Future Release to 4.9

2 weeks ago

Adds filters for theme editor

#38 in reply to: ↑ 31 @westonruter
2 weeks ago

Replying to wonderboymusic:

we need a better solution for recursion here

@wonderboymusic Do you have specific feedback on what needs improvement? Is it that recursion in general is bad or the current implementation in the patch needs revision?

I'm looking for other cases of recursive directory traversal in core and I see:

  • list_files(), which isn't even used in core. (⁉️)
  • recurse_dirsize() used in get_dirsize() which is used in get_space_used() which are then used in wp_dashboard_quota()). Results are stored in transient.
  • wpmu_delete_blog() which is recursing directories via iteration.
  • WP_Theme::scandir() which is used in WP_Theme::get_files() and currently limits depths by file type. This one seems it should be refactored to get the full list of files and then split them up by type, rather than do multiple directory iterations for each type.

One thing to consider as well is to add the list of theme/plugin files to a transient, with the theme/plugin version in the cache key, so that the recursion doesn't have to be done every single time the editor is loaded.

It seems that get_plugin_files() should be using list_files(), since the former is using nested loops rather than proper recursion.

I think it would be better to go the route of using a recursive directory iterator rather than add filters to extend the depth on a type-by-type basis.

#39 @netweb
2 weeks ago

#38470 was marked as a duplicate.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.

9 days ago

#41 @schlessera
9 days ago

I'm interested in tackling this for 4.9.

I'd like to investigate using proper PHP iterator objects, and provide some custom magic so that they can transparently cache into transients/object cache.

The main benefit is that you don't pull in all files and folders into memory just to pass them around, but rather you prepare an iterator object and pass that on. In some cases, the actual iteration will never happen as it is not needed, and in some cases you'll only end up with a partial iteration, which saves IO and memory as well.

Once the iterators are built, they can easily be adapted to replace the above functions.

Is there anything that would speak against such an approach?

#42 @westonruter
2 days ago

  • Priority changed from normal to high

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

Note: See TracTickets for help on using tickets.