From c36f94676b706dbf7b4090978715a5c7dd645002 Mon Sep 17 00:00:00 2001 From: colin Date: Mon, 26 Aug 2024 18:13:25 +0200 Subject: [PATCH 01/32] refactor: split use_external/internal, and wrapped download/copy to functions --- R/{use_files.R => use_files_external.R} | 313 +----------------- R/use_files_external_tools.R | 8 + R/use_files_internal.R | 270 +++++++++++++++ R/use_files_internal_tools.R | 10 + .../_snaps/use_files_external_tools.md | 16 + .../_snaps/use_files_internal_tools.md | 13 + .../testthat/test-use_files_external_tools.R | 15 + .../testthat/test-use_files_internal_tools.R | 16 + 8 files changed, 357 insertions(+), 304 deletions(-) rename R/{use_files.R => use_files_external.R} (51%) create mode 100644 R/use_files_external_tools.R create mode 100644 R/use_files_internal.R create mode 100644 R/use_files_internal_tools.R create mode 100644 tests/testthat/_snaps/use_files_external_tools.md create mode 100644 tests/testthat/_snaps/use_files_internal_tools.md create mode 100644 tests/testthat/test-use_files_external_tools.R create mode 100644 tests/testthat/test-use_files_internal_tools.R diff --git a/R/use_files.R b/R/use_files_external.R similarity index 51% rename from R/use_files.R rename to R/use_files_external.R index d4fe66e4..619731b2 100644 --- a/R/use_files.R +++ b/R/use_files_external.R @@ -1,5 +1,5 @@ # For mocking in test -utils_download_file <- function(...){ +utils_download_file <- function(...) { utils_download_file(...) } #' Use Files @@ -27,7 +27,7 @@ use_external_js_file <- function( dir = "inst/app/www", open = FALSE, dir_create = TRUE - ) { +) { old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) @@ -76,9 +76,7 @@ use_external_js_file <- function( return(invisible(FALSE)) } - cat_start_download() - - utils_download_file(url, where) + download_external(url, where) file_created_dance( where, @@ -101,7 +99,7 @@ use_external_css_file <- function( dir = "inst/app/www", open = FALSE, dir_create = TRUE - ) { +) { old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) @@ -150,9 +148,7 @@ use_external_css_file <- function( return(invisible(FALSE)) } - cat_start_download() - - utils_download_file(url, where) + download_external(url, where) file_created_dance( where, @@ -175,7 +171,7 @@ use_external_html_template <- function( dir = "inst/app/www", open = FALSE, dir_create = TRUE - ) { +) { old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) @@ -222,11 +218,7 @@ use_external_html_template <- function( return(invisible(FALSE)) } - cat_start_download() - - utils_download_file(url, where) - - cat_downloaded(where) + download_external(url, where) file_created_dance( where, @@ -248,7 +240,7 @@ use_external_file <- function( dir = "inst/app/www", open = FALSE, dir_create = TRUE - ) { +) { if (missing(name)) { name <- basename(url) } @@ -287,294 +279,7 @@ use_external_file <- function( return(invisible(FALSE)) } - cat_start_download() - - utils_download_file(url, where) - - cat_downloaded(where) - - file_created_dance( - where, - after_creation_message_any_file, - pkg, - dir, - name, - open, - open_or_go_to = FALSE - ) -} - -#' @export -#' @rdname use_files -use_internal_js_file <- function( - path, - name = NULL, - pkg = get_golem_wd(), - dir = "inst/app/www", - open = FALSE, - dir_create = TRUE - ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) - - if (missing(name)) { - name <- basename(path) - } - - check_name_length_is_one(name) - - name <- file_path_sans_ext(name) - new_file <- sprintf("%s.js", name) - - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } - - dir <- fs_path_abs(dir) - - where <- fs_path( - dir, - new_file - ) - - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } - - if (file_ext(path) != "js") { - cat_red_bullet( - "File not added (URL must end with .js extension)" - ) - return(invisible(FALSE)) - } - - cat_start_copy() - - fs_file_copy(path, where) - - file_created_dance( - where, - after_creation_message_css, - pkg, - dir, - name, - open, - catfun = cat_copied - ) -} - -#' @export -#' @rdname use_files -use_internal_css_file <- function( - path, - name = NULL, - pkg = get_golem_wd(), - dir = "inst/app/www", - open = FALSE, - dir_create = TRUE - ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) - - if (missing(name)) { - name <- basename(path) - } - - check_name_length_is_one(name) - - name <- file_path_sans_ext(name) - new_file <- sprintf("%s.css", name) - - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } - - dir <- fs_path_abs(dir) - - where <- fs_path( - dir, - new_file - ) - - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } - - if (file_ext(path) != "css") { - cat_red_bullet( - "File not added (URL must end with .css extension)" - ) - return(invisible(FALSE)) - } - - cat_start_copy() - - fs_file_copy(path, where) - - file_created_dance( - where, - after_creation_message_css, - pkg, - dir, - name, - open, - catfun = cat_copied - ) -} - -#' @export -#' @rdname use_files -use_internal_html_template <- function( - path, - name = "template.html", - pkg = get_golem_wd(), - dir = "inst/app/www", - open = FALSE, - dir_create = TRUE - ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) - - check_name_length_is_one(name) - - new_file <- sprintf( - "%s.html", - file_path_sans_ext(name) - ) - - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } - - dir <- fs_path_abs(dir) - - where <- fs_path( - dir, - new_file - ) - - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } - - if (file_ext(path) != "html") { - cat_red_bullet( - "File not added (URL must end with .html extension)" - ) - return(invisible(FALSE)) - } - - cat_start_copy() - - fs_file_copy(path, where) - - cat_copied(where) - - file_created_dance( - where, - after_creation_message_html_template, - pkg, - dir, - name, - open - ) -} - -#' @export -#' @rdname use_files -use_internal_file <- function( - path, - name = NULL, - pkg = get_golem_wd(), - dir = "inst/app/www", - open = FALSE, - dir_create = TRUE - ) { - if (missing(name)) { - name <- basename(path) - } - - check_name_length_is_one(name) - - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) - - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } - - dir <- fs_path_abs(dir) - - where <- fs_path( - dir, - name - ) - - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } - - cat_start_copy() - - fs_file_copy(path, where) - - cat_copied(where) + download_external(url, where) file_created_dance( where, diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R new file mode 100644 index 00000000..ca1b4f02 --- /dev/null +++ b/R/use_files_external_tools.R @@ -0,0 +1,8 @@ +download_external <- function( + url, + where +) { + cat_start_download() + utils_download_file(url, where) + cat_downloaded(where) +} \ No newline at end of file diff --git a/R/use_files_internal.R b/R/use_files_internal.R new file mode 100644 index 00000000..21e689e3 --- /dev/null +++ b/R/use_files_internal.R @@ -0,0 +1,270 @@ +#' @export +#' @rdname use_files +use_internal_js_file <- function( + path, + name = NULL, + pkg = get_golem_wd(), + dir = "inst/app/www", + open = FALSE, + dir_create = TRUE +) { + old <- setwd(fs_path_abs(pkg)) + on.exit(setwd(old)) + + if (missing(name)) { + name <- basename(path) + } + + check_name_length_is_one(name) + + name <- file_path_sans_ext(name) + new_file <- sprintf("%s.js", name) + + dir_created <- tryCatch( + create_if_needed( + dir, + type = "directory" + ), + error = function(e) { + out <- FALSE + names(out) <- e[[1]] + return(out) + } + ) + + if (isFALSE(dir_created)) { + cat_dir_necessary() + return(invisible(FALSE)) + } + + dir <- fs_path_abs(dir) + + where <- fs_path( + dir, + new_file + ) + + if (fs_file_exists(where)) { + cat_exists(where) + return(invisible(FALSE)) + } + + if (file_ext(path) != "js") { + cat_red_bullet( + "File not added (URL must end with .js extension)" + ) + return(invisible(FALSE)) + } + + copy_internal_file(path, where) + + file_created_dance( + where, + after_creation_message_css, + pkg, + dir, + name, + open, + catfun = cat_copied + ) +} + +#' @export +#' @rdname use_files +use_internal_css_file <- function( + path, + name = NULL, + pkg = get_golem_wd(), + dir = "inst/app/www", + open = FALSE, + dir_create = TRUE +) { + old <- setwd(fs_path_abs(pkg)) + on.exit(setwd(old)) + + if (missing(name)) { + name <- basename(path) + } + + check_name_length_is_one(name) + + name <- file_path_sans_ext(name) + new_file <- sprintf("%s.css", name) + + dir_created <- tryCatch( + create_if_needed( + dir, + type = "directory" + ), + error = function(e) { + out <- FALSE + names(out) <- e[[1]] + return(out) + } + ) + + if (isFALSE(dir_created)) { + cat_dir_necessary() + return(invisible(FALSE)) + } + + dir <- fs_path_abs(dir) + + where <- fs_path( + dir, + new_file + ) + + if (fs_file_exists(where)) { + cat_exists(where) + return(invisible(FALSE)) + } + + if (file_ext(path) != "css") { + cat_red_bullet( + "File not added (URL must end with .css extension)" + ) + return(invisible(FALSE)) + } + + copy_internal_file(path, where) + + file_created_dance( + where, + after_creation_message_css, + pkg, + dir, + name, + open, + catfun = cat_copied + ) +} + +#' @export +#' @rdname use_files +use_internal_html_template <- function( + path, + name = "template.html", + pkg = get_golem_wd(), + dir = "inst/app/www", + open = FALSE, + dir_create = TRUE +) { + old <- setwd(fs_path_abs(pkg)) + on.exit(setwd(old)) + + check_name_length_is_one(name) + + new_file <- sprintf( + "%s.html", + file_path_sans_ext(name) + ) + + dir_created <- tryCatch( + create_if_needed( + dir, + type = "directory" + ), + error = function(e) { + out <- FALSE + names(out) <- e[[1]] + return(out) + } + ) + + if (isFALSE(dir_created)) { + cat_dir_necessary() + return(invisible(FALSE)) + } + + dir <- fs_path_abs(dir) + + where <- fs_path( + dir, + new_file + ) + + if (fs_file_exists(where)) { + cat_exists(where) + return(invisible(FALSE)) + } + + if (file_ext(path) != "html") { + cat_red_bullet( + "File not added (URL must end with .html extension)" + ) + return(invisible(FALSE)) + } + + copy_internal_file(path, where) + + file_created_dance( + where, + after_creation_message_html_template, + pkg, + dir, + name, + open + ) +} + +#' @export +#' @rdname use_files +use_internal_file <- function( + path, + name = NULL, + pkg = get_golem_wd(), + dir = "inst/app/www", + open = FALSE, + dir_create = TRUE +) { + if (missing(name)) { + name <- basename(path) + } + + check_name_length_is_one(name) + + old <- setwd(fs_path_abs(pkg)) + on.exit(setwd(old)) + + dir_created <- tryCatch( + create_if_needed( + dir, + type = "directory" + ), + error = function(e) { + out <- FALSE + names(out) <- e[[1]] + return(out) + } + ) + + if (isFALSE(dir_created)) { + cat_dir_necessary() + return(invisible(FALSE)) + } + + dir <- fs_path_abs(dir) + + where <- fs_path( + dir, + name + ) + + if (fs_file_exists(where)) { + cat_exists(where) + return(invisible(FALSE)) + } + + copy_internal_file(path, where) + + file_created_dance( + where, + after_creation_message_any_file, + pkg, + dir, + name, + open, + open_or_go_to = FALSE + ) +} diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R new file mode 100644 index 00000000..6067c540 --- /dev/null +++ b/R/use_files_internal_tools.R @@ -0,0 +1,10 @@ +copy_internal_file <- function( + path, + where +){ + cat_start_copy() + + fs_file_copy(path, where) + + cat_copied(where) +} \ No newline at end of file diff --git a/tests/testthat/_snaps/use_files_external_tools.md b/tests/testthat/_snaps/use_files_external_tools.md new file mode 100644 index 00000000..1ad59c48 --- /dev/null +++ b/tests/testthat/_snaps/use_files_external_tools.md @@ -0,0 +1,16 @@ +# download_external(url, where) works + + Code + testthat::with_mocked_bindings(utils_download_file = function(url, where) { + print(url) + print(where) + }, { + download_external("https://www.google.com", "inst/app/www/google.html") + }) + Output + + Initiating file download + [1] "https://www.google.com" + [1] "inst/app/www/google.html" + v File downloaded at inst/app/www/google.html + diff --git a/tests/testthat/_snaps/use_files_internal_tools.md b/tests/testthat/_snaps/use_files_internal_tools.md new file mode 100644 index 00000000..b116a0d1 --- /dev/null +++ b/tests/testthat/_snaps/use_files_internal_tools.md @@ -0,0 +1,13 @@ +# copy_internal_file works + + Code + testthat::with_mocked_bindings(copy_internal_file = function(path, where) { + print(path) + print(where) + }, { + copy_internal_file("~/here/this.css", "inst/app/this.css") + }) + Output + [1] "~/here/this.css" + [1] "inst/app/this.css" + diff --git a/tests/testthat/test-use_files_external_tools.R b/tests/testthat/test-use_files_external_tools.R new file mode 100644 index 00000000..66e72cb8 --- /dev/null +++ b/tests/testthat/test-use_files_external_tools.R @@ -0,0 +1,15 @@ +test_that("download_external(url, where) works", { + testthat::expect_snapshot({ + testthat::with_mocked_bindings( + utils_download_file = function(url, where) { + print(url) + print(where) + },{ + download_external( + "https://www.google.com", + "inst/app/www/google.html" + ) + } + ) + }) +}) diff --git a/tests/testthat/test-use_files_internal_tools.R b/tests/testthat/test-use_files_internal_tools.R new file mode 100644 index 00000000..1136c563 --- /dev/null +++ b/tests/testthat/test-use_files_internal_tools.R @@ -0,0 +1,16 @@ +test_that("copy_internal_file works", { + testthat::expect_snapshot({ + testthat::with_mocked_bindings( + copy_internal_file = function(path, where) { + print(path) + print(where) + }, + { + copy_internal_file( + "~/here/this.css", + "inst/app/this.css" + ) + } + ) + }) +}) From 95735ebb80469e837007a4845db13bc961ed60fe Mon Sep 17 00:00:00 2001 From: colin Date: Mon, 26 Aug 2024 21:09:32 +0200 Subject: [PATCH 02/32] feat: checking the url extension now throws an error if the extension is not correct --- R/boostrap_cli.R | 9 ++++++ R/use_files_external.R | 30 ++++++++----------- R/use_files_external_tools.R | 19 +++++++++++- .../_snaps/use_files_external_tools.md | 10 ++++++- .../testthat/test-use_files_external_tools.R | 22 ++++++++++++++ 5 files changed, 70 insertions(+), 20 deletions(-) diff --git a/R/boostrap_cli.R b/R/boostrap_cli.R index f7d6541a..c605e2b4 100644 --- a/R/boostrap_cli.R +++ b/R/boostrap_cli.R @@ -43,3 +43,12 @@ cli_cli_alert_info <- function(...) { ) }) } + +cli_cli_abort <- function(message) { + if (rlang::is_installed("cli")) { + cli::cli_abort(message = message, call = NULL) + } else { + stop(message) + } +} + diff --git a/R/use_files_external.R b/R/use_files_external.R index 619731b2..1d1196d1 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -69,12 +69,10 @@ use_external_js_file <- function( return(invisible(FALSE)) } - if (file_ext(url) != "js") { - cat_red_bullet( - "File not added (URL must end with .js extension)" - ) - return(invisible(FALSE)) - } + check_url_has_the_correct_extension( + url, + "js" + ) download_external(url, where) @@ -141,12 +139,10 @@ use_external_css_file <- function( return(invisible(FALSE)) } - if (file_ext(url) != "css") { - cat_red_bullet( - "File not added (URL must end with .css extension)" - ) - return(invisible(FALSE)) - } + check_url_has_the_correct_extension( + url, + "css" + ) download_external(url, where) @@ -211,12 +207,10 @@ use_external_html_template <- function( return(invisible(FALSE)) } - if (file_ext(url) != "html") { - cat_red_bullet( - "File not added (URL must end with .html extension)" - ) - return(invisible(FALSE)) - } + check_url_has_the_correct_extension( + url, + "html" + ) download_external(url, where) diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index ca1b4f02..87d174c2 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -1,3 +1,19 @@ +check_url_has_the_correct_extension <- function( + url, + type = c("js", "css", "html") +) { + type <- match.arg(type) + if (file_ext(url) != type) { + cli_cli_abort( + paste0( + "File not added (URL must end with .", + type, + " extension)" + ) + ) + } +} + download_external <- function( url, where @@ -5,4 +21,5 @@ download_external <- function( cat_start_download() utils_download_file(url, where) cat_downloaded(where) -} \ No newline at end of file +} + diff --git a/tests/testthat/_snaps/use_files_external_tools.md b/tests/testthat/_snaps/use_files_external_tools.md index 1ad59c48..89a8c835 100644 --- a/tests/testthat/_snaps/use_files_external_tools.md +++ b/tests/testthat/_snaps/use_files_external_tools.md @@ -1,3 +1,11 @@ +# check_url_has_the_correct_extension(url, where) works + + Code + check_url_has_the_correct_extension("https://www.google.com", "js") + Condition + Error: + ! File not added (URL must end with .js extension) + # download_external(url, where) works Code @@ -8,7 +16,7 @@ download_external("https://www.google.com", "inst/app/www/google.html") }) Output - + Initiating file download [1] "https://www.google.com" [1] "inst/app/www/google.html" diff --git a/tests/testthat/test-use_files_external_tools.R b/tests/testthat/test-use_files_external_tools.R index 66e72cb8..8b46f53a 100644 --- a/tests/testthat/test-use_files_external_tools.R +++ b/tests/testthat/test-use_files_external_tools.R @@ -1,3 +1,25 @@ +test_that("check_url_has_the_correct_extension(url, where) works", { + testthat::expect_snapshot(error = TRUE, { + check_url_has_the_correct_extension( + "https://www.google.com", + "js" + ) + }) + expect_error({ + check_url_has_the_correct_extension( + "https://www.google.com", + "js" + ) + }) + testthat::expect_null( + check_url_has_the_correct_extension( + "https://www.google.com/test.js", + "js" + ) + ) +}) + + test_that("download_external(url, where) works", { testthat::expect_snapshot({ testthat::with_mocked_bindings( From 26e931f40c9b22204e06c8e9dbe45a615f0447b5 Mon Sep 17 00:00:00 2001 From: colin Date: Mon, 26 Aug 2024 21:35:13 +0200 Subject: [PATCH 03/32] refactor: use build_name inside external --- R/use_files_external.R | 36 +++++++++--------- R/use_files_external_tools.R | 11 ++++++ R/use_files_internal.R | 37 +++++++++---------- .../testthat/test-use_files_external_tools.R | 35 +++++++++++++++++- 4 files changed, 80 insertions(+), 39 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index 1d1196d1..9ee2f57d 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -31,13 +31,11 @@ use_external_js_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - if (missing(name)) { - name <- basename(url) - } - - check_name_length_is_one(name) + name <- build_name( + name, + url + ) - name <- file_path_sans_ext(name) new_file <- sprintf("%s.js", name) dir_created <- tryCatch( @@ -101,13 +99,11 @@ use_external_css_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - if (missing(name)) { - name <- basename(url) - } - - check_name_length_is_one(name) + name <- build_name( + name, + url + ) - name <- file_path_sans_ext(name) new_file <- sprintf("%s.css", name) dir_created <- tryCatch( @@ -176,7 +172,10 @@ use_external_html_template <- function( file_path_sans_ext(name) ) - check_name_length_is_one(name) + name <- build_name( + name, + url + ) dir_created <- tryCatch( create_if_needed( @@ -235,15 +234,14 @@ use_external_file <- function( open = FALSE, dir_create = TRUE ) { - if (missing(name)) { - name <- basename(url) - } - - check_name_length_is_one(name) - old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) + name <- build_name( + name, + url + ) + dir_created <- tryCatch( create_if_needed( dir, diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index 87d174c2..10b47324 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -1,3 +1,14 @@ +build_name <- function( + name, + url +) { + if (missing(name) || is.null(name)) { + name <- basename(url) + } + check_name_length_is_one(name) + file_path_sans_ext(name) +} + check_url_has_the_correct_extension <- function( url, type = c("js", "css", "html") diff --git a/R/use_files_internal.R b/R/use_files_internal.R index 21e689e3..0998af3e 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -11,13 +11,11 @@ use_internal_js_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - if (missing(name)) { - name <- basename(path) - } - - check_name_length_is_one(name) + name <- build_name_for_file( + name, + url + ) - name <- file_path_sans_ext(name) new_file <- sprintf("%s.js", name) dir_created <- tryCatch( @@ -82,13 +80,11 @@ use_internal_css_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - if (missing(name)) { - name <- basename(path) - } - - check_name_length_is_one(name) + name <- build_name_for_file( + name, + url + ) - name <- file_path_sans_ext(name) new_file <- sprintf("%s.css", name) dir_created <- tryCatch( @@ -153,7 +149,11 @@ use_internal_html_template <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_name_length_is_one(name) + name <- build_name_for_file( + name, + url + ) + new_file <- sprintf( "%s.html", @@ -218,15 +218,14 @@ use_internal_file <- function( open = FALSE, dir_create = TRUE ) { - if (missing(name)) { - name <- basename(path) - } - - check_name_length_is_one(name) - old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) + name <- build_name_for_file( + name, + url + ) + dir_created <- tryCatch( create_if_needed( dir, diff --git a/tests/testthat/test-use_files_external_tools.R b/tests/testthat/test-use_files_external_tools.R index 8b46f53a..3b4f1ecf 100644 --- a/tests/testthat/test-use_files_external_tools.R +++ b/tests/testthat/test-use_files_external_tools.R @@ -1,3 +1,35 @@ +test_that("build_name works as expected", { + expect_equal( + build_name( + "example.txt", + "http://example.com/file.txt" + ), + "example" + ) + + expect_equal( + build_name(NULL, "http://example.com/file.txt"), + "file" + ) + + expect_equal( + build_name(url = "http://example.com/file.txt"), + "file" + ) + + expect_equal( + build_name("example", "http://example.com/file.txt"), + "example" + ) + + expect_error( + build_name( + c("name1", "name2"), + "http://example.com/file.txt" + ) + ) +}) + test_that("check_url_has_the_correct_extension(url, where) works", { testthat::expect_snapshot(error = TRUE, { check_url_has_the_correct_extension( @@ -26,7 +58,8 @@ test_that("download_external(url, where) works", { utils_download_file = function(url, where) { print(url) print(where) - },{ + }, + { download_external( "https://www.google.com", "inst/app/www/google.html" From 8c5910fc2718db3c71cd9ad77229f4b39cc828fd Mon Sep 17 00:00:00 2001 From: colin Date: Mon, 26 Aug 2024 21:38:13 +0200 Subject: [PATCH 04/32] refactor: split into two files --- R/use_files_external_tools.R | 11 ------- R/use_files_shared_tools.R | 10 ++++++ .../testthat/test-use_files_external_tools.R | 31 ------------------- tests/testthat/test-use_files_shared_tools.R | 31 +++++++++++++++++++ 4 files changed, 41 insertions(+), 42 deletions(-) create mode 100644 R/use_files_shared_tools.R create mode 100644 tests/testthat/test-use_files_shared_tools.R diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index 10b47324..87d174c2 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -1,14 +1,3 @@ -build_name <- function( - name, - url -) { - if (missing(name) || is.null(name)) { - name <- basename(url) - } - check_name_length_is_one(name) - file_path_sans_ext(name) -} - check_url_has_the_correct_extension <- function( url, type = c("js", "css", "html") diff --git a/R/use_files_shared_tools.R b/R/use_files_shared_tools.R new file mode 100644 index 00000000..59cf064e --- /dev/null +++ b/R/use_files_shared_tools.R @@ -0,0 +1,10 @@ +build_name <- function( + name, + url +) { + if (missing(name) || is.null(name)) { + name <- basename(url) + } + check_name_length_is_one(name) + file_path_sans_ext(name) +} \ No newline at end of file diff --git a/tests/testthat/test-use_files_external_tools.R b/tests/testthat/test-use_files_external_tools.R index 3b4f1ecf..1498dcf9 100644 --- a/tests/testthat/test-use_files_external_tools.R +++ b/tests/testthat/test-use_files_external_tools.R @@ -1,34 +1,3 @@ -test_that("build_name works as expected", { - expect_equal( - build_name( - "example.txt", - "http://example.com/file.txt" - ), - "example" - ) - - expect_equal( - build_name(NULL, "http://example.com/file.txt"), - "file" - ) - - expect_equal( - build_name(url = "http://example.com/file.txt"), - "file" - ) - - expect_equal( - build_name("example", "http://example.com/file.txt"), - "example" - ) - - expect_error( - build_name( - c("name1", "name2"), - "http://example.com/file.txt" - ) - ) -}) test_that("check_url_has_the_correct_extension(url, where) works", { testthat::expect_snapshot(error = TRUE, { diff --git a/tests/testthat/test-use_files_shared_tools.R b/tests/testthat/test-use_files_shared_tools.R new file mode 100644 index 00000000..9e64227b --- /dev/null +++ b/tests/testthat/test-use_files_shared_tools.R @@ -0,0 +1,31 @@ +test_that("build_name works as expected", { + expect_equal( + build_name( + "example.txt", + "http://example.com/file.txt" + ), + "example" + ) + + expect_equal( + build_name(NULL, "http://example.com/file.txt"), + "file" + ) + + expect_equal( + build_name(url = "http://example.com/file.txt"), + "file" + ) + + expect_equal( + build_name("example", "http://example.com/file.txt"), + "example" + ) + + expect_error( + build_name( + c("name1", "name2"), + "http://example.com/file.txt" + ) + ) +}) From 82b5508ade7bfab368d1db4b58129c1732a5aca3 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 09:13:58 +0200 Subject: [PATCH 05/32] feat: the function creation files now fail if the dir doesn't not exist Instead of trying to create the dir, the function creation files now fail if the dir doesn't exist. --- R/use_files_external.R | 68 +---------- R/use_files_internal.R | 106 +++++------------- R/use_files_internal_tools.R | 16 +++ R/use_files_shared_tools.R | 14 ++- .../_snaps/use_files_internal_tools.md | 8 ++ tests/testthat/test-use_files.R | 3 +- .../testthat/test-use_files_internal_tools.R | 21 ++++ tests/testthat/test-use_files_shared_tools.R | 9 ++ 8 files changed, 100 insertions(+), 145 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index 9ee2f57d..54cb83b7 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -38,22 +38,7 @@ use_external_js_file <- function( new_file <- sprintf("%s.js", name) - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } + check_directory_exists(dir) dir <- fs_path_abs(dir) @@ -106,22 +91,7 @@ use_external_css_file <- function( new_file <- sprintf("%s.css", name) - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } + check_directory_exists(dir) dir <- fs_path_abs(dir) @@ -177,22 +147,7 @@ use_external_html_template <- function( url ) - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } + check_directory_exists(dir) dir <- fs_path_abs(dir) @@ -242,22 +197,7 @@ use_external_file <- function( url ) - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } + check_directory_exists(dir) dir <- fs_path_abs(dir) diff --git a/R/use_files_internal.R b/R/use_files_internal.R index 0998af3e..27283ca4 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -11,29 +11,19 @@ use_internal_js_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name_for_file( + check_file_has_the_correct_extension( + path, + "js" + ) + + name <- build_name( name, - url + path ) new_file <- sprintf("%s.js", name) - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } + check_directory_exists(dir) dir <- fs_path_abs(dir) @@ -80,29 +70,19 @@ use_internal_css_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name_for_file( + check_file_has_the_correct_extension( + path, + "css" + ) + + name <- build_name( name, - url + path ) new_file <- sprintf("%s.css", name) - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } + check_directory_exists(dir) dir <- fs_path_abs(dir) @@ -149,33 +129,22 @@ use_internal_html_template <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name_for_file( - name, - url + check_file_has_the_correct_extension( + path, + "html" ) + name <- build_name( + name, + path + ) new_file <- sprintf( "%s.html", file_path_sans_ext(name) ) - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } - ) - - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } + check_directory_exists(dir) dir <- fs_path_abs(dir) @@ -189,12 +158,6 @@ use_internal_html_template <- function( return(invisible(FALSE)) } - if (file_ext(path) != "html") { - cat_red_bullet( - "File not added (URL must end with .html extension)" - ) - return(invisible(FALSE)) - } copy_internal_file(path, where) @@ -221,27 +184,12 @@ use_internal_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name_for_file( + name <- build_name( name, - url - ) - - dir_created <- tryCatch( - create_if_needed( - dir, - type = "directory" - ), - error = function(e) { - out <- FALSE - names(out) <- e[[1]] - return(out) - } + path ) - if (isFALSE(dir_created)) { - cat_dir_necessary() - return(invisible(FALSE)) - } + check_directory_exists(dir) dir <- fs_path_abs(dir) diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R index 6067c540..013a5cee 100644 --- a/R/use_files_internal_tools.R +++ b/R/use_files_internal_tools.R @@ -7,4 +7,20 @@ copy_internal_file <- function( fs_file_copy(path, where) cat_copied(where) +} + +check_file_has_the_correct_extension <- function( + path, + type = c("js", "css", "html") +) { + type <- match.arg(type) + if (file_ext(path) != type) { + cli_cli_abort( + paste0( + "File not added (URL must end with .", + type, + " extension)" + ) + ) + } } \ No newline at end of file diff --git a/R/use_files_shared_tools.R b/R/use_files_shared_tools.R index 59cf064e..18937a14 100644 --- a/R/use_files_shared_tools.R +++ b/R/use_files_shared_tools.R @@ -7,4 +7,16 @@ build_name <- function( } check_name_length_is_one(name) file_path_sans_ext(name) -} \ No newline at end of file +} + +check_directory_exists <- function(dir){ + if (!fs_dir_exists(dir)) { + cli_cli_abort( + sprintf( + "The %s directory is required but does not exist.\n\nYou can create it with:\ndir.create('%s', recursive = TRUE)", + dir, + dir + ) + ) + } +} diff --git a/tests/testthat/_snaps/use_files_internal_tools.md b/tests/testthat/_snaps/use_files_internal_tools.md index b116a0d1..42d8e9c0 100644 --- a/tests/testthat/_snaps/use_files_internal_tools.md +++ b/tests/testthat/_snaps/use_files_internal_tools.md @@ -11,3 +11,11 @@ [1] "~/here/this.css" [1] "inst/app/this.css" +# check_url_has_the_correct_extension(url, where) works + + Code + check_file_has_the_correct_extension("https://www.google.com", "js") + Condition + Error: + ! File not added (URL must end with .js extension) + diff --git a/tests/testthat/test-use_files.R b/tests/testthat/test-use_files.R index 47ccd328..7572de8a 100644 --- a/tests/testthat/test-use_files.R +++ b/tests/testthat/test-use_files.R @@ -49,8 +49,9 @@ test_that( ) mapply( function(fun, ext) { + if (ext != "txt") { - expect_false( + expect_error( fun( path = "this.nop", pkg = "." diff --git a/tests/testthat/test-use_files_internal_tools.R b/tests/testthat/test-use_files_internal_tools.R index 1136c563..12a3eef8 100644 --- a/tests/testthat/test-use_files_internal_tools.R +++ b/tests/testthat/test-use_files_internal_tools.R @@ -14,3 +14,24 @@ test_that("copy_internal_file works", { ) }) }) + +test_that("check_url_has_the_correct_extension(url, where) works", { + testthat::expect_snapshot(error = TRUE, { + check_file_has_the_correct_extension( + "https://www.google.com", + "js" + ) + }) + expect_error({ + check_file_has_the_correct_extension( + "https://www.google.com", + "js" + ) + }) + testthat::expect_null( + check_file_has_the_correct_extension( + "https://www.google.com/test.js", + "js" + ) + ) +}) \ No newline at end of file diff --git a/tests/testthat/test-use_files_shared_tools.R b/tests/testthat/test-use_files_shared_tools.R index 9e64227b..ce22b9f2 100644 --- a/tests/testthat/test-use_files_shared_tools.R +++ b/tests/testthat/test-use_files_shared_tools.R @@ -29,3 +29,12 @@ test_that("build_name works as expected", { ) ) }) + +test_that("check_directory_exists works as expected", { + expect_error( + check_directory_exists("inst/app/www") + ) + expect_null( + check_directory_exists(getwd()) + ) +}) \ No newline at end of file From b3a1320cf6308225541ac33bf20885bf02dadf68 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 09:37:27 +0200 Subject: [PATCH 06/32] feat: use_*_files now throws an error if the file already exists --- R/use_files_external.R | 20 +++-------- R/use_files_internal.R | 36 +++---------------- R/use_files_shared_tools.R | 12 +++++++ .../testthat/_snaps/use_files_shared_tools.md | 10 ++++++ tests/testthat/test-use_files.R | 1 + tests/testthat/test-use_files_shared_tools.R | 29 ++++++++++++++- 6 files changed, 59 insertions(+), 49 deletions(-) create mode 100644 tests/testthat/_snaps/use_files_shared_tools.md diff --git a/R/use_files_external.R b/R/use_files_external.R index 54cb83b7..a1f4dbe8 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -47,10 +47,7 @@ use_external_js_file <- function( new_file ) - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } + check_file_exists(where) check_url_has_the_correct_extension( url, @@ -100,10 +97,7 @@ use_external_css_file <- function( new_file ) - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } + check_file_exists(where) check_url_has_the_correct_extension( url, @@ -156,10 +150,7 @@ use_external_html_template <- function( new_file ) - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } + check_file_exists(where) check_url_has_the_correct_extension( url, @@ -206,10 +197,7 @@ use_external_file <- function( name ) - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } + check_file_exists(where) download_external(url, where) diff --git a/R/use_files_internal.R b/R/use_files_internal.R index 27283ca4..52e41cb6 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -32,17 +32,7 @@ use_internal_js_file <- function( new_file ) - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } - - if (file_ext(path) != "js") { - cat_red_bullet( - "File not added (URL must end with .js extension)" - ) - return(invisible(FALSE)) - } + check_file_exists(where) copy_internal_file(path, where) @@ -91,18 +81,7 @@ use_internal_css_file <- function( new_file ) - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } - - if (file_ext(path) != "css") { - cat_red_bullet( - "File not added (URL must end with .css extension)" - ) - return(invisible(FALSE)) - } - + check_file_exists(where) copy_internal_file(path, where) file_created_dance( @@ -153,10 +132,7 @@ use_internal_html_template <- function( new_file ) - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } + check_file_exists(where) copy_internal_file(path, where) @@ -197,11 +173,7 @@ use_internal_file <- function( dir, name ) - - if (fs_file_exists(where)) { - cat_exists(where) - return(invisible(FALSE)) - } + check_file_exists(where) copy_internal_file(path, where) diff --git a/R/use_files_shared_tools.R b/R/use_files_shared_tools.R index 18937a14..255fc100 100644 --- a/R/use_files_shared_tools.R +++ b/R/use_files_shared_tools.R @@ -20,3 +20,15 @@ check_directory_exists <- function(dir){ ) } } + +check_file_exists <- function(where) { + if (fs_file_exists(where)) { + cli_cli_abort( + sprintf( + "%s already exists.\n\nYou can delete it with:\nunlink('%s', recursive = TRUE).", + where, + where + ) + ) + } +} diff --git a/tests/testthat/_snaps/use_files_shared_tools.md b/tests/testthat/_snaps/use_files_shared_tools.md new file mode 100644 index 00000000..08f22df2 --- /dev/null +++ b/tests/testthat/_snaps/use_files_shared_tools.md @@ -0,0 +1,10 @@ +# check_directory_exists works as expected + + Code + check_directory_exists("inst/app/www") + Condition + Error: + ! The inst/app/www directory is required but does not exist. + + You can create it with: dir.create('inst/app/www', recursive = TRUE) + diff --git a/tests/testthat/test-use_files.R b/tests/testthat/test-use_files.R index 7572de8a..63ffc145 100644 --- a/tests/testthat/test-use_files.R +++ b/tests/testthat/test-use_files.R @@ -15,6 +15,7 @@ test_that( ) mapply( function(fun, ext) { + unlink(paste0("this.", ext)) path_to_file <- fun( url = paste0("this.", ext), pkg = "." diff --git a/tests/testthat/test-use_files_shared_tools.R b/tests/testthat/test-use_files_shared_tools.R index ce22b9f2..20e71d9e 100644 --- a/tests/testthat/test-use_files_shared_tools.R +++ b/tests/testthat/test-use_files_shared_tools.R @@ -37,4 +37,31 @@ test_that("check_directory_exists works as expected", { expect_null( check_directory_exists(getwd()) ) -}) \ No newline at end of file +}) + +test_that("check_directory_exists works as expected", { + expect_snapshot( + error = TRUE, + { + check_directory_exists("inst/app/www") + } + ) + expect_error( + check_directory_exists("inst/app/www") + ) + expect_null( + check_directory_exists(getwd()) + ) +}) + +test_that("check_file_exists works as expected", { + file <- tempfile(fileext = ".txt") + file.create(file) + expect_error( + check_file_exists(file) + ) + expect_null( + check_file_exists(tempfile(fileext = ".txt")) + ) + unlink(file) +}) From c24e07e2e93623e652a61af95e530021e6ca0b6c Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 10:38:58 +0200 Subject: [PATCH 07/32] refactor: checks are performed at the top of the function and always in the same order --- R/use_files_external.R | 75 ++++++++++++++++++++---------------------- R/use_files_internal.R | 70 ++++++++++++++++++--------------------- R/use_utils_tools.R | 0 3 files changed, 67 insertions(+), 78 deletions(-) create mode 100644 R/use_utils_tools.R diff --git a/R/use_files_external.R b/R/use_files_external.R index a1f4dbe8..ff6dd2ad 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -31,27 +31,25 @@ use_external_js_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name( - name, - url - ) - - new_file <- sprintf("%s.js", name) - check_directory_exists(dir) + check_url_has_the_correct_extension( + url, + "js" + ) + dir <- fs_path_abs(dir) where <- fs_path( dir, - new_file + sprintf("%s.js", name) ) check_file_exists(where) - check_url_has_the_correct_extension( - url, - "js" + name <- build_name( + name, + url ) download_external(url, where) @@ -81,27 +79,25 @@ use_external_css_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name( - name, - url - ) - - new_file <- sprintf("%s.css", name) - check_directory_exists(dir) + check_url_has_the_correct_extension( + url, + "css" + ) + dir <- fs_path_abs(dir) where <- fs_path( dir, - new_file + sprintf("%s.css", name) ) check_file_exists(where) - check_url_has_the_correct_extension( - url, - "css" + name <- build_name( + name, + url ) download_external(url, where) @@ -131,30 +127,29 @@ use_external_html_template <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - new_file <- sprintf( - "%s.html", - file_path_sans_ext(name) - ) - - name <- build_name( - name, - url - ) check_directory_exists(dir) + check_url_has_the_correct_extension( + url, + "html" + ) + dir <- fs_path_abs(dir) where <- fs_path( dir, - new_file + sprintf( + "%s.html", + name + ) ) check_file_exists(where) - check_url_has_the_correct_extension( - url, - "html" + name <- build_name( + name, + url ) download_external(url, where) @@ -183,11 +178,6 @@ use_external_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name( - name, - url - ) - check_directory_exists(dir) dir <- fs_path_abs(dir) @@ -199,6 +189,11 @@ use_external_file <- function( check_file_exists(where) + name <- build_name( + name, + url + ) + download_external(url, where) file_created_dance( diff --git a/R/use_files_internal.R b/R/use_files_internal.R index 52e41cb6..f6aadef5 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -11,29 +11,27 @@ use_internal_js_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) + check_directory_exists(dir) + check_file_has_the_correct_extension( path, "js" ) - name <- build_name( - name, - path - ) - - new_file <- sprintf("%s.js", name) - - check_directory_exists(dir) - dir <- fs_path_abs(dir) where <- fs_path( dir, - new_file + sprintf("%s.js", name) ) check_file_exists(where) + name <- build_name( + name, + path + ) + copy_internal_file(path, where) file_created_dance( @@ -60,28 +58,27 @@ use_internal_css_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) + check_directory_exists(dir) + check_file_has_the_correct_extension( path, "css" ) - name <- build_name( - name, - path - ) - - new_file <- sprintf("%s.css", name) - - check_directory_exists(dir) - dir <- fs_path_abs(dir) where <- fs_path( dir, - new_file + sprintf("%s.css", name) ) check_file_exists(where) + + name <- build_name( + name, + path + ) + copy_internal_file(path, where) file_created_dance( @@ -108,32 +105,29 @@ use_internal_html_template <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) + check_directory_exists(dir) + check_file_has_the_correct_extension( path, "html" ) - name <- build_name( - name, - path - ) - - new_file <- sprintf( - "%s.html", - file_path_sans_ext(name) - ) - - check_directory_exists(dir) - dir <- fs_path_abs(dir) where <- fs_path( dir, - new_file + sprintf( + "%s.html", + name + ) ) check_file_exists(where) + name <- build_name( + name, + path + ) copy_internal_file(path, where) @@ -160,11 +154,6 @@ use_internal_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name( - name, - path - ) - check_directory_exists(dir) dir <- fs_path_abs(dir) @@ -175,6 +164,11 @@ use_internal_file <- function( ) check_file_exists(where) + name <- build_name( + name, + path + ) + copy_internal_file(path, where) file_created_dance( diff --git a/R/use_utils_tools.R b/R/use_utils_tools.R new file mode 100644 index 00000000..e69de29b From 7722dfb0e001374ea0ba30364bc0c9a758257711 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 10:44:31 +0200 Subject: [PATCH 08/32] fix: define name before --- R/use_files_external.R | 41 ++++++++++++++++++++--------------------- R/use_files_internal.R | 41 +++++++++++++++++++++-------------------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index ff6dd2ad..7eefa41e 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -38,6 +38,11 @@ use_external_js_file <- function( "js" ) + name <- build_name( + name, + url + ) + dir <- fs_path_abs(dir) where <- fs_path( @@ -47,11 +52,6 @@ use_external_js_file <- function( check_file_exists(where) - name <- build_name( - name, - url - ) - download_external(url, where) file_created_dance( @@ -86,6 +86,11 @@ use_external_css_file <- function( "css" ) + name <- build_name( + name, + url + ) + dir <- fs_path_abs(dir) where <- fs_path( @@ -95,11 +100,6 @@ use_external_css_file <- function( check_file_exists(where) - name <- build_name( - name, - url - ) - download_external(url, where) file_created_dance( @@ -127,7 +127,6 @@ use_external_html_template <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) check_url_has_the_correct_extension( @@ -135,6 +134,11 @@ use_external_html_template <- function( "html" ) + name <- build_name( + name, + url + ) + dir <- fs_path_abs(dir) where <- fs_path( @@ -147,11 +151,6 @@ use_external_html_template <- function( check_file_exists(where) - name <- build_name( - name, - url - ) - download_external(url, where) file_created_dance( @@ -180,6 +179,11 @@ use_external_file <- function( check_directory_exists(dir) + name <- build_name( + name, + url + ) + dir <- fs_path_abs(dir) where <- fs_path( @@ -189,11 +193,6 @@ use_external_file <- function( check_file_exists(where) - name <- build_name( - name, - url - ) - download_external(url, where) file_created_dance( diff --git a/R/use_files_internal.R b/R/use_files_internal.R index f6aadef5..44a54210 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -18,6 +18,11 @@ use_internal_js_file <- function( "js" ) + name <- build_name( + name, + path + ) + dir <- fs_path_abs(dir) where <- fs_path( @@ -26,12 +31,6 @@ use_internal_js_file <- function( ) check_file_exists(where) - - name <- build_name( - name, - path - ) - copy_internal_file(path, where) file_created_dance( @@ -65,6 +64,11 @@ use_internal_css_file <- function( "css" ) + name <- build_name( + name, + path + ) + dir <- fs_path_abs(dir) where <- fs_path( @@ -74,10 +78,7 @@ use_internal_css_file <- function( check_file_exists(where) - name <- build_name( - name, - path - ) + copy_internal_file(path, where) @@ -112,6 +113,11 @@ use_internal_html_template <- function( "html" ) + name <- build_name( + name, + path + ) + dir <- fs_path_abs(dir) where <- fs_path( @@ -124,11 +130,6 @@ use_internal_html_template <- function( check_file_exists(where) - name <- build_name( - name, - path - ) - copy_internal_file(path, where) file_created_dance( @@ -158,17 +159,17 @@ use_internal_file <- function( dir <- fs_path_abs(dir) + name <- build_name( + name, + path + ) + where <- fs_path( dir, name ) check_file_exists(where) - name <- build_name( - name, - path - ) - copy_internal_file(path, where) file_created_dance( From 105ea669bae3c018e37ceffa1dab9a3f9cd02bba Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 10:51:05 +0200 Subject: [PATCH 09/32] refactor: using basename of where instead of name in file_created_dance --- R/after_creation_msg.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/after_creation_msg.R b/R/after_creation_msg.R index d09e03d5..3c48f600 100644 --- a/R/after_creation_msg.R +++ b/R/after_creation_msg.R @@ -128,7 +128,7 @@ file_created_dance <- function( ) { catfun(where) - fun(pkg, dir, name) + fun(pkg, dir, basename(where)) if (open_or_go_to) { open_or_go_to( From b8f9e1508bb9fb258be3216e7e11764b20cb365a Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 11:03:59 +0200 Subject: [PATCH 10/32] refactor: merging check_file_exists and download_external --- R/use_files_external.R | 78 +++++++++++++++++++----------------- R/use_files_external_tools.R | 7 ++++ 2 files changed, 48 insertions(+), 37 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index 7eefa41e..f969c90c 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -45,22 +45,23 @@ use_external_js_file <- function( dir <- fs_path_abs(dir) - where <- fs_path( + where_to_download <- fs_path( dir, sprintf("%s.js", name) ) - check_file_exists(where) - - download_external(url, where) + check_if_file_exists_and_download_if_not( + url, + where_to_download = where_to_download + ) file_created_dance( - where, + where = where_to_download, after_creation_message_js, - pkg, - dir, - name, - open, + pkg = pkg, + dir = dir, + name = name, + open = open, open_or_go_to = FALSE, catfun = cat_downloaded ) @@ -93,22 +94,23 @@ use_external_css_file <- function( dir <- fs_path_abs(dir) - where <- fs_path( + where_to_download <- fs_path( dir, sprintf("%s.css", name) ) - check_file_exists(where) - - download_external(url, where) + check_if_file_exists_and_download_if_not( + url, + where_to_download = where_to_download + ) file_created_dance( - where, + where = where_to_download, after_creation_message_css, - pkg, - dir, - name, - open, + pkg = pkg, + dir = dir, + name = name, + open = open, open_or_go_to = FALSE, catfun = cat_downloaded ) @@ -141,7 +143,7 @@ use_external_html_template <- function( dir <- fs_path_abs(dir) - where <- fs_path( + where_to_download <- fs_path( dir, sprintf( "%s.html", @@ -149,17 +151,18 @@ use_external_html_template <- function( ) ) - check_file_exists(where) - - download_external(url, where) + check_if_file_exists_and_download_if_not( + url, + where_to_download = where_to_download + ) file_created_dance( - where, + where = where_to_download, after_creation_message_html_template, - pkg, - dir, - name, - open, + pkg = pkg, + dir = dir, + name = name, + open = open, open_or_go_to = FALSE ) } @@ -186,22 +189,23 @@ use_external_file <- function( dir <- fs_path_abs(dir) - where <- fs_path( + where_to_download <- fs_path( dir, name ) - check_file_exists(where) - - download_external(url, where) + check_if_file_exists_and_download_if_not( + url, + where_to_download = where_to_download + ) file_created_dance( - where, - after_creation_message_any_file, - pkg, - dir, - name, - open, + where = where_to_download, + fun = after_creation_message_any_file, + pkg = pkg, + dir = dir, + name = name, + open = open, open_or_go_to = FALSE ) } diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index 87d174c2..f2521121 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -23,3 +23,10 @@ download_external <- function( cat_downloaded(where) } +check_if_file_exists_and_download_if_not <- function( + url, + where_to_download +){ + check_file_exists(where_to_download) + download_external(url, where_to_download) +} \ No newline at end of file From 84cf0aeb5ba3de8a94052e7af6281eddb90c554f Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 11:15:54 +0200 Subject: [PATCH 11/32] refactor: merged check_file_exists and copy_internal_file --- R/use_files_internal.R | 44 ++++++++++++++++++++---------------- R/use_files_internal_tools.R | 8 +++++++ 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/R/use_files_internal.R b/R/use_files_internal.R index 44a54210..094adc86 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -25,16 +25,18 @@ use_internal_js_file <- function( dir <- fs_path_abs(dir) - where <- fs_path( + where_to_copy_to <- fs_path( dir, sprintf("%s.js", name) ) - check_file_exists(where) - copy_internal_file(path, where) + check_if_file_exists_and_copy_if_not( + path, + where_to_copy_to = where_to_copy_to + ) file_created_dance( - where, + where = where_to_copy_to, after_creation_message_css, pkg, dir, @@ -71,19 +73,18 @@ use_internal_css_file <- function( dir <- fs_path_abs(dir) - where <- fs_path( + where_to_copy_to <- fs_path( dir, sprintf("%s.css", name) ) - check_file_exists(where) - - - - copy_internal_file(path, where) + check_if_file_exists_and_copy_if_not( + path, + where_to_copy_to = where_to_copy_to + ) file_created_dance( - where, + where = where_to_copy_to, after_creation_message_css, pkg, dir, @@ -120,7 +121,7 @@ use_internal_html_template <- function( dir <- fs_path_abs(dir) - where <- fs_path( + where_to_copy_to <- fs_path( dir, sprintf( "%s.html", @@ -128,12 +129,13 @@ use_internal_html_template <- function( ) ) - check_file_exists(where) - - copy_internal_file(path, where) + check_if_file_exists_and_copy_if_not( + path, + where_to_copy_to = where_to_copy_to + ) file_created_dance( - where, + where = where_to_copy_to, after_creation_message_html_template, pkg, dir, @@ -164,16 +166,18 @@ use_internal_file <- function( path ) - where <- fs_path( + where_to_copy_to <- fs_path( dir, name ) - check_file_exists(where) - copy_internal_file(path, where) + check_if_file_exists_and_copy_if_not( + path, + where_to_copy_to = where_to_copy_to + ) file_created_dance( - where, + where = where_to_copy_to, after_creation_message_any_file, pkg, dir, diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R index 013a5cee..657ba18f 100644 --- a/R/use_files_internal_tools.R +++ b/R/use_files_internal_tools.R @@ -23,4 +23,12 @@ check_file_has_the_correct_extension <- function( ) ) } +} + +check_if_file_exists_and_copy_if_not <- function( + path, + where_to_copy_to +) { + check_file_exists(where_to_copy_to) + copy_internal_file(path, where_to_copy_to) } \ No newline at end of file From 4b590f2a1048ef1570cc70308bea51344dd456b0 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 11:32:50 +0200 Subject: [PATCH 12/32] refactor: renaming params --- R/use_files_external_tools.R | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index f2521121..431d7681 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -16,17 +16,20 @@ check_url_has_the_correct_extension <- function( download_external <- function( url, - where + where_to_download ) { cat_start_download() - utils_download_file(url, where) + utils_download_file( + url, + where_to_download + ) cat_downloaded(where) } check_if_file_exists_and_download_if_not <- function( url, where_to_download -){ +) { check_file_exists(where_to_download) - download_external(url, where_to_download) + download_external(url, where_to_download = where_to_download) } \ No newline at end of file From 9ac5ede4f6644f97addd946be01e9fe6f0c599e4 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 12:33:06 +0200 Subject: [PATCH 13/32] refactor: merging check & download --- R/use_files_external.R | 63 +++++++++++++----------------------- R/use_files_external_tools.R | 22 +++++++++---- 2 files changed, 38 insertions(+), 47 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index f969c90c..e7ef738b 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -1,7 +1,3 @@ -# For mocking in test -utils_download_file <- function(...) { - utils_download_file(...) -} #' Use Files #' #' These functions download files from external sources and put them inside the `inst/app/www` directory. @@ -31,13 +27,6 @@ use_external_js_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) - - check_url_has_the_correct_extension( - url, - "js" - ) - name <- build_name( name, url @@ -50,9 +39,11 @@ use_external_js_file <- function( sprintf("%s.js", name) ) - check_if_file_exists_and_download_if_not( - url, - where_to_download = where_to_download + perform_checks_and_download_if_everything_is_ok( + url_to_download_from = url, + directory_to_download_to = dir, + where_to_download_to = where_to_download, + file_type = "js" ) file_created_dance( @@ -80,13 +71,6 @@ use_external_css_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) - - check_url_has_the_correct_extension( - url, - "css" - ) - name <- build_name( name, url @@ -99,14 +83,16 @@ use_external_css_file <- function( sprintf("%s.css", name) ) - check_if_file_exists_and_download_if_not( - url, - where_to_download = where_to_download + perform_checks_and_download_if_everything_is_ok( + url_to_download_from = url, + directory_to_download_to = dir, + where_to_download_to = where_to_download, + file_type = "css" ) file_created_dance( where = where_to_download, - after_creation_message_css, + fun = after_creation_message_css, pkg = pkg, dir = dir, name = name, @@ -129,13 +115,6 @@ use_external_html_template <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) - - check_url_has_the_correct_extension( - url, - "html" - ) - name <- build_name( name, url @@ -151,14 +130,16 @@ use_external_html_template <- function( ) ) - check_if_file_exists_and_download_if_not( - url, - where_to_download = where_to_download + perform_checks_and_download_if_everything_is_ok( + url_to_download_from = url, + directory_to_download_to = dir, + where_to_download_to = where_to_download, + file_type = "html" ) file_created_dance( where = where_to_download, - after_creation_message_html_template, + fun = after_creation_message_html_template, pkg = pkg, dir = dir, name = name, @@ -180,8 +161,6 @@ use_external_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) - name <- build_name( name, url @@ -194,9 +173,11 @@ use_external_file <- function( name ) - check_if_file_exists_and_download_if_not( - url, - where_to_download = where_to_download + perform_checks_and_download_if_everything_is_ok( + url_to_download_from = url, + directory_to_download_to = dir, + where_to_download_to = where_to_download, + file_type = NULL ) file_created_dance( diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index 431d7681..1740f001 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -14,6 +14,7 @@ check_url_has_the_correct_extension <- function( } } + download_external <- function( url, where_to_download @@ -23,13 +24,22 @@ download_external <- function( url, where_to_download ) - cat_downloaded(where) + cat_downloaded(where_to_download) } -check_if_file_exists_and_download_if_not <- function( - url, - where_to_download +perform_checks_and_download_if_everything_is_ok <- function( + url_to_download_from, + directory_to_download_to, + where_to_download_to, + file_type ) { - check_file_exists(where_to_download) - download_external(url, where_to_download = where_to_download) + if (!is.null(file_type)) { + check_url_has_the_correct_extension( + url = url_to_download_from, + file_type + ) + } + check_directory_exists(directory_to_download_to) + check_file_exists(where_to_download_to) + download_external(url, where_to_download = where_to_download_to) } \ No newline at end of file From dbd3acfc30d1a9d54a10b7e0e99cc860fbb3f8de Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 13:28:04 +0200 Subject: [PATCH 14/32] refactor: merged perform_checks_and_download_if_everything_is_ok & file_created_dance --- R/use_files_external.R | 51 +++++++++--------------------------- R/use_files_external_tools.R | 33 ++++++++++++++++++----- 2 files changed, 40 insertions(+), 44 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index e7ef738b..e97716e7 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -43,19 +43,13 @@ use_external_js_file <- function( url_to_download_from = url, directory_to_download_to = dir, where_to_download_to = where_to_download, - file_type = "js" - ) - - file_created_dance( - where = where_to_download, - after_creation_message_js, + file_type = "js", + file_created_fun = after_creation_message_js, pkg = pkg, - dir = dir, name = name, - open = open, - open_or_go_to = FALSE, - catfun = cat_downloaded + open = open ) + } #' @export @@ -87,18 +81,11 @@ use_external_css_file <- function( url_to_download_from = url, directory_to_download_to = dir, where_to_download_to = where_to_download, - file_type = "css" - ) - - file_created_dance( - where = where_to_download, - fun = after_creation_message_css, + file_type = "css", + file_created_fun = after_creation_message_css, pkg = pkg, - dir = dir, name = name, - open = open, - open_or_go_to = FALSE, - catfun = cat_downloaded + open = open ) } @@ -134,17 +121,11 @@ use_external_html_template <- function( url_to_download_from = url, directory_to_download_to = dir, where_to_download_to = where_to_download, - file_type = "html" - ) - - file_created_dance( - where = where_to_download, - fun = after_creation_message_html_template, + file_type = "html", + file_created_fun = after_creation_message_html_template, pkg = pkg, - dir = dir, name = name, - open = open, - open_or_go_to = FALSE + open = open ) } @@ -177,16 +158,10 @@ use_external_file <- function( url_to_download_from = url, directory_to_download_to = dir, where_to_download_to = where_to_download, - file_type = NULL - ) - - file_created_dance( - where = where_to_download, - fun = after_creation_message_any_file, + file_type = NULL, + file_created_fun = after_creation_message_any_file, pkg = pkg, - dir = dir, name = name, - open = open, - open_or_go_to = FALSE + open = open ) } diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index 1740f001..ebbb4027 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -16,12 +16,12 @@ check_url_has_the_correct_extension <- function( download_external <- function( - url, + url_to_download_from, where_to_download ) { cat_start_download() utils_download_file( - url, + url_to_download_from, where_to_download ) cat_downloaded(where_to_download) @@ -31,7 +31,11 @@ perform_checks_and_download_if_everything_is_ok <- function( url_to_download_from, directory_to_download_to, where_to_download_to, - file_type + file_type, + file_created_fun, + pkg, + name, + open ) { if (!is.null(file_type)) { check_url_has_the_correct_extension( @@ -39,7 +43,24 @@ perform_checks_and_download_if_everything_is_ok <- function( file_type ) } - check_directory_exists(directory_to_download_to) - check_file_exists(where_to_download_to) - download_external(url, where_to_download = where_to_download_to) + check_directory_exists( + directory_to_download_to + ) + check_file_exists( + where_to_download_to + ) + download_external( + url_to_download_from = url_to_download_from, + where_to_download = where_to_download_to + ) + file_created_dance( + where = where_to_download_to, + fun = file_created_fun, + pkg = pkg, + dir = directory_to_download_to, + name = name, + open = open, + open_or_go_to = FALSE, + catfun = cat_downloaded + ) } \ No newline at end of file From 598fe99cb2f3de08d5c3e7e7e31676bfe8bf55e0 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 14:18:26 +0200 Subject: [PATCH 15/32] refactor: merged perform_check and file_created dance in use_files_internal --- R/use_files_internal.R | 110 ++++++++++++----------------------- R/use_files_internal_tools.R | 41 +++++++++++-- 2 files changed, 71 insertions(+), 80 deletions(-) diff --git a/R/use_files_internal.R b/R/use_files_internal.R index 094adc86..5ec1f136 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -11,13 +11,6 @@ use_internal_js_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) - - check_file_has_the_correct_extension( - path, - "js" - ) - name <- build_name( name, path @@ -30,19 +23,15 @@ use_internal_js_file <- function( sprintf("%s.js", name) ) - check_if_file_exists_and_copy_if_not( - path, - where_to_copy_to = where_to_copy_to - ) - - file_created_dance( - where = where_to_copy_to, - after_creation_message_css, - pkg, - dir, - name, - open, - catfun = cat_copied + perform_checks_and_copy_if_everything_is_ok( + path_to_copy_from = path, + directory_to_copy_to = dir, + where_to_copy_to = where_to_copy_to, + file_type = "js", + file_created_fun = after_creation_message_js, + pkg = pkg, + name = name, + open = open ) } @@ -59,13 +48,6 @@ use_internal_css_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) - - check_file_has_the_correct_extension( - path, - "css" - ) - name <- build_name( name, path @@ -78,19 +60,15 @@ use_internal_css_file <- function( sprintf("%s.css", name) ) - check_if_file_exists_and_copy_if_not( - path, - where_to_copy_to = where_to_copy_to - ) - - file_created_dance( - where = where_to_copy_to, - after_creation_message_css, - pkg, - dir, - name, - open, - catfun = cat_copied + perform_checks_and_copy_if_everything_is_ok( + path_to_copy_from = path, + directory_to_copy_to = dir, + where_to_copy_to = where_to_copy_to, + file_type = "css", + file_created_fun = after_creation_message_css, + pkg = pkg, + name = name, + open = open ) } @@ -107,13 +85,6 @@ use_internal_html_template <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) - - check_file_has_the_correct_extension( - path, - "html" - ) - name <- build_name( name, path @@ -129,18 +100,15 @@ use_internal_html_template <- function( ) ) - check_if_file_exists_and_copy_if_not( - path, - where_to_copy_to = where_to_copy_to - ) - - file_created_dance( - where = where_to_copy_to, - after_creation_message_html_template, - pkg, - dir, - name, - open + perform_checks_and_copy_if_everything_is_ok( + path_to_copy_from = path, + directory_to_copy_to = dir, + where_to_copy_to = where_to_copy_to, + file_type = "html", + file_created_fun = after_creation_message_html_template, + pkg = pkg, + name = name, + open = open ) } @@ -157,8 +125,6 @@ use_internal_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - check_directory_exists(dir) - dir <- fs_path_abs(dir) name <- build_name( @@ -171,18 +137,14 @@ use_internal_file <- function( name ) - check_if_file_exists_and_copy_if_not( - path, - where_to_copy_to = where_to_copy_to - ) - - file_created_dance( - where = where_to_copy_to, - after_creation_message_any_file, - pkg, - dir, - name, - open, - open_or_go_to = FALSE + perform_checks_and_copy_if_everything_is_ok( + path_to_copy_from = path, + directory_to_copy_to = dir, + where_to_copy_to = where_to_copy_to, + file_type = NULL, + file_created_fun = after_creation_message_html_template, + pkg = pkg, + name = name, + open = open ) } diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R index 657ba18f..0f1f07c7 100644 --- a/R/use_files_internal_tools.R +++ b/R/use_files_internal_tools.R @@ -1,7 +1,7 @@ copy_internal_file <- function( path, where -){ +) { cat_start_copy() fs_file_copy(path, where) @@ -25,10 +25,39 @@ check_file_has_the_correct_extension <- function( } } -check_if_file_exists_and_copy_if_not <- function( - path, - where_to_copy_to +perform_checks_and_copy_if_everything_is_ok <- function( + path_to_copy_from, + directory_to_copy_to, + where_to_copy_to, + file_type, + file_created_fun, + pkg, + name, + open ) { - check_file_exists(where_to_copy_to) - copy_internal_file(path, where_to_copy_to) + if (!is.null(file_type)) { + check_file_has_the_correct_extension( + path = path_to_copy_from, + file_type + ) + } + check_directory_exists( + directory_to_copy_to + ) + check_file_exists( + where_to_copy_to + ) + copy_internal_file( + path_to_copy_from, + where_to_copy_to + ) + file_created_dance( + where = where_to_copy_to, + fun = after_creation_message_css, + pkg = pkg, + dir = directory_to_copy_to, + name = name, + open_file = open, + catfun = cat_copied + ) } \ No newline at end of file From 23905aa90c67fbca32c1836b8f41df4af33c4f94 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 14:30:34 +0200 Subject: [PATCH 16/32] refactor: where_to_download moved inside the function --- R/use_files_external.R | 24 ------------------------ R/use_files_external_tools.R | 16 ++++++++++++++-- 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index e97716e7..fbc530ec 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -34,11 +34,6 @@ use_external_js_file <- function( dir <- fs_path_abs(dir) - where_to_download <- fs_path( - dir, - sprintf("%s.js", name) - ) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = dir, @@ -72,11 +67,6 @@ use_external_css_file <- function( dir <- fs_path_abs(dir) - where_to_download <- fs_path( - dir, - sprintf("%s.css", name) - ) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = dir, @@ -109,14 +99,6 @@ use_external_html_template <- function( dir <- fs_path_abs(dir) - where_to_download <- fs_path( - dir, - sprintf( - "%s.html", - name - ) - ) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = dir, @@ -149,15 +131,9 @@ use_external_file <- function( dir <- fs_path_abs(dir) - where_to_download <- fs_path( - dir, - name - ) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = dir, - where_to_download_to = where_to_download, file_type = NULL, file_created_fun = after_creation_message_any_file, pkg = pkg, diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index ebbb4027..be0d1f6b 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -30,18 +30,30 @@ download_external <- function( perform_checks_and_download_if_everything_is_ok <- function( url_to_download_from, directory_to_download_to, - where_to_download_to, file_type, file_created_fun, pkg, name, open ) { - if (!is.null(file_type)) { + if (is.null(file_type)) { + where_to_download_to <- fs_path( + dir, + name + ) + } else { check_url_has_the_correct_extension( url = url_to_download_from, file_type ) + where_to_download <- fs_path( + dir, + sprintf( + "%s.%s", + name, + file_type + ) + ) } check_directory_exists( directory_to_download_to From 461bb5aa692a04a94aa858479c8221b30da550ab Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 14:30:34 +0200 Subject: [PATCH 17/32] refactor: where_to_download moved inside the function --- R/use_files_external.R | 3 --- 1 file changed, 3 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index fbc530ec..411ddb68 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -37,7 +37,6 @@ use_external_js_file <- function( perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = dir, - where_to_download_to = where_to_download, file_type = "js", file_created_fun = after_creation_message_js, pkg = pkg, @@ -70,7 +69,6 @@ use_external_css_file <- function( perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = dir, - where_to_download_to = where_to_download, file_type = "css", file_created_fun = after_creation_message_css, pkg = pkg, @@ -102,7 +100,6 @@ use_external_html_template <- function( perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = dir, - where_to_download_to = where_to_download, file_type = "html", file_created_fun = after_creation_message_html_template, pkg = pkg, From 149ce193062b849846912d06fac52e7c42fc7686 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 14:39:11 +0200 Subject: [PATCH 18/32] fix: correct param name --- R/use_files_external_tools.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index be0d1f6b..c8ec152e 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -38,7 +38,7 @@ perform_checks_and_download_if_everything_is_ok <- function( ) { if (is.null(file_type)) { where_to_download_to <- fs_path( - dir, + directory_to_download_to, name ) } else { @@ -46,8 +46,8 @@ perform_checks_and_download_if_everything_is_ok <- function( url = url_to_download_from, file_type ) - where_to_download <- fs_path( - dir, + where_to_download_to <- fs_path( + directory_to_download_to, sprintf( "%s.%s", name, From 75af1ba26cc515f0ad3c23c9af77334e9df01260 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 14:39:25 +0200 Subject: [PATCH 19/32] refactor: path_abs as input --- R/use_files_external.R | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index 411ddb68..a7124318 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -32,11 +32,9 @@ use_external_js_file <- function( url ) - dir <- fs_path_abs(dir) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, - directory_to_download_to = dir, + directory_to_download_to = fs_path_abs(dir), file_type = "js", file_created_fun = after_creation_message_js, pkg = pkg, @@ -64,11 +62,9 @@ use_external_css_file <- function( url ) - dir <- fs_path_abs(dir) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, - directory_to_download_to = dir, + directory_to_download_to = fs_path_abs(dir), file_type = "css", file_created_fun = after_creation_message_css, pkg = pkg, @@ -95,11 +91,9 @@ use_external_html_template <- function( url ) - dir <- fs_path_abs(dir) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, - directory_to_download_to = dir, + directory_to_download_to = fs_path_abs(dir), file_type = "html", file_created_fun = after_creation_message_html_template, pkg = pkg, From 0a4dcab8450779369d66248b5d7da7940ec13413 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 14:40:40 +0200 Subject: [PATCH 20/32] refactor: name is now built inside perform_checks_and_download_if_everything_is_ok --- R/use_files_external.R | 24 +----------------------- R/use_files_external_tools.R | 4 ++++ 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index a7124318..8f454a86 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -27,11 +27,6 @@ use_external_js_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name( - name, - url - ) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = fs_path_abs(dir), @@ -57,11 +52,6 @@ use_external_css_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name( - name, - url - ) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = fs_path_abs(dir), @@ -86,11 +76,6 @@ use_external_html_template <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name( - name, - url - ) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, directory_to_download_to = fs_path_abs(dir), @@ -115,16 +100,9 @@ use_external_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - name <- build_name( - name, - url - ) - - dir <- fs_path_abs(dir) - perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, - directory_to_download_to = dir, + directory_to_download_to = fs_path_abs(dir), file_type = NULL, file_created_fun = after_creation_message_any_file, pkg = pkg, diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index c8ec152e..c53a0c30 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -36,6 +36,10 @@ perform_checks_and_download_if_everything_is_ok <- function( name, open ) { + name <- build_name( + name, + url + ) if (is.null(file_type)) { where_to_download_to <- fs_path( directory_to_download_to, From cbdc7f31390a8a96b10498a446ebe5d1020f6aeb Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 14:41:37 +0200 Subject: [PATCH 21/32] refactor: moving setwd() inside perform_checks_and_download_if_everything_is_ok --- R/use_files_external.R | 8 -------- R/use_files_external_tools.R | 2 ++ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index 8f454a86..dfdeeb22 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -24,8 +24,6 @@ use_external_js_file <- function( open = FALSE, dir_create = TRUE ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, @@ -49,8 +47,6 @@ use_external_css_file <- function( open = FALSE, dir_create = TRUE ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, @@ -73,8 +69,6 @@ use_external_html_template <- function( open = FALSE, dir_create = TRUE ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, @@ -97,8 +91,6 @@ use_external_file <- function( open = FALSE, dir_create = TRUE ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index c53a0c30..fe2d4bc4 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -36,6 +36,8 @@ perform_checks_and_download_if_everything_is_ok <- function( name, open ) { + old <- setwd(fs_path_abs(pkg)) + on.exit(setwd(old)) name <- build_name( name, url From c16f135897ef4d4bb5c544ca4deaaa8b7621aef6 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 14:46:46 +0200 Subject: [PATCH 22/32] fix: correct params in build_name --- R/use_files_external_tools.R | 2 +- R/use_files_shared_tools.R | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index fe2d4bc4..b72d72e1 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -40,7 +40,7 @@ perform_checks_and_download_if_everything_is_ok <- function( on.exit(setwd(old)) name <- build_name( name, - url + url_to_download_from ) if (is.null(file_type)) { where_to_download_to <- fs_path( diff --git a/R/use_files_shared_tools.R b/R/use_files_shared_tools.R index 255fc100..2b86b6f5 100644 --- a/R/use_files_shared_tools.R +++ b/R/use_files_shared_tools.R @@ -1,8 +1,8 @@ build_name <- function( - name, + name = NULL, url ) { - if (missing(name) || is.null(name)) { + if (is.null(name)) { name <- basename(url) } check_name_length_is_one(name) From d0d3276c82e498c232a4e84c713b2948361c20a4 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 16:49:23 +0200 Subject: [PATCH 23/32] refactor: path_abs(dir) as param --- R/use_files_internal.R | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/R/use_files_internal.R b/R/use_files_internal.R index 5ec1f136..d15b441b 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -16,8 +16,6 @@ use_internal_js_file <- function( path ) - dir <- fs_path_abs(dir) - where_to_copy_to <- fs_path( dir, sprintf("%s.js", name) @@ -25,7 +23,7 @@ use_internal_js_file <- function( perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, - directory_to_copy_to = dir, + directory_to_copy_to = fs_path_abs(dir), where_to_copy_to = where_to_copy_to, file_type = "js", file_created_fun = after_creation_message_js, @@ -53,8 +51,6 @@ use_internal_css_file <- function( path ) - dir <- fs_path_abs(dir) - where_to_copy_to <- fs_path( dir, sprintf("%s.css", name) @@ -62,7 +58,7 @@ use_internal_css_file <- function( perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, - directory_to_copy_to = dir, + directory_to_copy_to = fs_path_abs(dir), where_to_copy_to = where_to_copy_to, file_type = "css", file_created_fun = after_creation_message_css, @@ -90,8 +86,6 @@ use_internal_html_template <- function( path ) - dir <- fs_path_abs(dir) - where_to_copy_to <- fs_path( dir, sprintf( @@ -102,7 +96,7 @@ use_internal_html_template <- function( perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, - directory_to_copy_to = dir, + directory_to_copy_to = fs_path_abs(dir), where_to_copy_to = where_to_copy_to, file_type = "html", file_created_fun = after_creation_message_html_template, @@ -125,8 +119,6 @@ use_internal_file <- function( old <- setwd(fs_path_abs(pkg)) on.exit(setwd(old)) - dir <- fs_path_abs(dir) - name <- build_name( name, path @@ -139,7 +131,7 @@ use_internal_file <- function( perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, - directory_to_copy_to = dir, + directory_to_copy_to = fs_path_abs(dir), where_to_copy_to = where_to_copy_to, file_type = NULL, file_created_fun = after_creation_message_html_template, From 2165bf9f47890d1f0f43c593277dc981a9127d8c Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 17:09:58 +0200 Subject: [PATCH 24/32] feat: changed extension error message --- R/use_files_internal_tools.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R index 0f1f07c7..490e25d7 100644 --- a/R/use_files_internal_tools.R +++ b/R/use_files_internal_tools.R @@ -19,7 +19,7 @@ check_file_has_the_correct_extension <- function( paste0( "File not added (URL must end with .", type, - " extension)" + ")" ) ) } From ccfd1d1e09ad6048b70987ade62d7a600c73a160 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 17:17:42 +0200 Subject: [PATCH 25/32] test: update snapshot --- tests/testthat/_snaps/use_files_internal_tools.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/_snaps/use_files_internal_tools.md b/tests/testthat/_snaps/use_files_internal_tools.md index 42d8e9c0..ae05498a 100644 --- a/tests/testthat/_snaps/use_files_internal_tools.md +++ b/tests/testthat/_snaps/use_files_internal_tools.md @@ -17,5 +17,5 @@ check_file_has_the_correct_extension("https://www.google.com", "js") Condition Error: - ! File not added (URL must end with .js extension) + ! File not added (URL must end with .js) From 88c58f67f782d2c89bd5a318a01167370367bb28 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 17:29:36 +0200 Subject: [PATCH 26/32] refactor: where_to_copy_to is inside perform_checks_and_copy_if_everything_is_ok --- R/use_files_internal.R | 25 ------------------------- R/use_files_internal_tools.R | 16 ++++++++++++++-- 2 files changed, 14 insertions(+), 27 deletions(-) diff --git a/R/use_files_internal.R b/R/use_files_internal.R index d15b441b..531dc514 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -16,15 +16,9 @@ use_internal_js_file <- function( path ) - where_to_copy_to <- fs_path( - dir, - sprintf("%s.js", name) - ) - perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), - where_to_copy_to = where_to_copy_to, file_type = "js", file_created_fun = after_creation_message_js, pkg = pkg, @@ -51,15 +45,9 @@ use_internal_css_file <- function( path ) - where_to_copy_to <- fs_path( - dir, - sprintf("%s.css", name) - ) - perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), - where_to_copy_to = where_to_copy_to, file_type = "css", file_created_fun = after_creation_message_css, pkg = pkg, @@ -86,13 +74,6 @@ use_internal_html_template <- function( path ) - where_to_copy_to <- fs_path( - dir, - sprintf( - "%s.html", - name - ) - ) perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, @@ -123,12 +104,6 @@ use_internal_file <- function( name, path ) - - where_to_copy_to <- fs_path( - dir, - name - ) - perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R index 490e25d7..b754a29f 100644 --- a/R/use_files_internal_tools.R +++ b/R/use_files_internal_tools.R @@ -28,18 +28,30 @@ check_file_has_the_correct_extension <- function( perform_checks_and_copy_if_everything_is_ok <- function( path_to_copy_from, directory_to_copy_to, - where_to_copy_to, file_type, file_created_fun, pkg, name, open ) { - if (!is.null(file_type)) { + if (is.null(file_type)) { + where_to_copy_to <- fs_path( + directory_to_copy_to, + name + ) + } else { check_file_has_the_correct_extension( path = path_to_copy_from, file_type ) + where_to_copy_to <- fs_path( + directory_to_copy_to, + sprintf( + "%s.%s", + name, + file_type + ) + ) } check_directory_exists( directory_to_copy_to From f835ffce22d4119c332f3d5e94f7b265316ea309 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 17:31:40 +0200 Subject: [PATCH 27/32] refactor: name & setwd now in wrapper fun --- R/use_files_internal.R | 30 ------------------------------ R/use_files_internal_tools.R | 6 ++++++ 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/R/use_files_internal.R b/R/use_files_internal.R index 531dc514..b7e6349e 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -8,13 +8,6 @@ use_internal_js_file <- function( open = FALSE, dir_create = TRUE ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) - - name <- build_name( - name, - path - ) perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, @@ -37,13 +30,6 @@ use_internal_css_file <- function( open = FALSE, dir_create = TRUE ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) - - name <- build_name( - name, - path - ) perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, @@ -66,19 +52,10 @@ use_internal_html_template <- function( open = FALSE, dir_create = TRUE ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) - - name <- build_name( - name, - path - ) - perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), - where_to_copy_to = where_to_copy_to, file_type = "html", file_created_fun = after_creation_message_html_template, pkg = pkg, @@ -97,17 +74,10 @@ use_internal_file <- function( open = FALSE, dir_create = TRUE ) { - old <- setwd(fs_path_abs(pkg)) - on.exit(setwd(old)) - name <- build_name( - name, - path - ) perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), - where_to_copy_to = where_to_copy_to, file_type = NULL, file_created_fun = after_creation_message_html_template, pkg = pkg, diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R index b754a29f..05327ee2 100644 --- a/R/use_files_internal_tools.R +++ b/R/use_files_internal_tools.R @@ -34,6 +34,12 @@ perform_checks_and_copy_if_everything_is_ok <- function( name, open ) { + old <- setwd(fs_path_abs(pkg)) + on.exit(setwd(old)) + name <- build_name( + name, + path_to_copy_from + ) if (is.null(file_type)) { where_to_copy_to <- fs_path( directory_to_copy_to, From 5b331d54a0d308272057a6ba6bbe78413e1ee4f1 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 17:38:04 +0200 Subject: [PATCH 28/32] feat: deprecate the dir_create param --- R/use_files_external.R | 29 ++++++++++++++++++++++++----- R/use_files_internal.R | 32 ++++++++++++++++++++++++++++---- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/R/use_files_external.R b/R/use_files_external.R index dfdeeb22..3d77a55a 100644 --- a/R/use_files_external.R +++ b/R/use_files_external.R @@ -22,8 +22,13 @@ use_external_js_file <- function( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) { + if (!missing(dir_create)) { + cli_cli_abort( + "The dir_create argument is deprecated." + ) + } perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, @@ -34,7 +39,6 @@ use_external_js_file <- function( name = name, open = open ) - } #' @export @@ -45,8 +49,13 @@ use_external_css_file <- function( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) { + if (!missing(dir_create)) { + cli_cli_abort( + "The dir_create argument is deprecated." + ) + } perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, @@ -67,8 +76,13 @@ use_external_html_template <- function( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) { + if (!missing(dir_create)) { + cli_cli_abort( + "The dir_create argument is deprecated." + ) + } perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, @@ -89,8 +103,13 @@ use_external_file <- function( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) { + if (!missing(dir_create)) { + cli_cli_abort( + "The dir_create argument is deprecated." + ) + } perform_checks_and_download_if_everything_is_ok( url_to_download_from = url, diff --git a/R/use_files_internal.R b/R/use_files_internal.R index b7e6349e..c3ae061b 100644 --- a/R/use_files_internal.R +++ b/R/use_files_internal.R @@ -6,9 +6,15 @@ use_internal_js_file <- function( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) { + if (!missing(dir_create)){ + cli_cli_abort( + "The dir_create argument is deprecated." + ) + } + perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), @@ -28,9 +34,15 @@ use_internal_css_file <- function( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) { + if (!missing(dir_create)){ + cli_cli_abort( + "The dir_create argument is deprecated." + ) + } + perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), @@ -50,9 +62,15 @@ use_internal_html_template <- function( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) { + if (!missing(dir_create)){ + cli_cli_abort( + "The dir_create argument is deprecated." + ) + } + perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), @@ -72,9 +90,15 @@ use_internal_file <- function( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) { + if (!missing(dir_create)){ + cli_cli_abort( + "The dir_create argument is deprecated." + ) + } + perform_checks_and_copy_if_everything_is_ok( path_to_copy_from = path, directory_to_copy_to = fs_path_abs(dir), From 598fc7f4db6a3e00a7ba4c0ca3feeef6543010c8 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 17:45:19 +0200 Subject: [PATCH 29/32] refactor: rm name from file_create_dance in use_file --- R/use_files_external_tools.R | 3 +-- R/use_files_internal_tools.R | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index b72d72e1..7dfd202a 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -76,8 +76,7 @@ perform_checks_and_download_if_everything_is_ok <- function( fun = file_created_fun, pkg = pkg, dir = directory_to_download_to, - name = name, - open = open, + open_file = open, open_or_go_to = FALSE, catfun = cat_downloaded ) diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R index 05327ee2..be74a8b5 100644 --- a/R/use_files_internal_tools.R +++ b/R/use_files_internal_tools.R @@ -74,8 +74,8 @@ perform_checks_and_copy_if_everything_is_ok <- function( fun = after_creation_message_css, pkg = pkg, dir = directory_to_copy_to, - name = name, open_file = open, + open_or_go_to = FALSE, catfun = cat_copied ) } \ No newline at end of file From 0480655db699f9fd20ba952455151838ccae1289 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 18:00:01 +0200 Subject: [PATCH 30/32] refactor: rm the open_or_go_to arg from file_created_dance --- R/after_creation_msg.R | 13 ++++--------- R/use_files_external_tools.R | 1 - R/use_files_internal_tools.R | 1 - tests/testthat/_snaps/after_creation_msg.md | 3 ++- tests/testthat/test-after_creation_msg.R | 3 +-- 5 files changed, 7 insertions(+), 14 deletions(-) diff --git a/R/after_creation_msg.R b/R/after_creation_msg.R index 3c48f600..3c866f77 100644 --- a/R/after_creation_msg.R +++ b/R/after_creation_msg.R @@ -123,21 +123,16 @@ file_created_dance <- function( dir, name, open_file, - open_or_go_to = TRUE, catfun = cat_created ) { catfun(where) fun(pkg, dir, basename(where)) - if (open_or_go_to) { - open_or_go_to( - where = where, - open_file = open_file - ) - } else { - return(invisible(where)) - } + open_or_go_to( + where = where, + open_file = open_file + ) } file_already_there_dance <- function( diff --git a/R/use_files_external_tools.R b/R/use_files_external_tools.R index 7dfd202a..f820310f 100644 --- a/R/use_files_external_tools.R +++ b/R/use_files_external_tools.R @@ -77,7 +77,6 @@ perform_checks_and_download_if_everything_is_ok <- function( pkg = pkg, dir = directory_to_download_to, open_file = open, - open_or_go_to = FALSE, catfun = cat_downloaded ) } \ No newline at end of file diff --git a/R/use_files_internal_tools.R b/R/use_files_internal_tools.R index be74a8b5..adfa4fe8 100644 --- a/R/use_files_internal_tools.R +++ b/R/use_files_internal_tools.R @@ -75,7 +75,6 @@ perform_checks_and_copy_if_everything_is_ok <- function( pkg = pkg, dir = directory_to_copy_to, open_file = open, - open_or_go_to = FALSE, catfun = cat_copied ) } \ No newline at end of file diff --git a/tests/testthat/_snaps/after_creation_msg.md b/tests/testthat/_snaps/after_creation_msg.md index 640b786c..40bc7081 100644 --- a/tests/testthat/_snaps/after_creation_msg.md +++ b/tests/testthat/_snaps/after_creation_msg.md @@ -26,9 +26,10 @@ File downloaded at inst/app/www/myhtml Code file_created_dance("inst/app/www", after_creation_message_sass, "mypkg", - "inst/app/www", "mysass", open_file = FALSE, open_or_go_to = FALSE) + "inst/app/www", "mysass", open_file = FALSE) Output v File created at inst/app/www + * Go to inst/app/www Code file_already_there_dance("inst/app/www", open_file = FALSE) Output diff --git a/tests/testthat/test-after_creation_msg.R b/tests/testthat/test-after_creation_msg.R index b243f3e5..c885f3b1 100644 --- a/tests/testthat/test-after_creation_msg.R +++ b/tests/testthat/test-after_creation_msg.R @@ -41,8 +41,7 @@ test_that("after_creation_msg works", { "mypkg", "inst/app/www", "mysass", - open_file = FALSE, - open_or_go_to = FALSE + open_file = FALSE ) file_already_there_dance( "inst/app/www", From 5801c0f3148fa46228133a223e75455cac38b089 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 18:12:46 +0200 Subject: [PATCH 31/32] doc: news update --- DESCRIPTION | 2 +- NEWS.md | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 15a0a8d1..70bd028c 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: golem Title: A Framework for Robust Shiny Applications -Version: 0.5.1 +Version: 0.5.1.9001 Authors@R: c( person("Colin", "Fay", , "contact@colinfay.me", role = c("cre", "aut"), comment = c(ORCID = "0000-0001-7343-1846")), diff --git a/NEWS.md b/NEWS.md index 57a165d7..08cc227f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,17 @@ > Notes: the # between parenthesis refers to the related issue on GitHub, and the @ refers to an external contributor solving this issue. +# golem 0.5.1 to 0.6.0 + +## Breaking change + +- The use_*_files now fail when: + - The directory where the user tries to add the file doesn't exist. `{golem}` used to try to create the directory but that's not the function job — use_*_file functions should only be there to add file (Singe responsabily ) + - The file that the user tries to create already exists + +## Internal changes + +- Full refactoring of the use_*_files functions that now all share the same behavior + # golem 0.5.1 * Hotfixing a bug with utils_download_file (#1168) From 13376fee3a4d8be90cbc2198ea906eaca8137852 Mon Sep 17 00:00:00 2001 From: colin Date: Tue, 27 Aug 2024 18:39:43 +0200 Subject: [PATCH 32/32] doc: forgot to push the man --- man/use_files.Rd | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/man/use_files.Rd b/man/use_files.Rd index 81ca47ca..27d184dc 100644 --- a/man/use_files.Rd +++ b/man/use_files.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/use_files.R +% Please edit documentation in R/use_files_external.R, R/use_files_internal.R \name{use_external_js_file} \alias{use_external_js_file} \alias{use_external_css_file} @@ -17,7 +17,7 @@ use_external_js_file( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) use_external_css_file( @@ -26,7 +26,7 @@ use_external_css_file( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) use_external_html_template( @@ -35,7 +35,7 @@ use_external_html_template( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) use_external_file( @@ -44,7 +44,7 @@ use_external_file( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) use_internal_js_file( @@ -53,7 +53,7 @@ use_internal_js_file( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) use_internal_css_file( @@ -62,7 +62,7 @@ use_internal_css_file( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) use_internal_html_template( @@ -71,7 +71,7 @@ use_internal_html_template( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) use_internal_file( @@ -80,7 +80,7 @@ use_internal_file( pkg = get_golem_wd(), dir = "inst/app/www", open = FALSE, - dir_create = TRUE + dir_create ) } \arguments{