-
Notifications
You must be signed in to change notification settings - Fork 775
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
Create dest parent after applying filter in copy #517
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea here, however, the implementation is flawed.
Before your changes, startCopy
simply checked the filter and started the copy. Now, startCopy
does pathExists
/mkdirp
work as well. However, startCopy
is called when recursively copying items in subdirectories. In those cases, the destination exists logic has already been done, so this could have a major perf hit since we're double-doing the destination checking.
That's true. Let me see what I can do here! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; please squash to one commit.
Sure thing. |
8ca1074
to
199ec9f
Compare
Squash done! |
When applying filter, I think this is a little bit better than before although it still highly depends on the user's filter conditions!
But at least, if users choose their filter conditions intelligently, this way,
copy()
correctly doesn't create dest parent directory since its child path fails on filter.This is until @RyanZim adds his changes for filter function.