From e87eda12a2194e477111da224f8e3ed61174c3f0 Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Fri, 19 Dec 2025 19:37:15 -0500 Subject: [PATCH 01/10] feat: improve log capture during evaluation --- DESCRIPTION | 5 +- NAMESPACE | 2 + R/class_pkg.R | 82 ++++++++++++++++++++++------ R/class_resource.R | 12 ++-- R/data_coverage.R | 10 ++-- R/data_desc.R | 23 +++++++- R/data_r_cmd_check.R | 26 ++------- R/data_vignettes.R | 11 ++-- R/data_web_html.R | 7 +-- R/generic_pkg_data_derive.R | 47 ++++++++++++++++ R/options.R | 23 +++++--- R/utils_dcf.R | 14 +++-- R/utils_evaluate.R | 32 +++++++++++ R/utils_tmp.R | 8 ++- man/capture_pkg_data_derive.Rd | 24 ++++++++ man/options.Rd | 10 +++- man/options_params.Rd | 6 +- tests/testthat/test-convert-to-pkg.R | 47 +++++++++++++++- tests/testthat/test-derive-logs.R | 35 ++++++++++++ 19 files changed, 344 insertions(+), 80 deletions(-) create mode 100644 R/utils_evaluate.R create mode 100644 man/capture_pkg_data_derive.Rd create mode 100644 tests/testthat/test-derive-logs.R diff --git a/DESCRIPTION b/DESCRIPTION index 0fb2f50..aac0453 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -32,6 +32,7 @@ BugReports: Imports: cli, desc, + evaluate, options, S7, tools, @@ -43,6 +44,7 @@ Imports: Suggests: covr, rcmdcheck, + htmltools, igraph, knitr, rmarkdown, @@ -90,13 +92,14 @@ Collate: 'options.R' 'package.R' 'utils_backports.R' + 'utils_evaluate.R' 'utils_rand.R' 'utils_rstudio.R' 'utils_tmp.R' 'zzz.R' Encoding: UTF-8 Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.2 +RoxygenNote: 7.3.3 Depends: R (>= 3.5) LazyData: true diff --git a/NAMESPACE b/NAMESPACE index b68fdba..dbd274e 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -5,6 +5,7 @@ S3method(".DollarNames","val.meter::pkg") S3method("[","val.meter::pkg") S3method("[[","val.meter::pkg") S3method(as.data.frame,list_of_pkg) +S3method(format,evaluate_evaluation) S3method(format,val_meter_error) S3method(print,val_meter_error) export(class_metric_data_frame) @@ -55,6 +56,7 @@ importFrom(tools,getVignetteInfo) importFrom(tools,toRd) importFrom(utils,.DollarNames) importFrom(utils,available.packages) +importFrom(utils,capture.output) importFrom(utils,download.packages) importFrom(utils,getCRANmirrors) importFrom(utils,head) diff --git a/R/class_pkg.R b/R/class_pkg.R index f5d7658..44fa0da 100644 --- a/R/class_pkg.R +++ b/R/class_pkg.R @@ -16,6 +16,11 @@ pkg <- class_pkg <- new_class( # necessary data dependencies to be evaluated. data = class_environment, + # logs (not user-facing) + # A mutable environment, stores output logs captured using the `evaluate` + # package. Should contain an entry for each value in `@data`. + logs = class_environment, + #' @param resource [`resource`] (often a [`multi_resource`]), providing the #' resources to be used for deriving packages data. If a #' [`multi_resource`], the order of resources determines the precedence of @@ -62,6 +67,7 @@ pkg <- class_pkg <- new_class( new_object( .parent = S7::S7_object(), data = new.env(parent = emptyenv()), + logs = new.env(parent = emptyenv()), metrics = list(), resource = resource, permissions = policy@permissions @@ -69,6 +75,17 @@ pkg <- class_pkg <- new_class( } ) +method(convert, list(class_character, class_pkg)) <- + function(from, to, ...) { + if (endsWith(tolower(from), ".rds")) { + convert(readRDS(from), class_pkg) + } else if (grepl("\\bPackage:", from[[1L]])) { + pkg_from_dcf(from, ...) + } else { + pkg(from, ...) + } + } + #' Generate Random Package(s) #' #' Create a package object to simulate metric derivation. When generating a @@ -213,12 +230,13 @@ get_pkg_data <- function(x, name, ..., .raise = .state$raise) { assert_permissions(required_permissions, x@permissions) assert_suggests(required_suggests) - data <- pkg_data_derive(pkg = x, field = name, ...) + capture <- capture_pkg_data_derive(pkg = x, field = name, ...) + x@logs[[name]] <- capture$logs if (!identical(info@data_class, class_any)) { - data <- convert(data, info@data_class) + capture$data <- convert(capture$data, info@data_class) } - data + capture$data }, error = function(e, ...) { convert(e, class_val_meter_error, field = name) @@ -382,31 +400,59 @@ as.data.frame.list_of_pkg <- function(x, ...) { } #' @include utils_dcf.R -method(from_dcf, list(class_character, class_pkg)) <- - function(x, to, ...) { - dcf <- from_dcf(x, class_any) +method(convert, list(class_list, class_pkg)) <- + function(from, to, ...) { resource <- unknown_resource( - package = dcf[[1, "Package"]], - version = dcf[[1, "Version"]], - md5 = if ("MD5sum" %in% colnames(dcf)) { - dcf[[1, "MD5sum"]] - } else { - NA_character_ - } + package = from$name %||% from$Package, + version = from$version %||% from$Version, + md5 = from$MD5sum %||% NA_character_ ) data <- new.env(parent = emptyenv()) + for (name in names(from)) { + # recover gracefully from unknown fieldnames + info <- tryCatch(pkg_data_info(name), error = function(e) NULL) + if (is.null(info)) { + next + } + + data[[name]] <- metric_coerce(from[[name]], info@data_class) + } + + pkg <- pkg(resource) + pkg@data <- data + + pkg + } + +#' @include utils_dcf.R +method(from_dcf, list(class_character, class_pkg)) <- + function(x, to, ...) { + dcf <- from_dcf(x, class_any) + + data <- list() + data$name <- dcf[[1, "Package"]] + data$version <- dcf[[1, "Version"]] + data$md5 <- if ("MD5sum" %in% colnames(dcf)) { + dcf[[1, "MD5sum"]] + } else { + NA_character_ + } + prefix <- "Metric/" for (name in colnames(dcf)[startsWith(colnames(dcf), prefix)]) { field <- sub(prefix, "", name) - info <- pkg_data_info(field) + + # recover gracefully from unknown fieldnames + info <- tryCatch(pkg_data_info(field), error = function(e) NULL) + if (is.null(info)) { + next + } + val <- dcf[[1, name]] val <- metric_coerce(val, info@data_class) data[[field]] <- val } - pkg <- pkg(resource) - pkg@data <- data - - pkg + convert(data, class_pkg) } diff --git a/R/class_resource.R b/R/class_resource.R index 55effd7..c6333d2 100644 --- a/R/class_resource.R +++ b/R/class_resource.R @@ -284,9 +284,11 @@ method(convert, list(class_character, class_resource)) <- add_resource <- function(resource) { resource_type_name <- class_desc(S7::S7_class(resource)) idx <- match(resource_type_name, all_resource_type_names) + if (is.na(idx) || !is.null(resources[[idx]])) { return() } + resources[[idx]] <<- resource idx } @@ -317,8 +319,8 @@ method(convert, list(class_character, class_resource)) <- # iterate over other allowed resource types for (to_idx in seq_along(all_resource_types)) { # that are not yet populated with a known resource - to_resource <- resources[[to_idx]] - if (!is.null(to_resource)) { + existing_resource <- resources[[to_idx]] + if (!is.null(existing_resource)) { next } @@ -336,7 +338,7 @@ method(convert, list(class_character, class_resource)) <- # special handling for error conditions used to test discovery in tests if (inherits(result, "test_suite_signal")) { stop(result) - } else if (inherits(result, "error")) { + } else if (is.null(result) || inherits(result, "error")) { next } @@ -589,7 +591,9 @@ method(convert, list(class_local_source_resource, class_install_resource)) <- method(convert, list(class_resource, class_unknown_resource)) <- function(from, to, ...) { - set_props(to(), props(from, names(class_unknown_resource@properties))) + out <- to() + props(out) <- props(from, prop_names(out)) + out } method(to_dcf, class_resource) <- function(x, ...) { diff --git a/R/data_coverage.R b/R/data_coverage.R index 9911525..09baa48 100644 --- a/R/data_coverage.R +++ b/R/data_coverage.R @@ -9,8 +9,10 @@ impl_data( impl_data( "covr_coverage", for_resource = local_source_resource, - function(pkg, resource, field, ..., quiet = opt("quiet")) { - covr::package_coverage(resource@path, type = "tests", quiet = quiet) + function(pkg, resource, field, ...) { + # package installs use `system2()` whose output cannot be captured by sink() + # so we just execute quietly + covr::package_coverage(resource@path, type = "tests", quiet = TRUE) } ) @@ -23,7 +25,7 @@ impl_data( "The fraction of expressions of package code that are evaluated by any ", "test" ), - function(pkg, resource, field, ..., quiet = opt("quiet")) { + function(pkg, resource, field, ...) { tally <- covr::tally_coverage(pkg$covr_coverage, by = "expression") mean(tally$value > 0) } @@ -45,7 +47,7 @@ impl_data( description = paste0( "The fraction of lines of package code that are evaluated by any test" ), - function(pkg, resource, field, ..., quiet = opt("quiet")) { + function(pkg, resource, field, ...) { tally <- covr::tally_coverage(pkg$covr_coverage, by = "line") mean(tally$value > 0) } diff --git a/R/data_desc.R b/R/data_desc.R index d060a7f..874c175 100644 --- a/R/data_desc.R +++ b/R/data_desc.R @@ -14,6 +14,7 @@ impl_data( "name", title = "Package name", class = class_character, + for_resource = new_union(source_code_resource, install_resource), function(pkg, resource, field, ...) { pkg$desc$get_field("Package") } @@ -21,7 +22,7 @@ impl_data( impl_data( "name", - for_resource = repo_resource, + for_resource = class_resource, function(pkg, resource, field, ...) { resource@package } @@ -30,6 +31,7 @@ impl_data( impl_data( "version", class = class_character, + for_resource = new_union(source_code_resource, install_resource), function(pkg, resource, field, ...) { pkg$desc$get_field("Version") } @@ -37,12 +39,29 @@ impl_data( impl_data( "version", - for_resource = repo_resource, + for_resource = class_resource, function(pkg, resource, field, ...) { resource@version } ) +impl_data( + "md5", + class = class_character, + for_resource = new_union(source_code_resource, install_resource), + function(pkg, resource, field, ...) { + pkg$desc$get_field("MD5sum") + } +) + +impl_data( + "md5", + for_resource = class_resource, + function(pkg, resource, field, ...) { + resource@md5 + } +) + impl_data( "dependency_count", class = class_integer, diff --git a/R/data_r_cmd_check.R b/R/data_r_cmd_check.R index 748b7cf..8a799ee 100644 --- a/R/data_r_cmd_check.R +++ b/R/data_r_cmd_check.R @@ -14,26 +14,12 @@ impl_data( local_source_resource, source_archive_resource ), - function(pkg, resource, field, ..., quiet = opt("quiet")) { - # suppress messages to avoid stdout output from subprocess - # (eg warnings about latex availability not suppressed by rcmdcheck) - - wrapper <- if (quiet) { - function(...) capture.output(..., type = "message") - } else { - identity - } - - wrapper({ - result <- rcmdcheck::rcmdcheck( - resource@path, - quiet = quiet, - error_on = "never", - build_args = "--no-manual" - ) - }) - - result + function(pkg, resource, field, ...) { + rcmdcheck::rcmdcheck( + resource@path, + error_on = "never", + build_args = "--no-manual" + ) } ) diff --git a/R/data_vignettes.R b/R/data_vignettes.R index 967401e..45f26ec 100644 --- a/R/data_vignettes.R +++ b/R/data_vignettes.R @@ -31,12 +31,11 @@ impl_data( return(0) } - nodes |> - xml2::xml_attr("href") |> - basename() |> - tools::file_path_sans_ext() |> - unique() |> - length() + paths <- xml2::xml_attr(nodes, "href") + filenames <- basename(paths) + filestems <- tools::file_path_sans_ext(filenames) + + length(unique(filestems)) } ) diff --git a/R/data_web_html.R b/R/data_web_html.R index 9ce4249..fd378c8 100644 --- a/R/data_web_html.R +++ b/R/data_web_html.R @@ -20,9 +20,8 @@ impl_data( for_resource = cran_repo_resource, permissions = "network", function(pkg, resource, field, ...) { - pkg$web_url |> - httr2::request() |> - httr2::req_perform() |> - httr2::resp_body_html() + req <- httr2::request(pkg$web_url) + resp <- httr2::req_perform(req) + httr2::resp_body_html(resp) } ) diff --git a/R/generic_pkg_data_derive.R b/R/generic_pkg_data_derive.R index 1440ad2..b72bbc3 100644 --- a/R/generic_pkg_data_derive.R +++ b/R/generic_pkg_data_derive.R @@ -24,6 +24,53 @@ #' @export pkg_data_derive <- new_generic("pkg_data_derive", c("pkg", "resource", "field")) +#' Derive data and capture output +#' +#' Uses `evaluate::evaluate` to capture execution logs. +#' +#' @inheritParams pkg_data_derive +#' +#' @keywords internal +capture_pkg_data_derive <- function( + pkg, + resource, + field, + ..., + quiet = opt("quiet") +) { + # build a prettier call that will be output by evaluate() when not quiet + x <- pkg + pkg <- list(function() pkg_data_derive(pkg = x, field = field)) + names(pkg) <- field + evaluate_fn <- function() {} + body(evaluate_fn) <- as.call(list(call("$", as.symbol("pkg"), field))) + + # format output for a standard console width; force capture of ansi + original_opts <- options( + width = 80L, + crayon.enabled = TRUE, + cli.ansi = TRUE, + cli.dynamic = FALSE, + cli.num_colors = 256L + ) + + on.exit(options(original_opts)) + + capture <- evaluate::evaluate( + evaluate_fn, + stop_on_error = 1L, + debug = !isTRUE(quiet), + output_handler = evaluate::new_output_handler(value = identity) + ) + + list( + # omit code echo and return value + logs = capture[-c(1, length(capture))], + # just the return value + data = capture[[length(capture)]] + ) +} + #' Derive by Field Name #' #' When a field is provided by name, create an empty S3 object using the field diff --git a/R/options.R b/R/options.R index b38c869..9776494 100644 --- a/R/options.R +++ b/R/options.R @@ -9,19 +9,28 @@ NULL #' @include utils_cli.R define_options( - fmt("Set the default `{packageName()}` policies, specifying how package + fmt( + "Set the default `{packageName()}` policies, specifying how package resources will be discovered and what permissions are granted when - calculating metrics."), + calculating metrics." + ), policy = policy(), - fmt("Set the default `{packageName()}` tags policy. Tags characterize the + fmt( + "Set the default `{packageName()}` tags policy. Tags characterize the types of information various metrics contain. For more details, see - [`tags()`]."), + [`tags()`]." + ), tags = tags(TRUE), - fmt("Logging directory where artifacts will be stored. Defaults to a temporary - directory."), - logs = ns_tmp_root(), + fmt("Whether output should be captured during the evaluation of metrics."), + logs = TRUE, + + fmt( + "Directory where artifacts will be stored. This includes installation logs, + package source code and temporary libraries used while evaluating packages." + ), + artifacts = ns_tmp_root(), "Silences console output during evaluation. This applies when pulling package resources (such as download and installation output) and executing code diff --git a/R/utils_dcf.R b/R/utils_dcf.R index f53eab3..f9a9428 100644 --- a/R/utils_dcf.R +++ b/R/utils_dcf.R @@ -42,13 +42,15 @@ method(from_dcf, list(class_character, S7::new_S3_class("S7_class"))) <- method(from_dcf, list(class_character, class_any)) <- function( - x, - to, - ..., - eval = TRUE, - fragment = c("key-values", "key-value", "value")) { + x, + to, + ..., + eval = TRUE, + fragment = c("key-values", "key-value", "value") + ) { fragment <- match.arg(fragment) - switch(fragment, + switch( + fragment, "key-values" = { con <- textConnection(x) x <- as.data.frame(read.dcf(con, all = TRUE)) diff --git a/R/utils_evaluate.R b/R/utils_evaluate.R new file mode 100644 index 0000000..16410fc --- /dev/null +++ b/R/utils_evaluate.R @@ -0,0 +1,32 @@ +#' @importFrom utils capture.output +#' @export +format.evaluate_evaluation <- function( + x, + ..., + style = c("text", "ansi", "html") +) { + style <- match.arg(style) + + if ( + identical(style, "html") && !requireNamespace("htmltools", quietly = TRUE) + ) { + stop( + "html formatting of evaluation logs requires suggested package ", + "`htmltools`" + ) + } + + out <- utils::capture.output(evaluate::replay(x)) + switch( + style, + text = paste(cli::ansi_strip(out), collapse = "\n"), + ansi = paste(out, collapse = "\n"), + html = { + html <- cli::ansi_html(paste(out, collapse = "\n")) + htmltools::tags$div( + style = cli::ansi_html_style(colors = 8L), + htmltools::tags$pre(htmltools::HTML(html)) + ) + } + ) +} diff --git a/R/utils_tmp.R b/R/utils_tmp.R index f66cfcd..7f51b50 100644 --- a/R/utils_tmp.R +++ b/R/utils_tmp.R @@ -9,15 +9,17 @@ ns_tmp_root <- function() { dir_create(file.path(tempdir(), packageName())) } -pkg_dir <- function(pkg, ..., .root = opt("logs")) { +pkg_dir <- function(pkg, ..., .root = opt("artifacts")) { dir_create(file.path(.root, "pkg", pkg$name, ...)) } -resource_dir <- function(x, ..., .id = x@id, .root = opt("logs")) { +resource_dir <- function(x, ..., .id = x@id, .root = opt("artifacts")) { dir_create(file.path(.root, "rsrc", .id, ...)) } dir_create <- function(path) { - if (!dir.exists(path)) dir.create(path, recursive = TRUE) + if (!dir.exists(path)) { + dir.create(path, recursive = TRUE) + } path } diff --git a/man/capture_pkg_data_derive.Rd b/man/capture_pkg_data_derive.Rd new file mode 100644 index 0000000..fc8c503 --- /dev/null +++ b/man/capture_pkg_data_derive.Rd @@ -0,0 +1,24 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/generic_pkg_data_derive.R +\name{capture_pkg_data_derive} +\alias{capture_pkg_data_derive} +\title{Derive data and capture output} +\usage{ +capture_pkg_data_derive(pkg, resource, field, ..., quiet = opt("quiet")) +} +\arguments{ +\item{pkg}{A \code{\link[=pkg]{pkg()}}} + +\item{resource}{A \code{\link[=resource]{resource()}}, or if not provided, the \code{\link{resource}} +extracted from \code{pkg@resource}.} + +\item{field}{Used for dispatching on which field to derive. Methods are +provided such that a simple \code{character} field name can be passed and +used to build a class for dispatching to the right derivation function.} + +\item{...}{Used by specific methods.} +} +\description{ +Uses \code{evaluate::evaluate} to capture execution logs. +} +\keyword{internal} diff --git a/man/options.Rd b/man/options.Rd index 9436013..b561cec 100644 --- a/man/options.Rd +++ b/man/options.Rd @@ -37,12 +37,18 @@ types of information various metrics contain. For more details, see }} \item{logs}{\describe{ -Logging directory where artifacts will be stored. Defaults to a temporary -directory.\item{default: }{\preformatted{ns_tmp_root()}} +Whether output should be captured during the evaluation of metrics.\item{default: }{\preformatted{TRUE}} \item{option: }{val.meter.logs} \item{envvar: }{R_VAL_METER_LOGS (evaluated if possible, raw string otherwise)} }} +\item{artifacts}{\describe{ +Directory where artifacts will be stored. This includes installation logs, +package source code and temporary libraries used while evaluating packages.\item{default: }{\preformatted{ns_tmp_root()}} +\item{option: }{val.meter.artifacts} +\item{envvar: }{R_VAL_METER_ARTIFACTS (evaluated if possible, raw string otherwise)} +}} + \item{quiet}{\describe{ Silences console output during evaluation. This applies when pulling package resources (such as download and installation output) and executing code diff --git a/man/options_params.Rd b/man/options_params.Rd index e3a9d6f..0f451c4 100644 --- a/man/options_params.Rd +++ b/man/options_params.Rd @@ -8,6 +8,9 @@ types of information various metrics contain. For more details, see \code{\link[=tags]{tags()}}. (Defaults to \code{tags(TRUE)}, overwritable using option 'val.meter.tags' or environment variable 'R_VAL_METER_TAGS')} +\item{artifacts}{Directory where artifacts will be stored. This includes installation logs, +package source code and temporary libraries used while evaluating packages. (Defaults to \code{ns_tmp_root()}, overwritable using option 'val.meter.artifacts' or environment variable 'R_VAL_METER_ARTIFACTS')} + \item{quiet}{Silences console output during evaluation. This applies when pulling package resources (such as download and installation output) and executing code (for example, running \verb{R CMD check}) (Defaults to \code{TRUE}, overwritable using option 'val.meter.quiet' or environment variable 'R_VAL_METER_QUIET')} @@ -16,8 +19,7 @@ resources (such as download and installation output) and executing code resources will be discovered and what permissions are granted when calculating metrics. (Defaults to \code{policy()}, overwritable using option 'val.meter.policy' or environment variable 'R_VAL_METER_POLICY')} -\item{logs}{Logging directory where artifacts will be stored. Defaults to a temporary -directory. (Defaults to \code{ns_tmp_root()}, overwritable using option 'val.meter.logs' or environment variable 'R_VAL_METER_LOGS')} +\item{logs}{Whether output should be captured during the evaluation of metrics. (Defaults to \code{TRUE}, overwritable using option 'val.meter.logs' or environment variable 'R_VAL_METER_LOGS')} } \description{ Options As Parameters diff --git a/tests/testthat/test-convert-to-pkg.R b/tests/testthat/test-convert-to-pkg.R index e1ecbd7..c631651 100644 --- a/tests/testthat/test-convert-to-pkg.R +++ b/tests/testthat/test-convert-to-pkg.R @@ -1,4 +1,4 @@ -test_that("convert from character to pkg can discover resources", { +test_that("convert(from = character, to = class_pkg) can discover resources", { expected <- simpleError("downloading ...") class(expected) <- c("test_suite_signal", class(expected)) @@ -31,3 +31,48 @@ test_that("convert from character to pkg can discover resources", { } ) }) + +test_that("convert(from = class_pkg, to = class_pkg)", { + p <- random_pkg() + expect_identical(p, convert(p, class_pkg)) +}) + +test_that("convert(from = class_list, to = class_pkg)", { + pkg_data <- list( + name = "test", + version = "1.2.3", + r_cmd_check_error_count = 3L + ) + + expect_no_error(p <- convert(pkg_data, class_pkg)) + expect_identical(pkg_data$name, p$name) + expect_identical(pkg_data$version, p$version) + expect_identical( + pkg_data$r_cmd_check_error_count, + p$r_cmd_check_error_count + ) +}) + +test_that("convert(from = class_character [DCF], to = class_pkg)", { + desc <- " +Package: test +Version: 1.2.3 +Metric/r_cmd_check_error_count@R: 3L + " + + expect_no_error(p <- convert(desc, class_pkg)) + expect_identical(p$name, "test") + expect_identical(p$version, "1.2.3") + expect_identical(p$r_cmd_check_error_count, 3L) +}) + +test_that("convert(from = class_character [*.Rds], to = class_pkg)", { + orig_p <- random_pkg() + f <- tempfile("test-pkg-", fileext = ".Rds") + on.exit(file.remove(f)) + saveRDS(orig_p, f) + + expect_no_error(p <- convert(f, class_pkg)) + expect_identical(p$name, orig_p$name) + expect_identical(p$version, orig_p$version) +}) diff --git a/tests/testthat/test-derive-logs.R b/tests/testthat/test-derive-logs.R new file mode 100644 index 0000000..86400c1 --- /dev/null +++ b/tests/testthat/test-derive-logs.R @@ -0,0 +1,35 @@ +test_that("logs are captured during package data evaluation", { + impl_data( + "logs_test_name_character_count", + metric = TRUE, + class = class_integer, + overwrite = TRUE, + quiet = TRUE, + function(pkg, resource, field, ...) { + cat("text\n") + message("message") + warning("warning") + cli::cat_line(cli::col_blue("blue")) + nchar(pkg$name) + } + ) + + # expect that we have produced some logs + p <- pkg(mock_resource(package = "test", version = "1.2.3")) + expect_no_error(p$logs_test_name_character_count) + expect_true(!is.null(logs <- p@logs[["logs_test_name_character_count"]])) + + # expect to find our output in our logs + expect_true(is.character(text_logs <- format(logs, style = "text"))) + expect_match(text_logs, "text") + expect_match(text_logs, "\\bmessage\\b") + expect_match(text_logs, "\\bWarning in") + expect_match(text_logs, "\\bwarning\\b") + + # expect that we can format our logs as ansi strings + expect_true(is.character(ansi_logs <<- format(logs, style = "ansi"))) + expect_true(nchar(ansi_logs) > nchar(text_logs)) + + # expect that we can produce an html div from our logs + expect_s3_class(html_logs <- format(logs, style = "html"), "shiny.tag") +}) From 47956b22e39b01c31bbfc7fcc97482f083b148c8 Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Tue, 6 Jan 2026 11:31:31 -0500 Subject: [PATCH 02/10] feat: add global logging option --- R/class_pkg.R | 24 +++++++++++++++++++----- R/options.R | 6 ++++++ man/get_pkg_data.Rd | 5 ++++- man/options.Rd | 7 +++++++ man/options_params.Rd | 3 +++ tests/testthat/test-derive-logs.R | 30 ++++++++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 6 deletions(-) diff --git a/R/class_pkg.R b/R/class_pkg.R index 44fa0da..894962b 100644 --- a/R/class_pkg.R +++ b/R/class_pkg.R @@ -196,6 +196,8 @@ random_repo <- function(..., path = tempfile("repo")) { #' @param x [`pkg`] object to derive data for #' @param name `character(1L)` field name for the data to derive #' @param ... Additional arguments unused +#' @param logging `logical(1L)` flag indicating whether console output should be +#' captured during execution. #' @param .raise `logical(1L)` flag indicating whether errors should be raised #' or captured. This flag is not intended to be set directly, it is exposed #' so that recursive calls can raise lower-level errors while capturing them @@ -206,7 +208,13 @@ random_repo <- function(..., path = tempfile("repo")) { #' #' @keywords internal #' @include utils_err.R -get_pkg_data <- function(x, name, ..., .raise = .state$raise) { +get_pkg_data <- function( + x, + name, + ..., + logging = opt("logging"), + .raise = .state$raise +) { # RStudio, when trying to produce completions,will try to evaluate our lazy # list elements. Intercept those calls and return only the existing values. if (is_rs_rpc_get_completions_call()) { @@ -230,13 +238,19 @@ get_pkg_data <- function(x, name, ..., .raise = .state$raise) { assert_permissions(required_permissions, x@permissions) assert_suggests(required_suggests) - capture <- capture_pkg_data_derive(pkg = x, field = name, ...) - x@logs[[name]] <- capture$logs + if (logging) { + capture <- capture_pkg_data_derive(pkg = x, field = name, ...) + data <- capture$data + x@logs[[name]] <- capture$logs + } else { + data <- pkg_data_derive(pkg = x, field = name, ...) + } + if (!identical(info@data_class, class_any)) { - capture$data <- convert(capture$data, info@data_class) + data <- convert(data, info@data_class) } - capture$data + data }, error = function(e, ...) { convert(e, class_val_meter_error, field = name) diff --git a/R/options.R b/R/options.R index 9776494..f4a9518 100644 --- a/R/options.R +++ b/R/options.R @@ -32,6 +32,12 @@ define_options( ), artifacts = ns_tmp_root(), + fmt( + "Whether logs are captured during execution. When enabled, the `evaluate` + package is used to store console output during metric execution." + ), + logging = TRUE, + "Silences console output during evaluation. This applies when pulling package resources (such as download and installation output) and executing code (for example, running `R CMD check`)", diff --git a/man/get_pkg_data.Rd b/man/get_pkg_data.Rd index 82fcc0c..cfaf2d5 100644 --- a/man/get_pkg_data.Rd +++ b/man/get_pkg_data.Rd @@ -4,7 +4,7 @@ \alias{get_pkg_data} \title{Get \code{\link{pkg}} object data} \usage{ -get_pkg_data(x, name, ..., .raise = .state$raise) +get_pkg_data(x, name, ..., logging = opt("logging"), .raise = .state$raise) } \arguments{ \item{x}{\code{\link{pkg}} object to derive data for} @@ -13,6 +13,9 @@ get_pkg_data(x, name, ..., .raise = .state$raise) \item{...}{Additional arguments unused} +\item{logging}{\code{logical(1L)} flag indicating whether console output should be +captured during execution.} + \item{.raise}{\code{logical(1L)} flag indicating whether errors should be raised or captured. This flag is not intended to be set directly, it is exposed so that recursive calls can raise lower-level errors while capturing them diff --git a/man/options.Rd b/man/options.Rd index b561cec..0853b54 100644 --- a/man/options.Rd +++ b/man/options.Rd @@ -49,6 +49,13 @@ package source code and temporary libraries used while evaluating packages.\item \item{envvar: }{R_VAL_METER_ARTIFACTS (evaluated if possible, raw string otherwise)} }} +\item{logging}{\describe{ +Whether logs are captured during execution. When enabled, the \code{evaluate} +package is used to store console output during metric execution.\item{default: }{\preformatted{TRUE}} +\item{option: }{val.meter.logging} +\item{envvar: }{R_VAL_METER_LOGGING (evaluated if possible, raw string otherwise)} +}} + \item{quiet}{\describe{ Silences console output during evaluation. This applies when pulling package resources (such as download and installation output) and executing code diff --git a/man/options_params.Rd b/man/options_params.Rd index 0f451c4..e44faf4 100644 --- a/man/options_params.Rd +++ b/man/options_params.Rd @@ -19,6 +19,9 @@ resources (such as download and installation output) and executing code resources will be discovered and what permissions are granted when calculating metrics. (Defaults to \code{policy()}, overwritable using option 'val.meter.policy' or environment variable 'R_VAL_METER_POLICY')} +\item{logging}{Whether logs are captured during execution. When enabled, the \code{evaluate} +package is used to store console output during metric execution. (Defaults to \code{TRUE}, overwritable using option 'val.meter.logging' or environment variable 'R_VAL_METER_LOGGING')} + \item{logs}{Whether output should be captured during the evaluation of metrics. (Defaults to \code{TRUE}, overwritable using option 'val.meter.logs' or environment variable 'R_VAL_METER_LOGS')} } \description{ diff --git a/tests/testthat/test-derive-logs.R b/tests/testthat/test-derive-logs.R index 86400c1..2c52777 100644 --- a/tests/testthat/test-derive-logs.R +++ b/tests/testthat/test-derive-logs.R @@ -33,3 +33,33 @@ test_that("logs are captured during package data evaluation", { # expect that we can produce an html div from our logs expect_s3_class(html_logs <- format(logs, style = "html"), "shiny.tag") }) + +test_that("logging can be disabled by global option", { + old <- options(val.meter.logging = FALSE) + on.exit(options(old)) + + impl_data( + "logs_test_name_character_count", + metric = TRUE, + class = class_integer, + overwrite = TRUE, + quiet = TRUE, + function(pkg, resource, field, ...) { + cat("text\n") + message("message") + warning("warning") + cli::cat_line(cli::col_blue("blue")) + nchar(pkg$name) + } + ) + + # expect that we have produced some logs + p <- pkg(mock_resource(package = "test", version = "1.2.3")) + expect_message({ + expect_warning({ + expect_output(p$logs_test_name_character_count) + }) + }) + + expect_true(is.null(logs <- p@logs[["logs_test_name_character_count"]])) +}) From ee8c6e776aaaad3077380316f753b55c8de7b150 Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Tue, 6 Jan 2026 11:36:43 -0500 Subject: [PATCH 03/10] chore: disable logging by default --- R/options.R | 2 +- man/options.Rd | 2 +- man/options_params.Rd | 2 +- tests/testthat/test-derive-logs.R | 3 +++ 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/R/options.R b/R/options.R index f4a9518..30e56f8 100644 --- a/R/options.R +++ b/R/options.R @@ -36,7 +36,7 @@ define_options( "Whether logs are captured during execution. When enabled, the `evaluate` package is used to store console output during metric execution." ), - logging = TRUE, + logging = FALSE, "Silences console output during evaluation. This applies when pulling package resources (such as download and installation output) and executing code diff --git a/man/options.Rd b/man/options.Rd index 0853b54..b731e0c 100644 --- a/man/options.Rd +++ b/man/options.Rd @@ -51,7 +51,7 @@ package source code and temporary libraries used while evaluating packages.\item \item{logging}{\describe{ Whether logs are captured during execution. When enabled, the \code{evaluate} -package is used to store console output during metric execution.\item{default: }{\preformatted{TRUE}} +package is used to store console output during metric execution.\item{default: }{\preformatted{FALSE}} \item{option: }{val.meter.logging} \item{envvar: }{R_VAL_METER_LOGGING (evaluated if possible, raw string otherwise)} }} diff --git a/man/options_params.Rd b/man/options_params.Rd index e44faf4..330ae9c 100644 --- a/man/options_params.Rd +++ b/man/options_params.Rd @@ -20,7 +20,7 @@ resources will be discovered and what permissions are granted when calculating metrics. (Defaults to \code{policy()}, overwritable using option 'val.meter.policy' or environment variable 'R_VAL_METER_POLICY')} \item{logging}{Whether logs are captured during execution. When enabled, the \code{evaluate} -package is used to store console output during metric execution. (Defaults to \code{TRUE}, overwritable using option 'val.meter.logging' or environment variable 'R_VAL_METER_LOGGING')} +package is used to store console output during metric execution. (Defaults to \code{FALSE}, overwritable using option 'val.meter.logging' or environment variable 'R_VAL_METER_LOGGING')} \item{logs}{Whether output should be captured during the evaluation of metrics. (Defaults to \code{TRUE}, overwritable using option 'val.meter.logs' or environment variable 'R_VAL_METER_LOGS')} } diff --git a/tests/testthat/test-derive-logs.R b/tests/testthat/test-derive-logs.R index 2c52777..0f1e079 100644 --- a/tests/testthat/test-derive-logs.R +++ b/tests/testthat/test-derive-logs.R @@ -1,4 +1,7 @@ test_that("logs are captured during package data evaluation", { + old <- options(val.meter.logging = TRUE) + on.exit(options(old)) + impl_data( "logs_test_name_character_count", metric = TRUE, From 58b1f1d882a4a463fcaa9150956e15302fb489c5 Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Tue, 6 Jan 2026 12:00:24 -0500 Subject: [PATCH 04/10] chore: consolidate logging options --- R/class_pkg.R | 6 +++--- R/options.R | 8 +------- man/get_pkg_data.Rd | 4 ++-- man/options.Rd | 9 +-------- man/options_params.Rd | 5 +---- tests/testthat/test-derive-logs.R | 4 ++-- 6 files changed, 10 insertions(+), 26 deletions(-) diff --git a/R/class_pkg.R b/R/class_pkg.R index 894962b..bf4455c 100644 --- a/R/class_pkg.R +++ b/R/class_pkg.R @@ -196,7 +196,7 @@ random_repo <- function(..., path = tempfile("repo")) { #' @param x [`pkg`] object to derive data for #' @param name `character(1L)` field name for the data to derive #' @param ... Additional arguments unused -#' @param logging `logical(1L)` flag indicating whether console output should be +#' @param logs `logical(1L)` flag indicating whether console output should be #' captured during execution. #' @param .raise `logical(1L)` flag indicating whether errors should be raised #' or captured. This flag is not intended to be set directly, it is exposed @@ -212,7 +212,7 @@ get_pkg_data <- function( x, name, ..., - logging = opt("logging"), + logs = opt("logs"), .raise = .state$raise ) { # RStudio, when trying to produce completions,will try to evaluate our lazy @@ -238,7 +238,7 @@ get_pkg_data <- function( assert_permissions(required_permissions, x@permissions) assert_suggests(required_suggests) - if (logging) { + if (logs) { capture <- capture_pkg_data_derive(pkg = x, field = name, ...) data <- capture$data x@logs[[name]] <- capture$logs diff --git a/R/options.R b/R/options.R index 30e56f8..9678bd6 100644 --- a/R/options.R +++ b/R/options.R @@ -24,7 +24,7 @@ define_options( tags = tags(TRUE), fmt("Whether output should be captured during the evaluation of metrics."), - logs = TRUE, + logs = FALSE, fmt( "Directory where artifacts will be stored. This includes installation logs, @@ -32,12 +32,6 @@ define_options( ), artifacts = ns_tmp_root(), - fmt( - "Whether logs are captured during execution. When enabled, the `evaluate` - package is used to store console output during metric execution." - ), - logging = FALSE, - "Silences console output during evaluation. This applies when pulling package resources (such as download and installation output) and executing code (for example, running `R CMD check`)", diff --git a/man/get_pkg_data.Rd b/man/get_pkg_data.Rd index cfaf2d5..fba3040 100644 --- a/man/get_pkg_data.Rd +++ b/man/get_pkg_data.Rd @@ -4,7 +4,7 @@ \alias{get_pkg_data} \title{Get \code{\link{pkg}} object data} \usage{ -get_pkg_data(x, name, ..., logging = opt("logging"), .raise = .state$raise) +get_pkg_data(x, name, ..., logs = opt("logs"), .raise = .state$raise) } \arguments{ \item{x}{\code{\link{pkg}} object to derive data for} @@ -13,7 +13,7 @@ get_pkg_data(x, name, ..., logging = opt("logging"), .raise = .state$raise) \item{...}{Additional arguments unused} -\item{logging}{\code{logical(1L)} flag indicating whether console output should be +\item{logs}{\code{logical(1L)} flag indicating whether console output should be captured during execution.} \item{.raise}{\code{logical(1L)} flag indicating whether errors should be raised diff --git a/man/options.Rd b/man/options.Rd index b731e0c..c4641d4 100644 --- a/man/options.Rd +++ b/man/options.Rd @@ -37,7 +37,7 @@ types of information various metrics contain. For more details, see }} \item{logs}{\describe{ -Whether output should be captured during the evaluation of metrics.\item{default: }{\preformatted{TRUE}} +Whether output should be captured during the evaluation of metrics.\item{default: }{\preformatted{FALSE}} \item{option: }{val.meter.logs} \item{envvar: }{R_VAL_METER_LOGS (evaluated if possible, raw string otherwise)} }} @@ -49,13 +49,6 @@ package source code and temporary libraries used while evaluating packages.\item \item{envvar: }{R_VAL_METER_ARTIFACTS (evaluated if possible, raw string otherwise)} }} -\item{logging}{\describe{ -Whether logs are captured during execution. When enabled, the \code{evaluate} -package is used to store console output during metric execution.\item{default: }{\preformatted{FALSE}} -\item{option: }{val.meter.logging} -\item{envvar: }{R_VAL_METER_LOGGING (evaluated if possible, raw string otherwise)} -}} - \item{quiet}{\describe{ Silences console output during evaluation. This applies when pulling package resources (such as download and installation output) and executing code diff --git a/man/options_params.Rd b/man/options_params.Rd index 330ae9c..7caeec2 100644 --- a/man/options_params.Rd +++ b/man/options_params.Rd @@ -19,10 +19,7 @@ resources (such as download and installation output) and executing code resources will be discovered and what permissions are granted when calculating metrics. (Defaults to \code{policy()}, overwritable using option 'val.meter.policy' or environment variable 'R_VAL_METER_POLICY')} -\item{logging}{Whether logs are captured during execution. When enabled, the \code{evaluate} -package is used to store console output during metric execution. (Defaults to \code{FALSE}, overwritable using option 'val.meter.logging' or environment variable 'R_VAL_METER_LOGGING')} - -\item{logs}{Whether output should be captured during the evaluation of metrics. (Defaults to \code{TRUE}, overwritable using option 'val.meter.logs' or environment variable 'R_VAL_METER_LOGS')} +\item{logs}{Whether output should be captured during the evaluation of metrics. (Defaults to \code{FALSE}, overwritable using option 'val.meter.logs' or environment variable 'R_VAL_METER_LOGS')} } \description{ Options As Parameters diff --git a/tests/testthat/test-derive-logs.R b/tests/testthat/test-derive-logs.R index 0f1e079..ae39ddd 100644 --- a/tests/testthat/test-derive-logs.R +++ b/tests/testthat/test-derive-logs.R @@ -1,5 +1,5 @@ test_that("logs are captured during package data evaluation", { - old <- options(val.meter.logging = TRUE) + old <- options(val.meter.logs = TRUE) on.exit(options(old)) impl_data( @@ -38,7 +38,7 @@ test_that("logs are captured during package data evaluation", { }) test_that("logging can be disabled by global option", { - old <- options(val.meter.logging = FALSE) + old <- options(val.meter.logs = FALSE) on.exit(options(old)) impl_data( From 0d888f33a498e161adbfc10572e0ee367e2a3c7e Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Wed, 7 Jan 2026 13:20:54 -0500 Subject: [PATCH 05/10] chore: add test for error handling during logging --- R/generic_pkg_data_derive.R | 6 ++- tests/fixtures/pkg.local.source/DESCRIPTION | 11 ++++++ tests/fixtures/pkg.local.source/NAMESPACE | 2 + tests/testthat/test-derive-logs.R | 44 +++++++++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/pkg.local.source/DESCRIPTION create mode 100644 tests/fixtures/pkg.local.source/NAMESPACE diff --git a/R/generic_pkg_data_derive.R b/R/generic_pkg_data_derive.R index b72bbc3..3dc9315 100644 --- a/R/generic_pkg_data_derive.R +++ b/R/generic_pkg_data_derive.R @@ -40,7 +40,7 @@ capture_pkg_data_derive <- function( ) { # build a prettier call that will be output by evaluate() when not quiet x <- pkg - pkg <- list(function() pkg_data_derive(pkg = x, field = field)) + pkg <- list(function() pkg_data_derive(pkg = x, field = field, ...)) names(pkg) <- field evaluate_fn <- function() {} body(evaluate_fn) <- as.call(list(call("$", as.symbol("pkg"), field))) @@ -63,6 +63,10 @@ capture_pkg_data_derive <- function( output_handler = evaluate::new_output_handler(value = identity) ) + if (inherits(result_error <- capture[[length(capture)]], "error")) { + stop(result_error) + } + list( # omit code echo and return value logs = capture[-c(1, length(capture))], diff --git a/tests/fixtures/pkg.local.source/DESCRIPTION b/tests/fixtures/pkg.local.source/DESCRIPTION new file mode 100644 index 0000000..c7d0b14 --- /dev/null +++ b/tests/fixtures/pkg.local.source/DESCRIPTION @@ -0,0 +1,11 @@ +Package: pkg.local.source +Title: What the Package Does (One Line, Title Case) +Version: 0.0.0.9000 +Authors@R: + person("First", "Last", , "first.last@example.com", role = c("aut", "cre")) +Description: What the package does (one paragraph). +License: `use_mit_license()`, `use_gpl3_license()` or friends to pick a + license +Encoding: UTF-8 +Roxygen: list(markdown = TRUE) +RoxygenNote: 7.3.3 diff --git a/tests/fixtures/pkg.local.source/NAMESPACE b/tests/fixtures/pkg.local.source/NAMESPACE new file mode 100644 index 0000000..6ae9268 --- /dev/null +++ b/tests/fixtures/pkg.local.source/NAMESPACE @@ -0,0 +1,2 @@ +# Generated by roxygen2: do not edit by hand + diff --git a/tests/testthat/test-derive-logs.R b/tests/testthat/test-derive-logs.R index ae39ddd..9164ff9 100644 --- a/tests/testthat/test-derive-logs.R +++ b/tests/testthat/test-derive-logs.R @@ -37,6 +37,50 @@ test_that("logs are captured during package data evaluation", { expect_s3_class(html_logs <- format(logs, style = "html"), "shiny.tag") }) +test_that("logging disabled does not intercept error messages", { + old <- options(val.meter.logs = FALSE) + on.exit(options(old)) + + impl_data( + "logs_test_name_character_count", + metric = TRUE, + class = class_integer, + overwrite = TRUE, + quiet = TRUE, + function(pkg, resource, field, ...) { + stop("error!!") + cli::cat_line(cli::col_blue("blue")) + nchar(pkg$name) + } + ) + + p <- pkg(test_path("..", "fixtures", "pkg.local.source")) + expect_s3_class(p$logs_test_name_character_count, "error") + expect_equal(p$logs_test_name_character_count$body, "error!!") +}) + +test_that("logging enabled does not intercept error messages", { + old <- options(val.meter.logs = TRUE) + on.exit(options(old)) + + impl_data( + "logs_test_name_character_count", + metric = TRUE, + class = class_integer, + overwrite = TRUE, + quiet = TRUE, + function(pkg, resource, field, ...) { + stop("error!!") + cli::cat_line(cli::col_blue("blue")) + nchar(pkg$name) + } + ) + + p <- pkg(test_path("..", "fixtures", "pkg.local.source")) + expect_s3_class(p$logs_test_name_character_count, "error") + expect_equal(p$logs_test_name_character_count$body, "error!!") +}) + test_that("logging can be disabled by global option", { old <- options(val.meter.logs = FALSE) on.exit(options(old)) From 018f70d3fdd8a29570aad7b58ca7db33dffa7c25 Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Wed, 7 Jan 2026 13:24:23 -0500 Subject: [PATCH 06/10] chore: store logs before throwing captured error --- R/class_pkg.R | 5 +++++ R/generic_pkg_data_derive.R | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/R/class_pkg.R b/R/class_pkg.R index bf4455c..09a9546 100644 --- a/R/class_pkg.R +++ b/R/class_pkg.R @@ -242,6 +242,11 @@ get_pkg_data <- function( capture <- capture_pkg_data_derive(pkg = x, field = name, ...) data <- capture$data x@logs[[name]] <- capture$logs + + # re-throw error after storing logs if one was produced + if (inherits(data, "error")) { + stop(data) + } } else { data <- pkg_data_derive(pkg = x, field = name, ...) } diff --git a/R/generic_pkg_data_derive.R b/R/generic_pkg_data_derive.R index 3dc9315..0452848 100644 --- a/R/generic_pkg_data_derive.R +++ b/R/generic_pkg_data_derive.R @@ -63,10 +63,6 @@ capture_pkg_data_derive <- function( output_handler = evaluate::new_output_handler(value = identity) ) - if (inherits(result_error <- capture[[length(capture)]], "error")) { - stop(result_error) - } - list( # omit code echo and return value logs = capture[-c(1, length(capture))], From bc14bbb5209d6317e3e13ceff13f8585b254e77e Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:03:48 -0500 Subject: [PATCH 07/10] feat: improve output for knitr knit_print --- DESCRIPTION | 1 + NAMESPACE | 4 + R/share-register-s3.R | 132 ++++++++++++++++++++++++++ R/utils_evaluate.R | 132 ++++++++++++++++++++++++-- R/zzz.R | 1 + man/cli_singleton_html_style.Rd | 13 +++ man/format_output.Rd | 42 ++++++++ man/infer_format_style.Rd | 12 +++ man/knit_print.evaluate_evaluation.Rd | 18 ++++ man/s3_register.Rd | 63 ++++++++++++ 10 files changed, 411 insertions(+), 7 deletions(-) create mode 100644 R/share-register-s3.R create mode 100644 man/cli_singleton_html_style.Rd create mode 100644 man/format_output.Rd create mode 100644 man/infer_format_style.Rd create mode 100644 man/knit_print.evaluate_evaluation.Rd create mode 100644 man/s3_register.Rd diff --git a/DESCRIPTION b/DESCRIPTION index 08153f0..52611e3 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -99,6 +99,7 @@ Collate: 'generic_metric_coerce.R' 'options.R' 'package.R' + 'share-register-s3.R' 'utils_backports.R' 'utils_evaluate.R' 'utils_rand.R' diff --git a/NAMESPACE b/NAMESPACE index 1189d36..7618ee6 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -12,12 +12,14 @@ export(class_metric_data_frame) export(class_package_matrix) export(cran_repo_resource) export(error) +export(format_output) export(from_dcf) export(git_resource) export(impl_data) export(impl_data_derive) export(impl_data_info) export(install_resource) +export(knit_print.evaluate_evaluation) export(local_resource) export(local_source_resource) export(metric_coerce) @@ -37,6 +39,7 @@ export(random_repo) export(remote_resource) export(repo_resource) export(resource) +export(s3_register) export(source_archive_resource) export(source_code_resource) export(tags) @@ -45,6 +48,7 @@ import(S7) import(cli) import(options) importFrom(desc,desc) +importFrom(evaluate,replay) importFrom(httr2,req_perform) importFrom(httr2,request) importFrom(httr2,resp_body_html) diff --git a/R/share-register-s3.R b/R/share-register-s3.R new file mode 100644 index 0000000..e738fe9 --- /dev/null +++ b/R/share-register-s3.R @@ -0,0 +1,132 @@ +# This source code file is licensed under the unlicense license +# https://unlicense.org +#' Register a method for a suggested dependency +#' +#' Generally, the recommend way to register an S3 method is to use the +#' `S3Method()` namespace directive (often generated automatically by the +#' `@export` roxygen2 tag). However, this technique requires that the generic +#' be in an imported package, and sometimes you want to suggest a package, +#' and only provide a method when that package is loaded. `s3_register()` +#' can be called from your package's `.onLoad()` to dynamically register +#' a method only if the generic's package is loaded. +#' +#' For R 3.5.0 and later, `s3_register()` is also useful when demonstrating +#' class creation in a vignette, since method lookup no longer always involves +#' the lexical scope. For R 3.6.0 and later, you can achieve a similar effect +#' by using "delayed method registration", i.e. placing the following in your +#' `NAMESPACE` file: +#' +#' ``` +#' if (getRversion() >= "3.6.0") { +#' S3method(package::generic, class) +#' } +#' ``` +#' +#' @section Usage in other packages: +#' To avoid taking a dependency on vctrs, you copy the source of +#' [`s3_register()`](https://github.com/r-lib/vctrs/blob/main/R/register-s3.R) +#' into your own package. It is licensed under the permissive +#' [unlicense](https://choosealicense.com/licenses/unlicense/) to make it +#' crystal clear that we're happy for you to do this. There's no need to include +#' the license or even credit us when using this function. +#' +#' @usage NULL +#' @param generic Name of the generic in the form `pkg::generic`. +#' @param class Name of the class +#' @param method Optionally, the implementation of the method. By default, +#' this will be found by looking for a function called `generic.class` +#' in the package environment. +#' +#' Note that providing `method` can be dangerous if you use +#' devtools. When the namespace of the method is reloaded by +#' `devtools::load_all()`, the function will keep inheriting from +#' the old namespace. This might cause crashes because of dangling +#' `.Call()` pointers. +#' @export +#' @examples +#' # A typical use case is to dynamically register tibble/pillar methods +#' # for your class. That way you avoid creating a hard dependency on packages +#' # that are not essential, while still providing finer control over +#' # printing when they are used. +#' +#' .onLoad <- function(...) { +#' s3_register("pillar::pillar_shaft", "vctrs_vctr") +#' s3_register("tibble::type_sum", "vctrs_vctr") +#' } +#' @keywords internal + +# nolint start +# nocov start +s3_register <- function(generic, class, method = NULL) { + stopifnot(is.character(generic), length(generic) == 1) + stopifnot(is.character(class), length(class) == 1) + + pieces <- strsplit(generic, "::")[[1]] + stopifnot(length(pieces) == 2) + package <- pieces[[1]] + generic <- pieces[[2]] + + caller <- parent.frame() + + get_method_env <- function() { + top <- topenv(caller) + if (isNamespace(top)) { + asNamespace(environmentName(top)) + } else { + caller + } + } + get_method <- function(method) { + if (is.null(method)) { + get(paste0(generic, ".", class), envir = get_method_env()) + } else { + method + } + } + + register <- function(...) { + envir <- asNamespace(package) + + # Refresh the method each time, it might have been updated by + # `devtools::load_all()` + method_fn <- get_method(method) + stopifnot(is.function(method_fn)) + + # Only register if generic can be accessed + if (exists(generic, envir)) { + registerS3method(generic, class, method_fn, envir = envir) + } else if (identical(Sys.getenv("NOT_CRAN"), "true")) { + warning( + sprintf( + "Can't find generic `%s` in package %s to register S3 method.", + generic, + package + ) + ) + } + } + + # Always register hook in case package is later unloaded & reloaded + setHook(packageEvent(package, "onLoad"), function(...) { + register() + }) + + # For compatibility with R < 4.0 where base isn't locked + is_sealed <- function(pkg) { + identical(pkg, "base") || environmentIsLocked(asNamespace(pkg)) + } + + # Avoid registration failures during loading (pkgload or regular). + # Check that environment is locked because the registering package + # might be a dependency of the package that exports the generic. In + # that case, the exports (and the generic) might not be populated + # yet (#1225). + if (isNamespaceLoaded(package) && is_sealed(package)) { + register() + } + + invisible() +} + +# nocov end +# nolint end diff --git a/R/utils_evaluate.R b/R/utils_evaluate.R index 16410fc..0306334 100644 --- a/R/utils_evaluate.R +++ b/R/utils_evaluate.R @@ -1,11 +1,50 @@ +#' `knitr` Pretty-printing of rich ansi output captured with `evaluate()` +#' +#' @inheritParams knitr::knit_print +#' +#' @export +knit_print.evaluate_evaluation <- function(x, ...) { + res <- format_output(evaluate::replay(x)) + if (!is.character(res) && requireNamespace("knitr", quietly = TRUE)) { + knitr::knit_print(res) + } else { + cat(res) + } +} + +#' Formatted version of `capture.output()` +#' +#' Captures output and formats it according to `style`. Uses the [evaluate] +#' package to capture output to be more resilient to other processes sinking +#' output, causing issues with `capture.output` -- notably when used with +#' [knitr]. +#' +#' @param x An expression to capture output from. +#' @param ... Additional arguments unused. +#' @param style What style to format output as. When `knitr` is running, +#' infers the preferred format from the output document type. +#' @param evaluate Whether to evaluate `x` by first capturing output with +#' [evaluate::evaluate]. When `FALSE`, format `x` as a `character` value +#' directly. +#' @param envir An environment in which expression `x` should be evaluated. +#' +#' @examples +#' format_output( +#' cli::cli_text(cli::col_red("hello, world!")), +#' type = "message", # cli outputs to message stream +#' style = "html" +#' ) +#' #' @importFrom utils capture.output #' @export -format.evaluate_evaluation <- function( +format_output <- function( x, ..., - style = c("text", "ansi", "html") + style = infer_format_style(), + evaluate = TRUE, + envir = parent.frame() ) { - style <- match.arg(style) + style <- match.arg(style, choices = c("text", "ansi", "html")) if ( identical(style, "html") && !requireNamespace("htmltools", quietly = TRUE) @@ -16,17 +55,96 @@ format.evaluate_evaluation <- function( ) } - out <- utils::capture.output(evaluate::replay(x)) + old_options <- options(width = 80L, crayon.enabled = TRUE, cli.ansi = TRUE) + on.exit(options(old_options)) + + out <- if (evaluate) { + fn <- function() {} + body(fn) <- substitute(x) + ev <- evaluate::evaluate(fn, envir = envir)[-1L] + capture.output(evaluate::replay(ev)) + } else { + x + } + switch( style, text = paste(cli::ansi_strip(out), collapse = "\n"), ansi = paste(out, collapse = "\n"), html = { html <- cli::ansi_html(paste(out, collapse = "\n")) - htmltools::tags$div( - style = cli::ansi_html_style(colors = 8L), - htmltools::tags$pre(htmltools::HTML(html)) + singleton <- cli_singleton_html_style(colors = 8L) + htmltools::div( + singleton, + htmltools::tags$pre( + class = c("r-output", names(singleton)), + htmltools::HTML(html) + ) ) } ) } + +#' Try to infer the output format for formatting functions +#' +#' @keywords internal +#' +infer_format_style <- function() { + if ( + getOption("knitr.in.progress", FALSE) && + requireNamespace("knitr", quietly = TRUE) + ) { + return(switch( + knitr::opts_knit$get("rmarkdown.pandoc.to") %||% + knitr::opts_knit$get("out.format"), + "html" = "html", + "text" + )) + } + + if (cli::is_ansi_tty()) { + return("ansi") + } + + "text" +} + +#' Produce a html style for cli output +#' +#' Returns a list with one element, the html style tag, whose name is the +#' class name given to this style. +#' +#' @keywords internal +#' +cli_singleton_html_style <- function(...) { + args <- match.call(cli::ansi_html_style, expand.dots = TRUE)[-1L] + css_class <- paste0( + "r-cli--", + paste0(names(args), "-", args, collapse = "--") + ) + + style <- format(cli::ansi_html_style(colors = 8L)) + html <- htmltools::singleton( + htmltools::tags$style( + paste0( + "\n", + paste0(".", css_class, names(style), " ", style, collapse = "\n"), + "\n" + ) + ) + ) + + html <- list(html) + names(html) <- css_class + + html +} + +#' @importFrom evaluate replay +#' @export +format.evaluate_evaluation <- function( + x, + ... +) { + format_output(evaluate::replay(x), ...) +} diff --git a/R/zzz.R b/R/zzz.R index 2247696..6d0f49c 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -1,5 +1,6 @@ .onLoad <- function(lib, pkg) { S7::methods_register() + s3_register("knitr::knit_print", "evaluate_evaluation") } # Unclear why this is needed, but without it we get an R CMD check NOTE diff --git a/man/cli_singleton_html_style.Rd b/man/cli_singleton_html_style.Rd new file mode 100644 index 0000000..f4ffd1e --- /dev/null +++ b/man/cli_singleton_html_style.Rd @@ -0,0 +1,13 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils_evaluate.R +\name{cli_singleton_html_style} +\alias{cli_singleton_html_style} +\title{Produce a html style for cli output} +\usage{ +cli_singleton_html_style(...) +} +\description{ +Returns a list with one element, the html style tag, whose name is the +class name given to this style. +} +\keyword{internal} diff --git a/man/format_output.Rd b/man/format_output.Rd new file mode 100644 index 0000000..437a623 --- /dev/null +++ b/man/format_output.Rd @@ -0,0 +1,42 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils_evaluate.R +\name{format_output} +\alias{format_output} +\title{Formatted version of \code{capture.output()}} +\usage{ +format_output( + x, + ..., + style = infer_format_style(), + evaluate = TRUE, + envir = parent.frame() +) +} +\arguments{ +\item{x}{An expression to capture output from.} + +\item{...}{Additional arguments unused.} + +\item{style}{What style to format output as. When \code{knitr} is running, +infers the preferred format from the output document type.} + +\item{evaluate}{Whether to evaluate \code{x} by first capturing output with +\link[evaluate:evaluate]{evaluate::evaluate}. When \code{FALSE}, format \code{x} as a \code{character} value +directly.} + +\item{envir}{An environment in which expression \code{x} should be evaluated.} +} +\description{ +Captures output and formats it according to \code{style}. Uses the \link[evaluate:evaluate]{evaluate::evaluate} +package to capture output to be more resilient to other processes sinking +output, causing issues with \code{capture.output} -- notably when used with +\link[knitr:knitr-package]{knitr::knitr}. +} +\examples{ +format_output( + cli::cli_text(cli::col_red("hello, world!")), + type = "message", # cli outputs to message stream + style = "html" +) + +} diff --git a/man/infer_format_style.Rd b/man/infer_format_style.Rd new file mode 100644 index 0000000..351b64f --- /dev/null +++ b/man/infer_format_style.Rd @@ -0,0 +1,12 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils_evaluate.R +\name{infer_format_style} +\alias{infer_format_style} +\title{Try to infer the output format for formatting functions} +\usage{ +infer_format_style() +} +\description{ +Try to infer the output format for formatting functions +} +\keyword{internal} diff --git a/man/knit_print.evaluate_evaluation.Rd b/man/knit_print.evaluate_evaluation.Rd new file mode 100644 index 0000000..fe8a650 --- /dev/null +++ b/man/knit_print.evaluate_evaluation.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils_evaluate.R +\name{knit_print.evaluate_evaluation} +\alias{knit_print.evaluate_evaluation} +\title{\code{knitr} Pretty-printing of rich ansi output captured with \code{evaluate()}} +\usage{ +knit_print.evaluate_evaluation(x, ...) +} +\arguments{ +\item{x}{An R object to be printed} + +\item{...}{Additional arguments passed to the S3 method. Currently ignored, +except two optional arguments \code{options} and \code{inline}; see +the references below.} +} +\description{ +\code{knitr} Pretty-printing of rich ansi output captured with \code{evaluate()} +} diff --git a/man/s3_register.Rd b/man/s3_register.Rd new file mode 100644 index 0000000..0274af2 --- /dev/null +++ b/man/s3_register.Rd @@ -0,0 +1,63 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/share-register-s3.R +\name{s3_register} +\alias{s3_register} +\title{Register a method for a suggested dependency} +\arguments{ +\item{generic}{Name of the generic in the form \code{pkg::generic}.} + +\item{class}{Name of the class} + +\item{method}{Optionally, the implementation of the method. By default, +this will be found by looking for a function called \code{generic.class} +in the package environment. + +Note that providing \code{method} can be dangerous if you use +devtools. When the namespace of the method is reloaded by +\code{devtools::load_all()}, the function will keep inheriting from +the old namespace. This might cause crashes because of dangling +\code{.Call()} pointers.} +} +\description{ +Generally, the recommend way to register an S3 method is to use the +\code{S3Method()} namespace directive (often generated automatically by the +\verb{@export} roxygen2 tag). However, this technique requires that the generic +be in an imported package, and sometimes you want to suggest a package, +and only provide a method when that package is loaded. \code{s3_register()} +can be called from your package's \code{.onLoad()} to dynamically register +a method only if the generic's package is loaded. +} +\details{ +For R 3.5.0 and later, \code{s3_register()} is also useful when demonstrating +class creation in a vignette, since method lookup no longer always involves +the lexical scope. For R 3.6.0 and later, you can achieve a similar effect +by using "delayed method registration", i.e. placing the following in your +\code{NAMESPACE} file: + +\if{html}{\out{
}}\preformatted{if (getRversion() >= "3.6.0") \{ + S3method(package::generic, class) +\} +}\if{html}{\out{
}} +} +\section{Usage in other packages}{ + +To avoid taking a dependency on vctrs, you copy the source of +\href{https://github.com/r-lib/vctrs/blob/main/R/register-s3.R}{\code{s3_register()}} +into your own package. It is licensed under the permissive +\href{https://choosealicense.com/licenses/unlicense/}{unlicense} to make it +crystal clear that we're happy for you to do this. There's no need to include +the license or even credit us when using this function. +} + +\examples{ +# A typical use case is to dynamically register tibble/pillar methods +# for your class. That way you avoid creating a hard dependency on packages +# that are not essential, while still providing finer control over +# printing when they are used. + +.onLoad <- function(...) { + s3_register("pillar::pillar_shaft", "vctrs_vctr") + s3_register("tibble::type_sum", "vctrs_vctr") +} +} +\keyword{internal} From c61f27e4fa0227d9443b0b1d8dd2ceef7f20fe48 Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:24:26 -0500 Subject: [PATCH 08/10] chore: adding to pkgdown index --- R/utils_evaluate.R | 2 ++ pkgdown/_pkgdown.yml | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/R/utils_evaluate.R b/R/utils_evaluate.R index 0306334..01fa05d 100644 --- a/R/utils_evaluate.R +++ b/R/utils_evaluate.R @@ -3,7 +3,9 @@ #' @inheritParams knitr::knit_print #' #' @export +# nolint start knit_print.evaluate_evaluation <- function(x, ...) { + # nolint end res <- format_output(evaluate::replay(x)) if (!is.character(res) && requireNamespace("knitr", quietly = TRUE)) { knitr::knit_print(res) diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index 04e87e0..e12c5d8 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -86,3 +86,11 @@ reference: - subtitle: Miscellaneous - contents: - error + + - title: > + Package Extensions + + - subtitle: Knitr + - contents: + - starts_with("knit_print") + - format_output From ede9e84b373c451cc235ddf81bf3c2220c319906 Mon Sep 17 00:00:00 2001 From: dgkf <18220321+dgkf@users.noreply.github.com> Date: Thu, 5 Mar 2026 08:55:42 -0500 Subject: [PATCH 09/10] chore: add tests for order independence of logs --- tests/testthat/test-derive-logs.R | 78 +++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/testthat/test-derive-logs.R b/tests/testthat/test-derive-logs.R index 9164ff9..90da272 100644 --- a/tests/testthat/test-derive-logs.R +++ b/tests/testthat/test-derive-logs.R @@ -110,3 +110,81 @@ test_that("logging can be disabled by global option", { expect_true(is.null(logs <- p@logs[["logs_test_name_character_count"]])) }) + +test_that("logging captures logs for the deepest evaluated metric", { + # in situations where one metric requires the evaluation of another metric, + # the output should be attributed to the lowest evaluated metric on the call + # stack + + old <- options(val.meter.logs = TRUE) + on.exit(options(old)) + + impl_data( + "logs_random_word", + metric = TRUE, + class = class_character, + overwrite = TRUE, + quiet = TRUE, + function(pkg, resource, field, ...) { + cat("inner_cat\n") + message("inner_message") + chars <- sample(letters, round(runif(1, 20, 40)), replace = TRUE) + paste(chars, collapse = "") + } + ) + + impl_data( + "logs_random_word_character_count", + metric = TRUE, + class = class_integer, + overwrite = TRUE, + quiet = TRUE, + function(pkg, resource, field, ...) { + cat("outer_cat1\n") + message("outer_message1") + warning("outer_warning1") + cli::cat_line(cli::col_blue("blue")) + out <- nchar(pkg$logs_random_word) + cat("outer_cat2\n") + message("outer_message2") # after returning to outer evaluation + out + } + ) + + # expect that logs were captured + p1 <- pkg(mock_resource(package = "test", version = "1.2.3")) + p2 <- pkg(mock_resource(package = "test", version = "1.2.3")) + + # first evaluate outer metric, which will call inner metric + expect_silent(p1$logs_random_word_character_count) + expect_silent({ + p2$logs_random_word + p2$logs_random_word_character_count + }) + + # expect logs to be execution order independent + expect_identical( + p1@logs$logs_random_word_character_count, + p2@logs$logs_random_word_character_count + ) + expect_identical( + p1@logs$logs_random_word, + p2@logs$logs_random_word + ) + + # expect that inner logs capture messages from inner metric + expect_length(p1@logs$logs_random_word, 2L) + expect_match(p1@logs$logs_random_word[[1L]], "inner_cat") + expect_match( + conditionMessage(p1@logs$logs_random_word[[2L]]), + "inner_message" + ) + + # expect that outer logs capture its output after returning from inner capture + expect_length(p1@logs$logs_random_word_character_count, 5L) + expect_match(p1@logs$logs_random_word_character_count[[4L]], "outer_cat2") + expect_match( + conditionMessage(p1@logs$logs_random_word_character_count[[5L]]), + "outer_message2" + ) +}) From 0dfa4eeb244fe0ba51124db50c44659ec744d8b0 Mon Sep 17 00:00:00 2001 From: Doug Kelkhoff <18220321+dgkf@users.noreply.github.com> Date: Thu, 12 Mar 2026 13:05:01 -0400 Subject: [PATCH 10/10] Apply on.exit suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: LluĂ­s Revilla <185338939+llrs-roche@users.noreply.github.com> --- R/generic_pkg_data_derive.R | 2 +- R/utils_evaluate.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/generic_pkg_data_derive.R b/R/generic_pkg_data_derive.R index 0452848..8356be9 100644 --- a/R/generic_pkg_data_derive.R +++ b/R/generic_pkg_data_derive.R @@ -54,7 +54,7 @@ capture_pkg_data_derive <- function( cli.num_colors = 256L ) - on.exit(options(original_opts)) + on.exit(options(original_opts), add = TRUE) capture <- evaluate::evaluate( evaluate_fn, diff --git a/R/utils_evaluate.R b/R/utils_evaluate.R index 01fa05d..d2e4f17 100644 --- a/R/utils_evaluate.R +++ b/R/utils_evaluate.R @@ -58,7 +58,7 @@ format_output <- function( } old_options <- options(width = 80L, crayon.enabled = TRUE, cli.ansi = TRUE) - on.exit(options(old_options)) + on.exit(options(old_options), add = TRUE) out <- if (evaluate) { fn <- function() {}