Open franzfurbass opened 2 years ago
I agree that downloading to .
should definitely not delete the current directory. However, I'm not convinced that it's good for non-interactive programming usage that the behavior of download(url, "path")
depends on the state of the file system. I think it should either save the content of url
to "path"
or fail. It might make sense to add a force::Bool=false
option that controls whether the output path is overwritten or not. It might also make sense to have an into::Bool=false
option that causes the download to be saved into the target rather than at the target. The error message when someone tries download(url, "dir")
and it fails could suggest using into=true
to save into the directory or force=true
to replace the directory.
Technically adding a force
option defaulting to false is breaking, but I think we could do it in a minor release. It seems unlikely that people are relying on this deleting directories.
That would break my workflow of using the shorter and clearer download(url, "dir")
in place of rm("dir"; recursive=true)
.
:trollface:
The function
download
assumes that the output parameter (of type string) is a file. When given a folder name as output parameter the whole folder is deleted which is unexpected. e.g.download(url_to_file, ".")
removes the current directory. Compared to copy the expected behavior is to put the file into the directory like incp path_to_file/file.txt .
.The output parameter should be checked with
isdir
and in that case the a filename derived from the folder name should be used instead.