Make WordPress Core

Opened 17 years ago

Closed 7 years ago

Last modified 7 years ago

#6531 closed defect (bug) (fixed)

Recursively search for files in theme and plugin editors

Reported by: torbens's profile torbens Owned by: pento's profile pento
Milestone: 4.9 Priority: high
Severity: minor Version: 2.5
Component: Themes Keywords: theme-editor has-patch has-unit-tests
Focuses: administration Cc:

Description

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 (19)

patch-recursive-editors.diff (10.5 KB) - added by chsxf 13 years ago.
24050.diff (2.4 KB) - added by MikeHansenMe 12 years ago.
patch from 24050 that was a duplicate of this ticket.
24050.2.diff (2.4 KB) - added by MikeHansenMe 12 years ago.
should fix the bug mentioned in comment 19
6531.diff (3.6 KB) - added by MikeHansenMe 11 years ago.
Show path for plugins and themes
6531.2.diff (3.7 KB) - added by MikeHansenMe 11 years ago.
Does not show if description and relative file are the same
6531.3.diff (7.6 KB) - added by valendesigns 10 years ago.
6531.4.diff (7.6 KB) - added by valendesigns 10 years ago.
6531-theme.diff (2.2 KB) - added by WraithKenny 7 years ago.
Adds filters for theme editor
6531.5a.diff (3.4 KB) - added by WraithKenny 7 years ago.
Rewrites get_plugin_files() to use list_files, add filter to list_files (based on WP_Theme::scandir() ) and adds transient cache (based on recurse_dirsize() )
6531.5a.2.diff (2.5 KB) - added by WraithKenny 7 years ago.
same as last, but cleaned up and using plugin version in caching
6531.5a.2.2.diff (2.6 KB) - added by WraithKenny 7 years ago.
6531.5b.diff (1.5 KB) - added by WraithKenny 7 years ago.
Changes for WP_Theme::get_files(), caches all files, and reduces returns based on $type and $depth
6531.6.diff (4.9 KB) - added by WraithKenny 7 years ago.
full patch
6531.7.diff (6.5 KB) - added by westonruter 7 years ago.
Δ https://github.com/xwp/wordpress-develop/pull/271/commits/f80a849
6531.8.diff (7.8 KB) - added by WraithKenny 7 years ago.
Same as previous, but with extra excludes
6531.9.diff (7.8 KB) - added by WraithKenny 7 years ago.
6531.10.diff (13.8 KB) - added by schlessera 7 years ago.
Iterator implementation.
6531.10.a.diff (8.2 KB) - added by WraithKenny 7 years ago.
alternative
6531.11.diff (11.0 KB) - added by schlessera 7 years ago.
Recursive approach + tests

Download all attachments as: .zip

Change History (92)

#1 @lloydbudd
17 years ago

  • Milestone changed from 2.5.1 to 2.6

#2 @mrmist
16 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
16 years ago

See also #4131. This is possibly a dupe.

#4 @Denis-de-Bernardy
16 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:

wp-content/themes/sem-reloaded/skins/copywriter-blue/skin.css

#5 @Denis-de-Bernardy
16 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
15 years ago

  • Milestone changed from 2.9 to Future Release

#7 @chsxf
13 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
13 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 13 years ago by hm2k (previous) (diff)

#9 @chsxf
13 years ago

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

#10 @hm2k
13 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?

Thanks!

#11 @WraithKenny
13 years ago

  • Cc Ken@… added

#12 @prionkor
12 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
12 years ago

#23500 was marked as a duplicate.

#16 @SergeyBiryukov
12 years ago

#24050 was marked as a duplicate.

@MikeHansenMe
12 years ago

patch from 24050 that was a duplicate of this ticket.

#17 @Daedalon
12 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
12 years ago

  1. In order to improve performance, instead of calling this function twice inside a loop:
    plugin_basename("$dir/$file")
    

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);
...
"$plugin_basedir/$file"

The patch in #24049 also applies this change.

#19 @Daedalon
12 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.

@MikeHansenMe
12 years ago

should fix the bug mentioned in comment 19

#20 @MikeHansenMe
11 years ago

  • Cc mdhansen@… added

#21 @MikeHansenMe
11 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 11 years ago by MikeHansenMe (previous) (diff)

#22 @a.hoereth
11 years ago

  • Cc a.hoereth@… added

#23 @MikeHansenMe
11 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
11 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 11 years ago by helen (previous) (diff)

#25 @Daedalon
11 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.

@MikeHansenMe
11 years ago

Show path for plugins and themes

#26 @MikeHansenMe
11 years ago

  • Keywords has-patch added; needs-patch removed

@MikeHansenMe
11 years ago

Does not show if description and relative file are the same

#27 @nacin
11 years ago

  • Component changed from Template to Themes
  • Focuses administration added

@valendesigns
10 years ago

#28 @valendesigns
10 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.

@valendesigns
10 years ago

#29 @valendesigns
10 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 10 years ago by valendesigns (previous) (diff)

#30 @obenland
10 years ago

  • Keywords theme-editor added

#31 follow-up: @wonderboymusic
9 years ago

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

we need a better solution for recursion here

#32 @DrewAPicture
9 years ago

#24050 was marked as a duplicate.

#33 @Maikuolan
8 years ago

Any news on this?

#34 @WraithKenny
8 years 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 );
			break;
		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;
			break;
		default:
			$allowed_files += $theme->get_files( $type, apply_filters('wp_theme_scandir_depth', 0, $type, $stylesheet ) );
			break;
	}
}

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 7 years ago by WraithKenny (previous) (diff)

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


7 years ago

#36 @SergeyBiryukov
7 years ago

#38470 was marked as a duplicate.

#37 @melchoyce
7 years ago

  • Milestone changed from Future Release to 4.9

@WraithKenny
7 years ago

Adds filters for theme editor

#38 in reply to: ↑ 31 @westonruter
7 years 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
7 years ago

#38470 was marked as a duplicate.

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


7 years ago

#41 @schlessera
7 years 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
7 years 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.

@WraithKenny
7 years ago

Rewrites get_plugin_files() to use list_files, add filter to list_files (based on WP_Theme::scandir() ) and adds transient cache (based on recurse_dirsize() )

@WraithKenny
7 years ago

same as last, but cleaned up and using plugin version in caching

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 years ago

@WraithKenny
7 years ago

Changes for WP_Theme::get_files(), caches all files, and reduces returns based on $type and $depth

@WraithKenny
7 years ago

full patch

This ticket was mentioned in Slack in #accessibility by schlessera. View the logs.


7 years ago

#45 @afercia
7 years ago

Note: this is needed for #41729

#46 @WraithKenny
7 years ago

  • Keywords has-patch added; needs-patch removed

#47 @WraithKenny
7 years ago

If anyone has feedback on the patch, I can update it. Just let me know.

#48 @melchoyce
7 years ago

  • Owner changed from chsxf to westonruter
  • Status changed from assigned to reviewing

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


7 years ago

#50 @schlessera
7 years ago

@WraithKenny Here are some observations regarding the patch:

  • list_files() uses recursion, which can put a lot of strain on the system or even lead to failures in extreme cases.
  • The caching mechanism will fail if the result of the traversal is indeed an empty set (like if the only plugin was excluded). This will disable the caching and iterate over the files every single time.
  • The caching mechanism is prone to cache stampedes on high load servers, where the cache might be refreshed multiple times in parallel.

I would recommend returning PHP recursive iterators instead of using a recursive function and collecting the entire result set, and building a more fail-safe caching mechanism.

Using PHP iterators has the following advantages:

  • Depending on the use case, traversal can stop early.
  • Iterators can still be modified (i.e. further filtered) after they have been returned.
  • Depending on the PHP version support, they can even be better optimized to transparently support parallel crawling or other neat tricks.

#51 @westonruter
7 years ago

  • Keywords needs-unit-tests added

I've applied some fixes to PHP code style which were identified by PHPCS. See 6531.7.diff (and delta).

In addition to @schlessera's great feedback, one thing that became clear to me in testing the patch is that we should absolutely ignore certain directories from being traversed. I'm thinking node_modules, vendor, and bower_components to start. The list of files can get so incredibly long that it just becomes impossible to navigate. I don't think we can safely throw this on users if we don't also tackle #41729.

While this is out of scope for this ticket, the latency for recursing over all the files is compounded by the fact that every time the user saves a file, the result is a full page reload when there really is no need for that. In reality the new file's contents should get submitted over Ajax (er, eventually maybe REST API) and in that way the user wouldn't be continually pinging the server to re-generate the list of files every single time, and making the user have to re-locate their scroll position after saving. There is crossover here as well with #39766 and #39766 where we could avoid the whole need to detect when an PHP error happens and then when the plugin editor page reloads to throw in an iframe to try scraping for the error message again. The error message can simply be returned in the Ajax response and then shown to the user immediately. /cc @melchoyce

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

#52 @westonruter
7 years ago

  • Owner changed from westonruter to WraithKenny

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


7 years ago

@WraithKenny
7 years ago

Same as previous, but with extra excludes

@WraithKenny
7 years ago

#54 @WraithKenny
7 years ago

Here is a rundown of the current patch

  • list_files() now contains a filter called 'list_files_exclusions' that will ignore hidden files and folders and folders named 'CVS', 'node_modules', 'vendor', or 'bower_components' when scanning for files. ('CVS' because it existed in 'theme_scandir_exclusions'.)
  • WP_Theme::scandir() filter 'theme_scandir_exclusions' updated to exclude 'vendor', or 'bower_components' also.
  • get_plugin_files now uses list_files()
  • get_files now does scandir only once for all files at all depths, and filters it's return per call.
  • get_plugin_files now caches file lists returned from list_files() into a transient for an hour, keyed to directory and Version.
  • WP_Theme::get_files() caches file lists returned from scandir() into a transient for an hour, keyed to theme Name and Version.
  • theme-editor.php calls to $theme->get_files() are passed with -1 to get all files of the type.

Patch 6531.9.diff is to address some of @schlessera's comments, particularly the "empty set" condition. I'm not sure I fully understand the implications however, since an empty set would cause the function to scan for 0 files when the theme editor is loaded. I updated the patch to check for false === $all_files which should make sure that it won't run more than once per hour. As for the "stampedes" issue, I don't fully understand, but that'd be a wider issue for transients and caching in general rather than this ticket, right?

As for PHP recursive iterators, I'm not too familiar with them, but I'll research after work to see what I can do, unless you'd like to take a stab at it :) Thanks for the feedback!

@westonruter thanks for the link to wpcs/phpcs; I've got it installed now.

I've got the folders you've suggested filtered out now (and devs can use the filter to adjust to their liking, if they really really want to opt into including 'vendor' or whatever after #24048 folders are added).

It also looks like you've already solved a lot of the issues in the second paragraph over in #24048! Awesome job there!

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

#55 @WraithKenny
7 years ago

@schlessera and @westonruter I could add back in the filters from 6531-theme.diff and add a similar one (or better named one) onto the list_files() call in get_plugin_files() so that sites concerned could put the depth back to 1 if they have issues. (it'd be no worse of a load then currently, right?) Thoughts?

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


7 years ago

#57 follow-up: @schlessera
7 years ago

@WraithKenny :

particularly the "empty set" condition. I'm not sure I fully understand the implications however, since an empty set would cause the function to scan for 0 files when the theme editor is loaded.

The main issue was that caching would have been disabled as a side effect, thus fetching from cache every single time, failing every single time, and then starting to look through the filesystem every single time.

With the check for false you did now, the cache will be checked only once, and it will correctly cache the fact that it didn't find any files.

As for PHP recursive iterators, I'm not too familiar with them, but I'll research after work to see what I can do, unless you'd like to take a stab at it :) Thanks for the feedback!

I will see if I can manage to fit in that change before the deadline...

Additionally:

  • I'd suggest having a way to define/override the exclusions for list_files() through the function call, and putting the filters outside of that "reusable" function. I don't think the filter conceptually fits into the list_files() implementation.
  • The transient name needs to be truncated to control the length, to avoid running into obscure bugs where a long directory name breaks the caching.

#58 in reply to: ↑ 57 @WraithKenny
7 years ago

I can work on the 2 "additionally"

@schlessera
7 years ago

Iterator implementation.

#59 @schlessera
7 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Starting from @WraithKenny 's 6531.9.diff patch, I did the following changes:

  • I switched the list_files() implementation from a recursive function to an iterator-based approach.
  • The max. $level argument has been deprecated, as it doesn't make any sense with iterators. I kept it in the signature though to stay safe wrt BC. If we know this is not used, we can remove it from the signature to clean up the code.
  • The function returns an iterator object, not an array. This allows consuming code to add further filtering, while working as usual with foreach() loops.
  • The iterator returns SplFileInfo objects. You can retrieve the path or the filename individually from these. Casting them to a string will result in a filename that includes the absolute path.
  • I moved the exclusions filter from list_info() to get_plugin_files(). As a result, the filter was renamed to get_plugin_files_exclusions.
  • I added the possibility to match a regex. This helps further filtering files.
  • I added the possibility to choose whether to skip hidden files or not.
  • I truncated the transient label and added an md5 to keep it unique.
  • I added basic unit tests for the list_files() function.

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


7 years ago

#61 @WraithKenny
7 years ago

I'm getting an error testing the patch:

<?php
Fatal error: Uncaught Error: Trying to clone an uncloneable object of class RecursiveIteratorIterator in /var/www/html/build/wp-includes/option.php:452 Stack trace: #0 /var/www/html/build/wp-includes/option.php(766): add_option('_transient_list...', Object(RecursiveIteratorIterator), '', 'no') #1 /var/www/html/build/wp-admin/includes/plugin.php(227): set_transient('list_files_cach...', Object(RecursiveIteratorIterator), 3600) #2 /var/www/html/build/wp-admin/plugin-editor.php(69): get_plugin_files('gutenberg/guten...') #3 {main} thrown in /var/www/html/build/wp-includes/option.php on line 452

@WraithKenny
7 years ago

alternative

#62 @WraithKenny
7 years ago

I'm trying to debug 6531.10.diff to see if it's my dev setup or something, but in the meantime, I had the patch I was working on 6531.10.a.diff which is like .9 but with the filter for list_files moved out, and better transient keys.

#63 @schlessera
7 years ago

I tried several permutations now to transform the iterator into an array to stay safe and not try to serialize the actual iterator object, but the RecursiveFilterIterator seems buggy in its PHP 5.2 incarnation.

Normally, with PHP 5.4+, you'd use a CallbackFilterIterator, but with PHP 5.2, you have to create a custom class instead, which will need a second constructor argument to inject the options. And whatever combinations I have tried, the conversion to an array always created new instances of that filter and omitted the second argument to the constructor.

Without the filter, it is not possible to filter folders, only files, so it would not be possible to exclude folders such a node_modules. This is a show-stopper for using iterators in this case.

I suggest going with the recursive function for now, and re-investigate using iterators when WordPress Core uses PHP 5.4 as a minimum (hopefully next year, then ;) ).

@schlessera
7 years ago

Recursive approach + tests

#64 @schlessera
7 years ago

I added another patch. It takes @WraithKenny 's 6531.10.a.diff patch and adds the following changes:

  • Transient key is truncated and made unique with an MD5
  • Fixes an issue with double slashes in the generated paths
  • Renames the filter in get_plugin_files() from list_files_exclusions to get_plugin_files_exclusions to bring it in line with the theme specific filter
  • Makes sure single files are always producing the correct path by wrapping them into plugin_basename()
  • Adds unit test for get_plugin_files() when testing against a plugin with subfolders
  • Adds unit test for list_files() to return arbitrary files
  • Adds unit test for list_files() to support exclusions
Version 0, edited 7 years ago by schlessera (next)

#65 @WraithKenny
7 years ago

The latest patch looks really solid at this point. Tested on php 5.6 and 7.1

#66 @WraithKenny
7 years ago

#39316 was marked as a duplicate.

This ticket was mentioned in Slack in #accessibility by wraithkenny. View the logs.


7 years ago

#68 @pento
7 years ago

  • Owner changed from WraithKenny to pento

#69 @pento
7 years ago

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

In 41806:

File Editor: Add support for more than one sub-directory level.

The theme and plugin editors now list all files in the selected theme or plugin, recursing through subdirectories as necessary.

Props WraithKenny, schlessera, chsxf, MikeHansenMe, Daedalon, valendesigns, westonruter, pento.
Fixes #6531.

#70 @pento
7 years ago

A few notes on [41086], changed from 6531.11.diff:

  • Changed the get_plugin_files_exclusions filter name to plugin_files_exclusions. Though it's slightly different from the function name, it more accurately reflects what it's for. (It's for filtering the plugin files exclusions, not for filtering whether to get the plugin files exclusions.)
  • In get_plugin_files(), swap the $plugin_files += $list_files line for an array_merge(), because the former didn't work properly in PHP 5.2/5.3 ($list_files had different keys in different PHP versions). Similarly, add an array_values() call on the next line, to normalise the array keys.
  • In the listFiles.php tests, use assertContains() and assertNotContains(), and test for index.php instead of about.php.
  • A handful for inline docs changes.

#71 @westonruter
7 years ago

In 42112:

Theme Editor: Ensure files listed recursively can be both viewed and edited.

Prevent edits to 2-level deep theme files from returning a disallowed_theme_file error when attempting to save an edit. Aligns logic for gathering $allowed_files in theme-editor.php for listing files with the validation logic in wp_edit_theme_plugin_file().

Amends [41806].
See #6531.
Fixes #42425.

#72 @dd32
7 years ago

In 42242:

Theme/Plugin Editor: Remove the caching added in [41806] as it causes more problems than it fixes.

While caching here seemed like a good idea in theory, in practice the cache would be often stale causing development issues.
We exclude common folders (such as node_modules) from the scanning to avoid directories which are not useful to the end-user, so as long as those exclusion lists are held up this shouldn't cause too much of a degredation in the future.
We may consider adding caching here again in the future if it's determined that it is really needed.

Props precies, ibenic, mariovalney, schlessera, and all the others who commented on the ticket(s).
This partually reverts [41806].
See #6531.
Fixes #42573.

#73 @dd32
7 years ago

In 42243:

Theme/Plugin Editor: Remove the caching added in [41806] as it causes more problems than it fixes.

While caching here seemed like a good idea in theory, in practice the cache would be often stale causing development issues.
We exclude common folders (such as node_modules) from the scanning to avoid directories which are not useful to the end-user, so as long as those exclusion lists are held up this shouldn't cause too much of a degredation in the future.
We may consider adding caching here again in the future if it's determined that it is really needed.

Props precies, ibenic, mariovalney, schlessera, and all the others who commented on the ticket(s).
This partually reverts [41806].
Merges [42242] to the 4.9 branch.
See #6531.
Fixes #42573 for 4.9.

Note: See TracTickets for help on using tickets.