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

Check isdir(dest) and copy into that directory in cp and friends? #26423

Closed
fredrikekre opened this issue Mar 12, 2018 · 7 comments
Closed

Check isdir(dest) and copy into that directory in cp and friends? #26423

fredrikekre opened this issue Mar 12, 2018 · 7 comments
Labels
domain:filesystem Underlying file system and functions that use it

Comments

@fredrikekre
Copy link
Member

Should we check if "dest" is a directory, and if so, copy "src" to that directory?

julia> mkdir("dest"); touch("src"); cp("src", "dest")
ERROR: ArgumentError: 'dest' exists. `force=true` is required to remove 'dest' before copying.
Stacktrace:
 [1] #checkfor_mv_cp_cptree#10(::Bool, ::Function, ::String, ::String, ::String) at ./file.jl:191
 [2] #checkfor_mv_cp_cptree at ./<missing>:0 [inlined]
 [3] #cp#12(::Bool, ::Bool, ::Nothing, ::Function, ::String, ::String) at ./file.jl:241
 [4] cp(::String, ::String) at ./file.jl:236
 [5] top-level scope

Compare e.g. with

fredrik@fredrik-laptop:~$ mkdir dest && touch src && cp src dest && ls dest
src
@fredrikekre fredrikekre added the domain:filesystem Underlying file system and functions that use it label Mar 12, 2018
@KristofferC KristofferC added the status:triage This should be discussed on a triage call label Mar 12, 2018
@KristofferC
Copy link
Sponsor Member

Adding triage since this changes behavior and probably needs to be decided before 1.0.

@nalimilan
Copy link
Member

What do other languages do? I would think it's reasonable to be stricter in a programming language than in the shell, which allows catching potential mistakes.

@fredrikekre
Copy link
Member Author

fredrikekre commented Mar 12, 2018

Just noting also that the current behaviour is not very useful; it is unlikely that you want to replace a folder with a file (which is the result of setting force = true in the call above), and that, what you currently have to do

cp("src", joinpath(dir, "src"))

is not very nice.

@mbauman
Copy link
Sponsor Member

mbauman commented Mar 12, 2018

Python has both copyfile (with our current semantics) and copy (which supports directory targets). Ruby is similar with copy_file and cp.

@martinholters
Copy link
Member

Would cp(src, into=dir) be a good API here?

@JeffBezanson
Copy link
Sponsor Member

I think changing this would be worse in terms of orthogonality --- if you actually want the logic "if this is a directory then copy into it", then you should write that. Conditional behavior should not be built into the library function itself.

@stevengj
Copy link
Member

stevengj commented Mar 12, 2018

cp(src, into=dir) doesn't seem that much better than cp(src, joinpath(dir, src)). (If somebody needs this function a lot they could always define a cpinto(src, dir) = cp(src, joinpath(dir, src)) function.)

This use-case would have to come up a lot to put this specific functionality in to Base. Grepping Julia packages for uses of cp with joinpath, I see a handful of code that could be marginally shortened with an into= keyword, but it doesn't seem like enough to be worthwhile.

@JeffBezanson JeffBezanson removed the status:triage This should be discussed on a triage call label Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:filesystem Underlying file system and functions that use it
Projects
None yet
Development

No branches or pull requests

7 participants