Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hidden files are not deleted #27

Closed
bluec opened this issue Jan 18, 2019 · 3 comments
Closed

Hidden files are not deleted #27

bluec opened this issue Jan 18, 2019 · 3 comments
Labels

Comments

@bluec
Copy link
Contributor

bluec commented Jan 18, 2019

Any hidden files in the specified directory are not deleted, even with the default DeleteEverything policy. In fact it's currently impossible to write a policy that does delete hidden files.

Note that hidden files in any subdirectory are deleted so this is inconsistent behaviour (see #26)

@freekmurze freekmurze added the bug label Jan 18, 2019
@freekmurze
Copy link
Member

Feel free to submit a PR with tests that fixes this

@bluec
Copy link
Contributor Author

bluec commented Jan 18, 2019

Ok I can see why this is happening but have a question because fixing it may cause people some problems when they upgrade.

First, the reason it happens is because \Illuminate\Filesystem\Filesystem allFiles() method has the following signature

    public function allFiles($directory, $hidden = false)

But DirectoryCleaner calls the method without the second parameter so hidden files are never included in the file list collections.

If we alter (fix) this behaviour then the default DeleteEverything policy will then delete hidden files. It's possible this may cause problems for people in the wild because they may be unknowingly reliant on the fact that hidden files are not being deleted.

So, if we fix it, should we also change DeleteEverything policy to actually NOT delete hidden files? Then maybe include a new policy called DeleteEverythingAndHidden? I don't like this because DeleteEverything suggests it should delete everything including hidden files!

@freekmurze
Copy link
Member

I'm going to keep this simple and threat this as a bugfix release, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants