Add $max_depth parameter to file_scan_directory()

Interesting idea.. this is probably an issue in all versions of Drupal, so it is best to start with the development version, where more new code will get reviewed, and backport from there.

Are any of the files being checked twice?

Patch needs work, the extra parameter needs to be second to last, since the last parameter is used internally in the function to determine it's current recursion depth. There's also a call to file_scan_directory inside the function that needs to be changed as well.

it'd be great to postpone this until after #255551: DX: Array-itize file_scan_directory()'s parameters gets committed.

The parameterizing is complete. Here's a new patch to get the ball rolling.

need to document the new parameter and add unit tests.

I have rewritten this patch, including documentation and a test.

#7: file_scan_directory_max_depth-343444-7.patch queued for re-testing.

Same patch as above for the new 8.x-dev.

+++ b/core/includes/file.incundefined
@@ -1421,15 +1421,23 @@ function file_download() {
+  // Check if this function should run.
...
+    // No? Then immediately return.

I would just have one comment above the function, and make it less conversational :) Though it does sound friendlier

+++ b/core/modules/system/lib/Drupal/system/Tests/File/ScanDirectoryTest.phpundefined
@@ -70,7 +70,7 @@ function testOptionCallback() {
+    // Grab a listing of all the Javascript files and check that they're

This should still be JavaScript.

+++ b/core/modules/system/lib/Drupal/system/Tests/File/ScanDirectoryTest.phpundefined
@@ -132,10 +132,8 @@ function testOptionRecurse() {
+   * Check that the min_depth options lets us ignore files in the starting dir.

Start with "Checks"

----

The functional changes of this make sense.

This new patch adds in Tim's review.

+++ b/core/includes/file.inc
@@ -1421,15 +1421,22 @@ function file_download() {
+ * @param number $depth

s/number/integer/

This is RTBC after adjusting that.

That said. On a second look:

+++ b/core/includes/file.inc
@@ -1421,15 +1421,22 @@ function file_download() {
+ *   - 'max_depth': Maximum depth of directories to return files form. Defaults
+ *     to unlimited.
...
+  if (isset($options['max_depth']) && $options['max_depth'] < $depth) {

+++ b/core/modules/system/lib/Drupal/system/Tests/File/ScanDirectoryTest.php
@@ -144,4 +142,14 @@ function testOptionMinDepth() {
+  function testOptionMaxDepth() {
+    // There are only CSS files in the 'css_test_files' directory. A max depth
+    // of zero (zero children down) will not show any of these.
+    $files = file_scan_directory($this->path, '/css/', array('max_depth' => 0));
+    $this->assertEqual(0, count($files), 'Maximum depth limits files found');
+  }

Can we flip the second condition, so that it becomes $depth > 'max_depth' ?

This inherently leads me to the question whether the condition shouldn't be >= or not?

Because it is not really clear for potential users of this function whether max_depth is inclusive or exclusive (unlike the min_depth argument, which is relatively clear), we should add an example to the phpDoc for the option that explains the effect on a hypothetical directory scan.

Additionally, the needs some more love, since it only verifies the special condition of 0, which of course won't ever return anything (but is still good to test, since that is a possible value, so let's keep that).

The test should additionally verify that scan that has files in depth = 0, depth = 1, as well as depth = 2, does not return files from depth = 2 if max_depth = 1.

E.g., you can scan /core/modules/node with the above parameters and you should only get files from:

/core/modules/node/*
/core/modules/node/templates/*
/core/modules/node/...

but not from:

/core/modules/node/lib/Drupal/...

Same as the patch above with the improved comments and additional tests.

+++ b/core/includes/file.inc
@@ -1421,15 +1421,23 @@ function file_download() {
+ *     to unlimited. Depth is measured based on number of children, so depth as
+ *     0 would be the given directory only.
...
 function file_scan_directory($dir, $mask, $options = array(), $depth = 0) {
+  // Check if this function should run, if not immediately return.
+  if (isset($options['max_depth']) && $depth > $options['max_depth']) {

1) This still does not clarify whether max_depth is inclusive or exclusive.

2) The particular case being described in the phpDoc is actually identical to the 'recurse' => FALSE option, so I don't think it makes sense to describe that case.

3) Given the above, I wonder whether it would even make sense to merge 'recurse' into 'max_depth'. We can potentially keep 'recurse' as a simple convenience helper option, but its only effect would be that 'max_depth' is set to 0 (zero).

+++ b/core/modules/system/lib/Drupal/system/Tests/File/ScanDirectoryTest.php
@@ -144,4 +142,23 @@ function testOptionMinDepth() {
+    // When the depth is increased to include the CSS directory this should
+    // show all 11 CSS files.
+    $files = file_scan_directory($this->path, '/css/', array('max_depth' => 1));
+    $this->assertEqual(11, count($files), 'Maximum depth limits files found - Max 1');
+
+    // Same as above going up another directory. This adds another CSS file.
+    $files = file_scan_directory($this->path . '/../', '/css/', array('max_depth' => 2));
+    $this->assertEqual(12, count($files), 'Maximum depth limits files found - Max 2');

Mmm... this will cause test failures for many other innocent patches in the queue ;)

Instead of testing the count, we should verify that the path names of the returned files are limited in the expected way; i.e., as outlined above, verifying that files up to a certain depth appear, but no files beyond that depth.

So I've added some more to the documentation, merged the recurse option and changed the tests.

Closing as outdated. Please re-open if you feel this is still relevant.