-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
RFC: Recursive cp #10448
RFC: Recursive cp #10448
Conversation
for p in readdir(src) | ||
cp(joinpath(src, p), joinpath(dst, p), recursive=recursive) | ||
end | ||
end |
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.
We should probably have an else
here that throws an error if the user tries to copy a directory without recursive
.
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.
Agreed, should we create a new error for this? Python throws IsADirectoryError
.
We should also have a suggestion to use the recursive flag in the error message. Otherwise we'll get bug reports that cp can't copy directories, which is a waste of our users time (and our own when we have to answer ). |
Pushed new version that raises |
end | ||
else | ||
throw(ArgumentError(string("'$src' is a directory. ", | ||
"To copy, set the `recursive` keyword argument to true."))) |
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.
How about
'$src' is a directory.
Use `cp(src, dst, recursive=true)` to copy directories recursively.
I don't like to rely on users understanding terminology.
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.
Agreed, updating and pushing.
Travis Linux i686 failure appears unrelated, OS X timed out. All others passed. |
OS X timed out again. |
Good to see another PR from you @ninjin! |
@tkelman: I am recovering, hopefully I can take a stab at extending the Travis integration shortly. |
this is probably worth mentioning in the docs, huh |
Docs in 45e5fa1 |
thanks @ivarne |
Addresses #10434, went with keyword argument as proposed by @simonster to be in line with
rm
. Tests included.Late-night recreational coding, second opinions appreciated.