From d2c92189cbedfbc325585e2187f991c07fef2779 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 10 Oct 2022 11:47:10 +0200 Subject: [PATCH 1/6] cpp_register(quiet=) set to TRUE on non-interactive session (incl testthat). --- DESCRIPTION | 2 +- R/register.R | 4 ++-- R/source.R | 2 +- man/cpp_register.Rd | 2 +- tests/testthat/helper.R | 2 +- tests/testthat/test-register.R | 10 ++++++++-- 6 files changed, 14 insertions(+), 8 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 15fdb8d6..fa80bf47 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -58,7 +58,7 @@ Config/Needs/cpp11/cpp_register: vctrs Encoding: UTF-8 Roxygen: list(markdown = TRUE) -RoxygenNote: 7.1.2 +RoxygenNote: 7.2.1 SystemRequirements: C++11 Remotes: r-lib/testthat@ad8b726d1734c59ed08ec7d11df4a3d76d8e69df diff --git a/R/register.R b/R/register.R index ac99ae1f..65727f91 100644 --- a/R/register.R +++ b/R/register.R @@ -34,7 +34,7 @@ #' #' # cleanup #' unlink(dir, recursive = TRUE) -cpp_register <- function(path = ".", quiet = FALSE) { +cpp_register <- function(path = ".", quiet = !is_interactive()) { stop_unless_installed(get_cpp_register_needs()) r_path <- file.path(path, "R", "cpp11.R") @@ -138,7 +138,7 @@ cpp_register <- function(path = ".", quiet = FALSE) { utils::globalVariables(c("name", "return_type", "line", "decoration", "context", ".", "functions", "res")) -get_registered_functions <- function(decorations, tag, quiet = FALSE) { +get_registered_functions <- function(decorations, tag, quiet = !is_interactive()) { if (NROW(decorations) == 0) { return(tibble::tibble(file = character(), line = integer(), decoration = character(), params = list(), context = list(), name = character(), return_type = character(), args = list())) } diff --git a/R/source.R b/R/source.R index d2820ccd..b8c8411d 100644 --- a/R/source.R +++ b/R/source.R @@ -110,7 +110,7 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu check_valid_attributes(all_decorations, file = orig_file_path) cli_suppress( - funs <- get_registered_functions(all_decorations, "cpp11::register") + funs <- get_registered_functions(all_decorations, "cpp11::register", quiet = quiet) ) cpp_functions_definitions <- generate_cpp_functions(funs, package = package) diff --git a/man/cpp_register.Rd b/man/cpp_register.Rd index 3316cab4..8b1628ed 100644 --- a/man/cpp_register.Rd +++ b/man/cpp_register.Rd @@ -4,7 +4,7 @@ \alias{cpp_register} \title{Generates wrappers for registered C++ functions} \usage{ -cpp_register(path = ".", quiet = FALSE) +cpp_register(path = ".", quiet = !is_interactive()) } \arguments{ \item{path}{The path to the package root directory} diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index 2f41fa79..eded02b5 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -14,7 +14,7 @@ pkg_path <- function(pkg) { get_funs <- function(path) { all_decorations <- decor::cpp_decorations(path, is_attribute = TRUE) - get_registered_functions(all_decorations, "cpp11::register", quiet = FALSE) + get_registered_functions(all_decorations, "cpp11::register", quiet = TRUE) } get_package_name <- function(path) { diff --git a/tests/testthat/test-register.R b/tests/testthat/test-register.R index d9e890a5..2ef35a22 100644 --- a/tests/testthat/test-register.R +++ b/tests/testthat/test-register.R @@ -614,12 +614,18 @@ extern \"C\" attribute_visible void R_init_testPkg(DllInfo* dll){ }) - it("can be run with messages, by default", { + it("can be run with messages", { pkg <- local_package() p <- pkg_path(pkg) dir.create(file.path(p, "src")) file.copy(test_path("single.cpp"), file.path(p, "src", "single.cpp")) - expect_message(cpp_register(p), "1 functions decorated with [[cpp11::register]]", fixed = TRUE) + suppressMessages( + # cpp_register() sends 3 messages, but expect_message() only checks + # for one, so suppress the other ones, so that they don't appear in the + # testthat output + expect_message( + cpp_register(p, quiet = FALSE), "1 functions decorated with [[cpp11::register]]", fixed = TRUE) + ) }) it("includes pkg_types.h if included in src", { From b55cc4405ccd6738d0d7c9d90a90907688824760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 10 Oct 2022 12:55:49 +0200 Subject: [PATCH 2/6] news --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 2ad5492d..4b3840cf 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,8 @@ * `cpp_source()` errors on non-existent file (#261). +* `cpp_register()` is quiet by default when R is non interactive. + # cpp11 0.4.2 * Romain François is now the maintainer. From 542c23725481d4e6610c417d5c9b0f3f558f7263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 10 Oct 2022 17:53:12 +0200 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Davis Vaughan --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 4b3840cf..132d1d54 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,7 +6,7 @@ * `cpp_source()` errors on non-existent file (#261). -* `cpp_register()` is quiet by default when R is non interactive. +* `cpp_register()` is quiet by default when R is non interactive (#289). # cpp11 0.4.2 From ed3fff19320e0393bbe7a0259135a8992af34969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 10 Oct 2022 18:26:33 +0200 Subject: [PATCH 4/6] remove redundant cli_suppress() call --- R/source.R | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/R/source.R b/R/source.R index b8c8411d..fc9124d5 100644 --- a/R/source.R +++ b/R/source.R @@ -109,9 +109,7 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu #provide original path for error messages check_valid_attributes(all_decorations, file = orig_file_path) - cli_suppress( - funs <- get_registered_functions(all_decorations, "cpp11::register", quiet = quiet) - ) + funs <- get_registered_functions(all_decorations, "cpp11::register", quiet = quiet) cpp_functions_definitions <- generate_cpp_functions(funs, package = package) cpp_path <- file.path(dirname(new_file_path), "cpp11.cpp") From 224ba19592e870c951099c759c6fabfff228c9e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 10 Oct 2022 18:31:50 +0200 Subject: [PATCH 5/6] simplify with expect_snapshot() --- tests/testthat/_snaps/register.md | 9 +++++++++ tests/testthat/test-register.R | 9 +++------ 2 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 tests/testthat/_snaps/register.md diff --git a/tests/testthat/_snaps/register.md b/tests/testthat/_snaps/register.md new file mode 100644 index 00000000..8b72a67e --- /dev/null +++ b/tests/testthat/_snaps/register.md @@ -0,0 +1,9 @@ +# cpp_register: can be run with messages + + Code + cpp_register(p, quiet = FALSE) + Message + ℹ 1 functions decorated with [[cpp11::register]] + ✔ generated file ']8;;file:///Users/romainfrancois/git/cpp11/tests/testthat/cpp11.Rcpp11.R]8;;' + ✔ generated file ']8;;file:///Users/romainfrancois/git/cpp11/tests/testthat/cpp11.cppcpp11.cpp]8;;' + diff --git a/tests/testthat/test-register.R b/tests/testthat/test-register.R index 2ef35a22..e26af279 100644 --- a/tests/testthat/test-register.R +++ b/tests/testthat/test-register.R @@ -619,12 +619,9 @@ extern \"C\" attribute_visible void R_init_testPkg(DllInfo* dll){ p <- pkg_path(pkg) dir.create(file.path(p, "src")) file.copy(test_path("single.cpp"), file.path(p, "src", "single.cpp")) - suppressMessages( - # cpp_register() sends 3 messages, but expect_message() only checks - # for one, so suppress the other ones, so that they don't appear in the - # testthat output - expect_message( - cpp_register(p, quiet = FALSE), "1 functions decorated with [[cpp11::register]]", fixed = TRUE) + + expect_snapshot( + cpp_register(p, quiet = FALSE) ) }) From 0284dd47289c14d2aa29fc16f1e2ab17431ba61a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 10 Oct 2022 18:35:17 +0200 Subject: [PATCH 6/6] remove cli_suppress() function as it's no longer needed. --- R/utils.R | 9 --------- tests/testthat/test-utils.R | 8 -------- 2 files changed, 17 deletions(-) diff --git a/R/utils.R b/R/utils.R index e84e48fa..0dddfaf9 100644 --- a/R/utils.R +++ b/R/utils.R @@ -1,12 +1,3 @@ -cli_suppress <- function(expr) { - withCallingHandlers( - expr, - cli_message = function(c) { - invokeRestart("cli_message_handled") - } - ) -} - glue_collapse_data <- function(data, ..., sep = ", ", last = "") { res <- glue::glue_collapse(glue::glue_data(data, ...), sep = sep, last = last) if (length(res) == 0) { diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index ce4430dc..92932c20 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -1,11 +1,3 @@ -describe("cli_suppress", { - it("suppresses cli outputs", { - f <- function() cli::cli_text("hi") - expect_message(f(), "hi") - expect_message(cli_suppress(f()), NA) - }) -}) - describe("glue_collapse_data", { it("works with empty inputs", { expect_equal(