DX: Array-itize file_scan_directory()'s parameters

love the idea. subscribing.

better title.

I'd probably be best to wait until #74645: modify file_scan_directory to include a regex for the nomask. gets committed.

YES! This function could really use this!

#74645: modify file_scan_directory to include a regex for the nomask. got committed (and wrecked havoc along the way) so lets get this patch rolling again.

here's a start on it. one of the system.module tests is failing but i can't quite track down why. i haven't tried running the installer yet... odds are that it'll break that horribly.

re-ran the tests and it looked good. ran through the installer and it worked fine.

we should probably add some more tests to file.test for file_scan_directory(). i'll set to needs review so the test bot can take a swing at it.

Really nice. Since HEAD was broken due to this function twice in one week, would be nice to have tests alongside the patch.

catch, not arguing that more tests would be good but just want to correct the record: the problems weren't with the function itself but the calling of it in install.php--which at this point is untestable.

started some more unit tests and in the process found a bug in the PHPDocs. it suggested that "path", "basename", and "name" are returned when in fact it's really "filename", "basename" and "name".

more tests would be good but this is all i've got time for right now.

Reroll.

Hmm, on my box, all File scan directory tests pass. Not sure how to debug this. The failing node test is due to #345632: SimpleTest: correct assertTitle logic.

testbotbug last night, marking for retesting.

I started to review this and then realized it still needed a couple of unit tests to cover all of file_scan_directory()'s options. I wrapped those up so this should be good to go.

Hopefully we can get some reviews and then get this marked RTBC?

Short review of code comments:

Typos (funciton, paramter, decends, curre):

+ * Each time the funciton is called the file is stored in a static variable.
+   * Check that key paramter sets the return value's key.
+   * Check that the recurse option decends into subdirectories.
+    $this->assertTrue(empty($files), t("Min-depth of 1 successfully excludes files from curre."));

JS and javascript should be "JavaScript":

+    // Grab a listing of all the JS files and check that they're passed to the
+    $this->assertEqual(2, count($all_files), t('Found two, expected javascript files.'));
+    // Grab a listing of all the JS files and check that they're passed to the
     // Grab a listing of all the JS files.
     $this->assertEqual(2, count($all_files), t('Found two, expected javascript files.'));
+    $this->assertEqual(2, count($files), t('With recursion we found the expected javascript files.'));

Don't abbreviate things like "min":

+   * Check that the min depth lets us ignore files in the starting directory.
+    $this->assertTrue(empty($files), t("Min-depth of 1 successfully excludes files from curre."));

keith.smith thanks for the review... i tend to get sloppy with the unit tests. i've corrected the issues you raised.

i think that was a test bot fail.

no clue why that's failing. re-rolled.

Got no idea why this is failing. Works fine on my machine. Dumbing down the tests a bit.

The testbot failures may occur, because the tests included in the patch expect file_scan_directory() to return the files in alphabetical order, but no particular sorting is guaranteed. I have added some sort() calls.

I also changed the filename masks in the tests from /javascript*/ to /^javascript-/. While not being incorrect, the former gives the impression that file_scan_directory() accepts regular filesystem wildcards instead of regular expressions.

great, i think the regex changes are a good idea.

well now that the test bot's happy i'd say this is RTBC.

Added a note to the upgrade docs: http://drupal.org/node/224333#file_scan_directory_array-itize

The upgrade doc links to #125030: Javascript: Allow compatibility with other libraries. instead of this issue. drewish, will you fix that (I don't have access to edit that page)? At the same time, perhaps you will add the upgrade note for #380064: DX: Make file_scan_directory() use save property names as file_load() too - thanks :-)

i've actually tried to add it a couple of times but had the changes disappear... think there's some problems with the book.module post d6 upgrade.

I changed the link yesterday (I somehow got permission to edit the page), and the change seems to stick.