Conversation
| download.file(input, tmpFile, method=method, mode="wb", quiet=!showProgress) | ||
| # In text mode on Windows-only, R doubles up \r to make \r\r\n line endings. mode="wb" avoids that. See ?connections:"CRLF" | ||
| } | ||
| assign("file", tmpFile, envir=parent.env(environment())) |
There was a problem hiding this comment.
Maybe my view was locked in thinking about the different environments. A return is definitely the more straightforward way.
| do.call( | ||
| on.exit, list(substitute(unlink(tmpFile)), add = TRUE), | ||
| envir = parent.frame() | ||
| ) |
There was a problem hiding this comment.
Why the do.call here; i.e. can't it just be on.exit(unlink(tmpFile), add=TRUE) as before?
There was a problem hiding this comment.
I think we need the do.call here, because we want to unlink the file only after the exiting the parent.frame() (which is the fread function). Using on.exit(unlink(tmpFile), add=TRUE) as before would lead to unlinking the file after leaving the newly added download_file function.
| nThread=as.integer(nThread) | ||
| stopifnot(nThread>=1L) | ||
|
|
||
| is_url = function(x) any(startsWith(x, c("http://", "https://", "ftp://", "ftps://", "file://"))) |
There was a problem hiding this comment.
I glanced at our backport of startsWith to support R prior to 3.3.0 (see utils.R) and it doesn't look like it supports a set like that. If so, our backport needs a tweak to support this usage then. Currently the PR would fail on GLCI after merge when it tests using R 3.1.0.
There was a problem hiding this comment.
I changed the usage here, before fiddling with the backport. This also seems okayish since supported set of URLs will probably not change.
included error so that if folk getting the error search for the error, they might find this news item
4 participants
Closes #4952.
Refactors file download to separate function for better reuse/maintenance.