Refactor use_course() and gain progress reporting #380
Conversation
Fixes #276 Thanks @jeroen for all the pointers! Key things: * Explicit use of a handle is how we hook into progress reporting, even though curl_download() doesn't expose * curl_download() has a built-in check for http status 200 * Using a custom progressfunction because default doesn't always look right in RStudio * Switch to fs functions for file system work
|
Quick screenshot snagged while downloading some hefty course materials from @hadley
This updates in place during the download. |
R/course.R
Outdated
| "A ZIP file named:\n", | ||
| " ", value(base_name), "\n", | ||
| "will be downloaded to this folder:\n", |
There was a problem hiding this comment.
They've already watched the download happen at this point, so seems better to talk about where we're going to put the file.
| @@ -328,6 +331,19 @@ parse_content_disposition <- function(cd) { | |||
| ) | |||
| } | |||
|
|
|||
| progress_fun <- function(down, up) { | |||
There was a problem hiding this comment.
The default curl progress bar gets mangled in (some versions of?) RStudio, so I took @jeroen's advice to copy an approach from tesseract.
|
@jeroen I can't formally request a review from you and I think I followed your advice fairly faithfully, but feel free to comment. |
|
Yes I like it, nice how it saves it under the server suggested filename. Maybe somewhere insert a
|
R/course.R
Outdated
| @@ -188,28 +191,27 @@ download_zip <- function(url, destdir = getwd(), pedantic = FALSE) { | |||
| message( | |||
| "A ZIP file named:\n", | |||
| " ", value(base_name), "\n", | |||
| "will be downloaded to this folder:\n", | |||
| "will be written to this folder:\n", | |||
R/course.R
Outdated
| } | ||
| } | ||
| full_path <- file.path(base_path, base_name) | ||
| full_path <- fs::path(base_path, base_name) | ||
|
|
||
| if (!can_overwrite(full_path)) { | ||
| ## TO DO: it pains me that can_overwrite() always strips to basename | ||
| stop("Aborting download", call. = FALSE) |
There was a problem hiding this comment.
Reword because it's not the download anymore?
Fixes #276 Refactor use_course()
Thanks @jeroen for all the pointers!
Key things:
reporting, even though curl_download() doesn't expose
always look right in RStudio