diff --git a/NEWS.md b/NEWS.md index 351de6b..9d1aaf1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,10 @@ # pkgbuild (development version) +* `build()` no longer fails with an unrelated error when you answer "No" to + the interactive prompt about deleting `inst/doc`. `inst/doc` is now kept and + the build continues, and the prompt says what "Yes" and "No" do + (@taekop, #186). + * The documentation for the `clean_doc` argument of `build()` now fully describes the behavior for `TRUE`, `FALSE`, and `NULL`, including the non-interactive case (@jimhester, #187). diff --git a/R/build.R b/R/build.R index 0c03dbf..fd22473 100644 --- a/R/build.R +++ b/R/build.R @@ -312,18 +312,18 @@ build_setup_source <- function( if (build_vignettes && (is.null(clean_doc) || isTRUE(clean_doc))) { doc_dir <- file.path(path, "inst", "doc") if (dir.exists(doc_dir)) { - if (is.null(clean_doc) && interactive()) { + delete <- TRUE + if (is.null(clean_doc) && is_interactive()) { message( "Building the package will delete...\n '", doc_dir, - "'\nAre you sure?" + "'\nDelete it and build ('Yes'), or keep it and build ('No')?" ) - res <- utils::menu(c("Yes", "No")) - if (res == 2) { - return() - } + delete <- utils::menu(c("Yes", "No")) == 1 + } + if (delete) { + unlink(doc_dir, recursive = TRUE) } - unlink(doc_dir, recursive = TRUE) } } diff --git a/R/utils.R b/R/utils.R index f5e4b49..9e7efdc 100644 --- a/R/utils.R +++ b/R/utils.R @@ -19,6 +19,10 @@ is_windows <- function() { .Platform$OS.type == "windows" } +is_interactive <- function() { + interactive() +} + is_string <- function(x) { is.character(x) && length(x) == 1 && !is.na(x) } diff --git a/tests/testthat/test-build.R b/tests/testthat/test-build.R index 099ab45..1d70b20 100644 --- a/tests/testthat/test-build.R +++ b/tests/testthat/test-build.R @@ -294,6 +294,44 @@ test_that("Config/build/clean-inst-doc TRUE", { expect_true("testInstDoc/inst/doc/test.html" %in% pkg_files) }) +test_that("interactive clean_doc prompt", { + if (Sys.which("pandoc") == "") skip("No pandoc") + src <- withr::local_tempdir() + dest <- withr::local_tempdir() + file.copy(test_path("testInstDoc"), src, recursive = TRUE, copy.mode = FALSE) + pkg_dir <- file.path(src, "testInstDoc") + desc::desc_del("Config/build/clean-inst-doc", file = pkg_dir) + + answer <- 2L + local_mocked_bindings(is_interactive = function() TRUE) + local_mocked_bindings(menu = function(...) answer, .package = "utils") + build_quiet <- function() { + suppressMessages( + build(pkg_dir, dest_path = dest, quiet = TRUE, vignettes = TRUE) + ) + } + + # No keeps inst/doc and builds + pkg <- build_quiet() + expect_true(file.exists(pkg)) + pkg_files <- untar(pkg, list = TRUE) + expect_true("testInstDoc/inst/doc/keep.me" %in% pkg_files) + expect_true("testInstDoc/inst/doc/test.html" %in% pkg_files) + expect_true(file.exists(file.path(pkg_dir, "inst", "doc", "keep.me"))) + + # Cancelling the prompt also keeps inst/doc + answer <- 0L + pkg <- build_quiet() + expect_true("testInstDoc/inst/doc/keep.me" %in% untar(pkg, list = TRUE)) + + # Yes deletes it + answer <- 1L + pkg <- build_quiet() + pkg_files <- untar(pkg, list = TRUE) + expect_false("testInstDoc/inst/doc/keep.me" %in% pkg_files) + expect_true("testInstDoc/inst/doc/test.html" %in% pkg_files) +}) + test_that("bootstrap.R runs on build if present", { src <- withr::local_tempdir() dest <- withr::local_tempdir()