[5.5] Filesystem files() to return similar structured arrays as allFiles() #18874
Conversation
|
Tests please. |
tillkruss
changed the title
tillkruss
changed the title
| return array_filter($glob, function ($file) { | ||
| return filetype($file) == 'file'; | ||
| }); | ||
| return iterator_to_array(Finder::create()->files()->ignoreDotFiles(! $hidden)->in($directory)->depth(0), false); |
There was a problem hiding this comment.
This is hard to read. Can you break it into several lines?
| @@ -372,20 +372,9 @@ public function glob($pattern, $flags = 0) | |||
| * @param string $directory | |||
| * @return array | |||
| */ | |||
| public function files($directory) | |||
| public function files($directory, $hidden = false) | |||
There was a problem hiding this comment.
This should be called $ignoreDotFiles to make it clear.
|
@tillkruss Added tests to verify that both files() and allFiles() return SplFileInfo objects. No change to allFiles() and it passes the test I added. I mostly copied the function allFiles() which is why I had it written in that manner. Let me know if that was what you intended or if I need to break it out more. I also changed the allFiles() $hidden parameter to match the $ignoreDotFiles variable name...should be the same in both (which is why I had it as $hidden originally). |
|
Also, thanks for the feedback...I also updated the Changelog. |
|
@ahuggins: Nice! |
| @@ -53,6 +53,9 @@ | |||
| ### Events | |||
| - ⚠️ Removed calling queue method on handlers ([0360cb1](https://github.com/laravel/framework/commit/0360cb1c6b71ec89d406517b19d1508511e98fb5), [ec96979](https://github.com/laravel/framework/commit/ec969797878f2c731034455af2397110732d14c4), [d9be4bf](https://github.com/laravel/framework/commit/d9be4bfe0367a8e07eed4931bdabf135292abb1b)) | |||
|
|
|||
| ### Filesystem | |||
| - ⚠️ Made `Storage::files()` work like `Storage::allFiles()` ([#18874](https://github.com/laravel/framework/pull/18874)) | |||
|
|
|||
I did some work using the Filesystem the other day, and quickly started with the allFiles method, then realized that I needed only the files in the given directory. So I switched the call from allFiles to files, ran my test and something broke. Digging into it, I found that both files and allFiles return an array, but allFiles returns an array of SplFileInfo objects, where files returns an array of strings.
This seemed a little weird, and I thought it might be convenient if both functions had similar return array values. If you parse the string from files, then I think switching to allFiles will still work because SplFileInfo has __toString() implemented. But if you use any method on the SplFileInfo object that allFiles returns, then switching to files from allFiles will break. Which is why this change is being proposed.
Here is a gist that shows the difference in the arrays: https://gist.github.com/ahuggins/21d1af3c1e274cfa305d8220fb5303f2
A side benefit of this is that the api of the files and allFiles methods would be the same...how allFiles handles hidden files, would be the same as files with this PR.
** Was closed against 5.4, so submitting to master (5.5)