Keep inst/doc and continue when answering No to the clean_doc prompt

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).

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)
     }
   }
 

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)
 }

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()