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

Fix glob for nested directories #80

Merged
merged 3 commits into from Apr 1, 2019
Merged

Fix glob for nested directories #80

merged 3 commits into from Apr 1, 2019

Conversation

medusalix
Copy link
Contributor

Makes it possible to use the option glob with nested directories through sorting the filepaths by their depth before passing them to the trash function.

Fixes #75

@sindresorhus
Copy link
Owner

This way works fine, but I had another idea in mind. If we have ['foo', 'foo/bar', 'foo/baz'], couldn't we just remove foo/bar and foo/baz? As foo will remove those anyway. I think this would help speed up the deletion process too and reduce the situations where users encounter sindresorhus/trash-cli#9.

@medusalix
Copy link
Contributor Author

@sindresorhus Your proposed solution is actually slower the way I have implemented it 😄. I traverse the whole array for each item to check if there's already a directory above it.

To be honest, I think the operation system is much quicker at moving files to the trash than JS is at de-duplicating an array.

@sindresorhus
Copy link
Owner

Your proposed solution is actually slower the way I have implemented it

How was that measured? The difference is probably negligible for macOS, but for Linux we actually remove each file using JS, so I think my solution would be more efficient then.

To be honest, I think the operation system is much quicker at moving files to the trash than JS is at de-duplicating an array.

I highly doubt that.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@sindresorhus sindresorhus merged commit 78e705c into sindresorhus:master Apr 1, 2019
@medusalix medusalix deleted the fix-nested-dirs branch April 1, 2019 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants