Skip to content

Commit

Permalink
Use cli for messaging (#2429)
Browse files Browse the repository at this point in the history
* Update active.R + test
* Update build-readme.R and tests
* Update find_active_file(). Includes wrapper around rstudioapi::isAvailable() that returns FALSE when testing.
* Update change_maintainer_email() + tests. Refactoring Maintainer check to make it more consistent
* use_lifecycle()
* Update package.R and tests. I realised that there was no way for `create` to work because `package_file()` will always error if it doesn't find a `DESCRIPTION`. So I deprecated it and removed the unused code
* Re-document
* Update release.R. I removed `email()` and `email_browser()` since as far as I can tell they are not used anywhere, and represent a very old style of CRAN submission. I don't know how to unit test this code so I did my best to carefully execute each change by hand.
* Improve package_file() error
* Fix R CMD check failure
* Simplify local_package_create(). And give it a name consistent with latest standards
* Update run-source.R + tests
* Fix remaining simple stop() calls
* Convert cli_alert_* to cli_inform() (except for vignette.R, which I'll do in #2425)

Closes #2364
  • Loading branch information
hadley authored May 24, 2022
1 parent c2637bd commit 7cc2bdf
Show file tree
Hide file tree
Showing 54 changed files with 455 additions and 326 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Imports:
DT (>= 0.21),
ellipsis (>= 0.3.2),
fs (>= 1.5.2),
lifecycle (>= 1.0.1),
memoise (>= 2.0.1),
miniUI (>= 0.1.1.1),
pkgbuild (>= 1.3.1),
Expand Down Expand Up @@ -51,7 +52,6 @@ Suggests:
gmailr (>= 1.0.1),
httr (>= 1.4.2),
knitr (>= 1.37),
lifecycle (>= 1.0.1),
lintr (>= 2.0.1),
MASS,
mockery (>= 0.4.3),
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import(fs)
importFrom(cli,cat_bullet)
importFrom(cli,cat_rule)
importFrom(ellipsis,check_dots_used)
importFrom(lifecycle,deprecated)
importFrom(memoise,memoise)
importFrom(pkgbuild,clean_dll)
importFrom(pkgbuild,find_rtools)
Expand Down
19 changes: 10 additions & 9 deletions R/active.R
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
find_active_file <- function(arg = "file") {
if (!rstudioapi::isAvailable()) {
stop("Argument `", arg, "` is missing, with no default", call. = FALSE)
find_active_file <- function(arg = "file", call = parent.frame()) {
if (!is_rstudio_running()) {
cli::cli_abort("Argument {.arg {arg}} is missing, with no default", call = call)
}
normalizePath(rstudioapi::getSourceEditorContext()$path)
}

find_test_file <- function(path) {
find_test_file <- function(path, call = parent.frame()) {
type <- test_file_type(path)
if (any(is.na(type))) {
rlang::abort(c(
"Don't know how to find tests associated with the active file:",
path[is.na(type)]
))
file <- path_file(path[is.na(type)])
cli::cli_abort(
"Don't know how to find tests associated with the active file {.file {file}}",
call = call
)
}

is_test <- type == "test"
path[!is_test] <- paste0("tests/testthat/test-", name_source(path[!is_test]), ".R")
path <- unique(path[file_exists(path)])

if (length(path) == 0) {
rlang::abort("No test files found")
cli::cli_abort("No test files found", call = call)
}
path
}
Expand Down
2 changes: 1 addition & 1 deletion R/build-manual.R
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ build_manual <- function(pkg = ".", path = NULL) {
), fail_on_status = TRUE, stderr = "2>&1", spinner = FALSE),
error = function(e) {
cat(e$stdout)
stop("Failed to build manual", call. = FALSE)
cli::cli_abort("Failed to build manual")
})

cat(msg$stdout)
Expand Down
9 changes: 4 additions & 5 deletions R/build-readme.R
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ build_rmd <- function(files, path = ".", output_options = list(), ..., quiet = T
cli::cli_abort("Can't find file{?s}: {.path {files[!ok]}}.")
}

cli::cli_alert_info("Installing {.pkg {pkg$package}} in temporary library")
cli::cli_inform(c(i = "Installing {.pkg {pkg$package}} in temporary library"))
withr::local_temp_libpaths()
install(pkg, upgrade = "never", reload = FALSE, quick = TRUE, quiet = quiet)

Expand All @@ -37,7 +37,7 @@ build_rmd <- function(files, path = ".", output_options = list(), ..., quiet = T


for (path in paths) {
cli::cli_alert_info("Building {.path {path}}")
cli::cli_inform(c(i = "Building {.path {path}}"))
callr::r_safe(
function(...) rmarkdown::render(...),
args = list(input = path, ..., output_options = output_options, quiet = quiet),
Expand All @@ -58,11 +58,10 @@ build_readme <- function(path = ".", quiet = TRUE, ...) {
readme_path <- path_abs(dir_ls(pkg$path, ignore.case = TRUE, regexp = "(inst/)?readme[.]rmd", recurse = 1, type = "file"))

if (length(readme_path) == 0) {
rlang::abort("Can't find a 'README.Rmd' or 'inst/README.Rmd' file.")
cli::cli_abort("Can't find {.file README.Rmd} or {.file inst/README.Rmd}.")
}

if (length(readme_path) > 1) {
rlang::abort("Can't have both a 'README.Rmd' and 'inst/README.Rmd' file.")
cli::cli_abort("Can't have both {.file README.Rmd} and {.file inst/README.Rmd}.")
}

build_rmd(readme_path, path = path, quiet = quiet, ...)
Expand Down
4 changes: 2 additions & 2 deletions R/check-devtools.R
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ check_status <- function(status, name, warning) {
cat(" OK\n")
} else {
cat("\n")
cli::cli_alert_danger("WARNING: {warning}")
cli::cli_inform(c(x = "WARNING: {warning}"))
},
error = function(e) {
cat("\n")
cli::cli_alert_danger("ERROR: {conditionMessage(e)}")
cli::cli_inform(c(x = "ERROR: {conditionMessage(e)}"))
FALSE
}
)
Expand Down
4 changes: 2 additions & 2 deletions R/check-doc.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ check_man <- function(pkg = ".") {
old <- options(warn = -1)
on.exit(options(old))

cli::cli_alert_info("Checking documentation...")
cli::cli_inform(c(i = "Checking documentation..."))

check_Rd_contents <- if (getRversion() < "4.1") {
asNamespace("tools")$.check_Rd_contents
Expand All @@ -42,7 +42,7 @@ check_man <- function(pkg = ".") {
)

if (ok) {
cli::cli_alert_success("No issues detected")
cli::cli_inform(c(v = "No issues detected"))
}

invisible()
Expand Down
13 changes: 7 additions & 6 deletions R/check-mac.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ check_mac_release <- function(pkg = ".", dep_pkgs = character(), args = NULL, ma
pkg <- as.package(pkg)

if (!quiet) {
cli::cli_alert_info(
cli::cli_inform(c(
"Building macOS version of {.pkg {pkg$package}} ({pkg$version})",
"with https://mac.r-project.org/macbuilder/submit.html."
)
i = "Using https://mac.r-project.org/macbuilder/submit.html."
))
}

built_path <- pkgbuild::build(pkg$path, tempdir(),
Expand Down Expand Up @@ -62,9 +62,10 @@ check_mac_release <- function(pkg = ".", dep_pkgs = character(), args = NULL, ma
if (!quiet) {
time <- strftime(Sys.time() + 10 * 60, "%I:%M %p")

cli::cli_alert_success(
"[{Sys.Date()}] Check {.url {response_url}} for the results in 5-10 mins (~{time})."
)
cli::cat_rule(col = "cyan")
cli::cli_inform(c(
i = "Check {.url {response_url}} the results in 5-10 mins (~{time})."
))
}

invisible(response_url)
Expand Down
44 changes: 26 additions & 18 deletions R/check-win.R
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,21 @@ check_win <- function(pkg = ".", version = c("R-devel", "R-release", "R-oldrelea
file_copy(desc_file, backup)
on.exit(file_move(backup, desc_file), add = TRUE)

change_maintainer_email(desc_file, email)
change_maintainer_email(desc_file, email, call = parent.frame())

pkg <- as.package(pkg$path)
}

version <- match.arg(version, several.ok = TRUE)

if (!quiet) {
cli::cli_alert_info(
cli::cli_inform(c(
"Building windows version of {.pkg {pkg$package}} ({pkg$version})",
" for {paste(version, collapse = ', ')} with win-builder.r-project.org."
)
i = "Using {paste(version, collapse = ', ')} with win-builder.r-project.org."
))

email <- cli::style_bold(maintainer(pkg)$email)
if (interactive() && yesno("Email results to ", email, "?")) {
email <- maintainer(pkg)$email
if (interactive() && yesno("Email results to {.strong {email}}?")) {
return(invisible())
}
}
Expand All @@ -96,32 +96,40 @@ check_win <- function(pkg = ".", version = c("R-devel", "R-release", "R-oldrelea
time <- strftime(Sys.time() + 30 * 60, "%I:%M %p")
email <- maintainer(pkg)$email

cli::cli_alert_success(
"[{Sys.Date()}] Check <{.email {email}}> for a link to results in 15-30 mins (~{time})."
)
cli::cat_rule(col = "cyan")
cli::cli_inform(c(
i = "Check <{.email {email}}> for the results in 15-30 mins (~{time})."
))
}

invisible()
}

change_maintainer_email <- function(desc, email) {
desc <- desc::desc(file = desc)
change_maintainer_email <- function(path, email, call = parent.frame()) {
desc <- desc::desc(file = path)

if (!desc$has_fields("Authors@R")) {
stop("DESCRIPTION must use `Authors@R` field to change the maintainer email", call. = FALSE)
cli::cli_abort(
"DESCRIPTION must use {.field Authors@R} field when changing {.arg email}",
call = call
)
}
if (desc$has_fields("Maintainer")) {
cli::cli_abort(
"DESCRIPTION can't use {.field Maintainer} field when changing {.arg email}",
call = call
)
}

aut <- desc$get_authors()
roles <- aut$role
## Broken person() API, vector for 1 author, list otherwise...
if (!is.list(roles)) roles <- list(roles)
if (!is.list(roles)) {
roles <- list(roles)
}
is_maintainer <- vapply(roles, function(r) all("cre" %in% r), logical(1))
aut[is_maintainer]$email <- email

desc$set_authors(aut)
## Check if the email is actually changed before we actually send the email
if(!grepl(email, desc$get_maintainer())){
stop("Changing maintainer email failed. Possible reason is using both Authors@R and Maintainer fields in the DESCRIPTION file.", call. = FALSE)
}

desc$write()
}
Expand Down
13 changes: 6 additions & 7 deletions R/check.R
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ check <- function(pkg = ".",
save_all()

if (!missing(cleanup)) {
warning("`cleanup` is deprecated", call. = FALSE)
lifecycle::deprecate_stop("1.11.0", "lifecycle::check(cleanup = )")
}

if (missing(error_on) && !interactive()) {
Expand Down Expand Up @@ -145,12 +145,11 @@ can_document <- function(pkg) {
installed <- packageVersion("roxygen2")
if (required != installed) {
cli::cli_rule()
cli::cli_alert_info(
"Installed roxygen2 version ({installed}) doesn't match required version ({required})"
)
cli::cli_alert_danger("check() will not re-document this package")
cli::cli_inform(c(
i = "Installed roxygen2 version ({installed}) doesn't match required ({required})",
x = "{.fun check} will not re-document this package"
))
cli::cli_rule()

FALSE
} else {
TRUE
Expand Down Expand Up @@ -199,7 +198,7 @@ check_built <- function(path = NULL, cran = TRUE,
}

if (manual && !pkgbuild::has_latex()) {
cli::cli_alert_danger("pdflatex not found! Not building PDF manual or vignettes")
cli::cli_inform(c(x = "pdflatex not found! Not building PDF manual"))
manual <- FALSE
}

Expand Down
14 changes: 7 additions & 7 deletions R/dev-mode.R
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,25 @@ dev_mode <- local({
dir_create(path)
}
if (!file_exists(path)) {
stop("Failed to create ", path, call. = FALSE)
cli::cli_abort("Failed to create {.path {path}}")
}

if (!is_library(path)) {
warning(path, " does not appear to be a library. ",
"Are sure you specified the correct directory?",
call. = FALSE
)
cli::cli_warn(c(
"{.path {path}} does not appear to be a library.",
"Are sure you specified the correct directory?"
))
}

cli::cli_alert_success("Dev mode: ON")
cli::cli_inform(c(v = "Dev mode: ON"))
options(dev_path = path)

if (is.null(.prompt)) .prompt <<- getOption("prompt")
options(prompt = paste("d> "))

.libPaths(c(path, lib_paths))
} else {
cli::cli_alert_success("Dev mode: OFF")
cli::cli_inform(c(v = "Dev mode: OFF"))
options(dev_path = NULL)

if (!is.null(.prompt)) options(prompt = .prompt)
Expand Down
36 changes: 36 additions & 0 deletions R/devtools-package.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#' @section Package options:
#'
#' Devtools uses the following [options()] to configure behaviour:
#'
#' \itemize{
#' \item `devtools.path`: path to use for [dev_mode()]
#'
#' \item `devtools.name`: your name, used when signing draft
#' emails.
#'
#' \item `devtools.install.args`: a string giving extra arguments passed
#' to `R CMD install` by [install()].
#'
#' \item `devtools.desc.author`: a string providing a default Authors@@R
#' string to be used in new \file{DESCRIPTION}s. Should be a R code, and
#' look like `"Hadley Wickham <h.wickham@@gmail.com> [aut, cre]"`. See
#' [utils::as.person()] for more details.
#'
#' \item `devtools.desc.license`: a default license string to use for
#' new packages.
#'
#' \item `devtools.desc.suggests`: a character vector listing packages to
#' to add to suggests by defaults for new packages.
#
#' \item `devtools.desc`: a named list listing any other
#' extra options to add to \file{DESCRIPTION}
#'
#' }
#' @docType package
#' @keywords internal
"_PACKAGE"

## usethis namespace: start
#' @importFrom lifecycle deprecated
## usethis namespace: end
NULL
2 changes: 1 addition & 1 deletion R/document.R
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
document <- function(pkg = ".", roclets = NULL, quiet = FALSE) {
pkg <- as.package(pkg)
if (!isTRUE(quiet)) {
cli::cli_alert_info("Updating {.pkg {pkg$package}} documentation")
cli::cli_inform(c(i = "Updating {.pkg {pkg$package}} documentation"))
}

save_all()
Expand Down
2 changes: 1 addition & 1 deletion R/lint.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ lint <- function(pkg = ".", cache = TRUE, ...) {
rlang::check_installed("lintr")
pkg <- as.package(pkg)

cli::cli_alert_info("Linting {.pkg {pkg$package}}")
cli::cli_inform(c(i = "Linting {.pkg {pkg$package}}"))

check_dots_used(action = getOption("devtools.ellipsis_action", rlang::warn))

Expand Down
Loading

0 comments on commit 7cc2bdf

Please sign in to comment.