From d009c60aa96d98ae1dd4b5c428a892a732d6c416 Mon Sep 17 00:00:00 2001 From: Jon Harmon Date: Fri, 15 May 2026 06:51:41 -0500 Subject: [PATCH 1/4] Use stbl warn and inform Implement `.pkg_inform()` and `.pkg_warn()`, and use them in place of `cli_inform()` and `cli_warn()`. Also recommend them. Also update `pkg_abort()` (both in the template and in `R/aaa-conditions.R`) to include the `parent` argument and `...`. I also updated to roxygen2 8.0.0, which resulted in some unrelated `man/` changes. Closes #81 --- DESCRIPTION | 11 ++-- R/aaa-conditions.R | 60 ++++++++++++++++++- R/use_agent.R | 19 +++--- R/use_github_copilot_whitelist.R | 16 +++-- R/use_skill.R | 5 +- R/use_skill_create_issue.R | 5 +- R/use_skill_document.R | 17 +++--- R/use_skill_r_code.R | 17 +++--- inst/templates/aaa-conditions.R | 58 +++++++++++++++++- man/dot-find_pattern_idx.Rd | 2 +- man/dot-pkg_abort.Rd | 19 +++++- man/dot-pkg_inform.Rd | 40 +++++++++++++ man/dot-pkg_warn.Rd | 40 +++++++++++++ man/pkgskills-package.Rd | 5 ++ tests/testthat/_snaps/use_agent.md | 10 +++- tests/testthat/_snaps/use_skill.md | 12 ++-- .../testthat/_snaps/use_skill_create_issue.md | 11 ++++ tests/testthat/_snaps/use_skill_document.md | 11 ++-- tests/testthat/_snaps/use_skill_r_code.md | 11 ++-- tests/testthat/test-use_agent.R | 13 ++-- .../test-use_github_copilot_whitelist.R | 8 ++- tests/testthat/test-use_skill.R | 29 +++++---- tests/testthat/test-use_skill_create_issue.R | 30 ++++++++++ tests/testthat/test-use_skill_document.R | 14 ++++- tests/testthat/test-use_skill_r_code.R | 14 ++++- 25 files changed, 395 insertions(+), 82 deletions(-) create mode 100644 man/dot-pkg_inform.Rd create mode 100644 man/dot-pkg_warn.Rd diff --git a/DESCRIPTION b/DESCRIPTION index f54213d..38f9b1e 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -13,7 +13,6 @@ BugReports: https://github.com/api2r/pkgskills/issues Depends: R (>= 4.1.0) Imports: - cli, desc, fs, gert, @@ -21,19 +20,23 @@ Imports: glue, purrr, rlang, - stbl (>= 0.3.0), + stbl (>= 0.3.0.9000), stringr, usethis Suggests: astgrepr, callr, + cli, knitr, rmarkdown, testthat (>= 3.0.0), withr +VignetteBuilder: + knitr +Remotes: + stbl=wranglezone/stbl Config/testthat/edition: 3 Encoding: UTF-8 Language: en-US Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.3 -VignetteBuilder: knitr +Config/roxygen2/version: 8.0.0 diff --git a/R/aaa-conditions.R b/R/aaa-conditions.R index c5c04f1..9422d22 100644 --- a/R/aaa-conditions.R +++ b/R/aaa-conditions.R @@ -1,4 +1,4 @@ -#' Raise a package-scoped error +#' Signal a package-scoped error #' #' @inheritParams .shared-params #' @inheritParams stbl::pkg_abort @@ -7,14 +7,68 @@ .pkg_abort <- function( message, subclass, + parent = NULL, call = caller_env(), - message_env = caller_env() + message_env = caller_env(), + ... ) { stbl::pkg_abort( "pkgskills", message, subclass, call = call, - message_env = message_env + message_env = message_env, + parent = parent, + ... + ) +} + +#' Signal a package-scoped warning +#' +#' @inheritParams .shared-params +#' @inheritParams stbl::pkg_warn +#' @returns Does not return. +#' @keywords internal +.pkg_warn <- function( + message, + subclass, + parent = NULL, + call = caller_env(), + message_env = caller_env(), + ... +) { + stbl::pkg_warn( + "pkgskills", + message, + subclass, + call = call, + message_env = message_env, + parent = parent, + ... + ) +} + +#' Signal a package-scoped message +#' +#' @inheritParams .shared-params +#' @inheritParams stbl::pkg_inform +#' @returns Does not return. +#' @keywords internal +.pkg_inform <- function( + message, + subclass, + parent = NULL, + call = caller_env(), + message_env = caller_env(), + ... +) { + stbl::pkg_inform( + "pkgskills", + message, + subclass, + call = call, + message_env = message_env, + parent = parent, + ... ) } diff --git a/R/use_agent.R b/R/use_agent.R index c3d3599..3e9e7bd 100644 --- a/R/use_agent.R +++ b/R/use_agent.R @@ -20,13 +20,16 @@ use_agent <- function( data <- .get_desc_fields(c("Package", "Title", "Description", "URL")) .use_template("AGENTS.md", save_as, data = data, open = open) usethis::use_build_ignore(save_as) - cli::cli_inform(c( - "{.file AGENTS.md} created.", - "i" = paste0( - "To tailor it to your project, tell your AI agent this: ", - "\"Tailor @AGENTS.md to reflect this repository's actual structure. ", - "Focus on the **Repository overview** and the **Key files** table.\"" - ) - )) + .pkg_inform( + c( + "{.file AGENTS.md} created.", + "i" = paste0( + "To tailor it to your project, tell your AI agent this: ", + "\"Tailor @AGENTS.md to reflect this repository's actual structure. ", + "Focus on the **Repository overview** and the **Key files** table.\"" + ) + ), + c("ai_implementation", "agent") + ) invisible(usethis::proj_path(save_as)) } diff --git a/R/use_github_copilot_whitelist.R b/R/use_github_copilot_whitelist.R index d9aae0a..2ed09c7 100644 --- a/R/use_github_copilot_whitelist.R +++ b/R/use_github_copilot_whitelist.R @@ -23,7 +23,10 @@ use_github_copilot_whitelist <- function( rlang::try_fetch( .set_copilot_allowlist(owner, repo, allowlist, gh_token), error = function(cnd) { - cli::cli_warn(rlang::cnd_message(cnd)) + .pkg_warn( + rlang::cnd_message(cnd), + c("allowlist_api_error") + ) .inform_copilot_allowlist(owner, repo, allowlist) } ) @@ -85,9 +88,12 @@ default_allowlist <- function() { url <- glue::glue( "https://github.com/{owner}/{repo}/settings/copilot/coding_agent/allowlist" ) - cli::cli_inform(c( - "Add the following hosts to the Copilot coding agent firewall allowlist at {.url {url}}:", - allowlist - )) + .pkg_inform( + c( + "Add the following hosts to the Copilot coding agent firewall allowlist at {.url {url}}:", + allowlist + ), + c("ai_implementation", "github_copilot", "whitelist") + ) invisible(NULL) } diff --git a/R/use_skill.R b/R/use_skill.R index d00e6cf..c5770b5 100644 --- a/R/use_skill.R +++ b/R/use_skill.R @@ -29,7 +29,10 @@ call = call ) .upsert_agents_skill_from_template(skill_path_relative, save_as, call = call) - cli::cli_inform("Skill {.file {save_as}} installed.") + .pkg_inform( + "Skill {.file {save_as}} installed.", + c("ai_implementation", "skill") + ) invisible(save_as_absolute) } diff --git a/R/use_skill_create_issue.R b/R/use_skill_create_issue.R index 98fa677..903325c 100644 --- a/R/use_skill_create_issue.R +++ b/R/use_skill_create_issue.R @@ -119,8 +119,9 @@ use_skill_create_issue <- function( repo <- url_match[[3L]] bug_reports_url <- glue::glue("https://github.com/{owner}/{repo}/issues") desc::desc_set(BugReports = bug_reports_url, normalize = TRUE) - cli::cli_inform( - "Added {.field BugReports} to {.file DESCRIPTION}: {.url {bug_reports_url}}" + .pkg_inform( + "Added {.field BugReports} to {.file DESCRIPTION}: {.url {bug_reports_url}}", + c("description_update", "bugreports") ) bug_reports_url } diff --git a/R/use_skill_document.R b/R/use_skill_document.R index a4d9b4b..d9b5780 100644 --- a/R/use_skill_document.R +++ b/R/use_skill_document.R @@ -27,13 +27,16 @@ use_skill_document <- function( shared_params_path <- usethis::proj_path("R/aaa-shared_params.R") if (!fs::file_exists(shared_params_path)) { .use_template("aaa-shared_params.R", "R/aaa-shared_params.R") - cli::cli_inform(c( - "{.file R/aaa-shared_params.R} created.", - "i" = paste( - "Add shared parameters to it as your package grows.", - "Functions can inherit shared parameters with {.code @inheritParams .shared-params}." - ) - )) + .pkg_inform( + c( + "{.file R/aaa-shared_params.R} created.", + "i" = paste( + "Add shared parameters to it as your package grows.", + "Functions can inherit shared parameters with {.code @inheritParams .shared-params}." + ) + ), + c("shared_file", "params") + ) } invisible(skill_path) diff --git a/R/use_skill_r_code.R b/R/use_skill_r_code.R index da0b15f..f63ba16 100644 --- a/R/use_skill_r_code.R +++ b/R/use_skill_r_code.R @@ -29,13 +29,16 @@ use_skill_r_code <- function( data <- .get_desc_fields("Package") .use_template("aaa-conditions.R", "R/aaa-conditions.R", data = data) usethis::use_package("stbl", min_version = "0.3.0") - cli::cli_inform(c( - "{.file R/aaa-conditions.R} created.", - "i" = paste( - "Use {.fn .pkg_abort} for package errors.", - "Add more error helpers here as your package grows." - ) - )) + .pkg_inform( + c( + "{.file R/aaa-conditions.R} created.", + "i" = paste( + "Use {.fn .pkg_abort} for package errors.", + "Add more error helpers here as your package grows." + ) + ), + c("shared_file", "conditions") + ) } invisible(skill_path) diff --git a/inst/templates/aaa-conditions.R b/inst/templates/aaa-conditions.R index 463c037..1eaa043 100644 --- a/inst/templates/aaa-conditions.R +++ b/inst/templates/aaa-conditions.R @@ -1,4 +1,4 @@ -#' Raise a package-scoped error +#' Signal a package-scoped error #' #' @inheritParams .shared-params #' @inheritParams stbl::pkg_abort @@ -7,8 +7,9 @@ .pkg_abort <- function( message, subclass, - call = rlang::caller_env(), - message_env = rlang::caller_env(), + parent = NULL, + call = caller_env(), + message_env = caller_env(), ... ) { stbl::pkg_abort( @@ -17,6 +18,57 @@ subclass, call = call, message_env = message_env, + parent = parent, + ... + ) +} + +#' Signal a package-scoped warning +#' +#' @inheritParams .shared-params +#' @inheritParams stbl::pkg_warn +#' @returns Does not return. +#' @keywords internal +.pkg_warn <- function( + message, + subclass, + parent = NULL, + call = caller_env(), + message_env = caller_env(), + ... +) { + stbl::pkg_warn( + "{{{Package}}}", + message, + subclass, + call = call, + message_env = message_env, + parent = parent, + ... + ) +} + +#' Signal a package-scoped message +#' +#' @inheritParams .shared-params +#' @inheritParams stbl::pkg_inform +#' @returns Does not return. +#' @keywords internal +.pkg_inform <- function( + message, + subclass, + parent = NULL, + call = caller_env(), + message_env = caller_env(), + ... +) { + stbl::pkg_inform( + "{{{Package}}}", + message, + subclass, + call = call, + message_env = message_env, + parent = parent, ... ) } diff --git a/man/dot-find_pattern_idx.Rd b/man/dot-find_pattern_idx.Rd index deff9f9..68a5771 100644 --- a/man/dot-find_pattern_idx.Rd +++ b/man/dot-find_pattern_idx.Rd @@ -9,7 +9,7 @@ \arguments{ \item{lines}{(\code{character}) Lines of a file, as returned by \code{\link[=readLines]{readLines()}}.} -\item{pattern}{(\code{character(1)}) A regex pattern or \code{\link[stringr:modifiers]{stringr::fixed()}} string +\item{pattern}{(\code{character(1)}) A regex pattern or \code{\link[stringr:fixed]{stringr::fixed()}} string to match against each line.} } \value{ diff --git a/man/dot-pkg_abort.Rd b/man/dot-pkg_abort.Rd index b9f0032..85a8870 100644 --- a/man/dot-pkg_abort.Rd +++ b/man/dot-pkg_abort.Rd @@ -2,9 +2,16 @@ % Please edit documentation in R/aaa-conditions.R \name{.pkg_abort} \alias{.pkg_abort} -\title{Raise a package-scoped error} +\title{Signal a package-scoped error} \usage{ -.pkg_abort(message, subclass, call = caller_env(), message_env = caller_env()) +.pkg_abort( + message, + subclass, + parent = NULL, + call = caller_env(), + message_env = caller_env(), + ... +) } \arguments{ \item{message}{(\code{character}) The message for the new error. Messages will be @@ -13,15 +20,21 @@ formatted with \code{\link[cli:cli_bullets]{cli::cli_bullets()}}.} \item{subclass}{(\code{character}) Class(es) to assign to the error. Will be prefixed by "\{package\}-error-".} +\item{parent}{A parent condition, as you might create during a +\code{\link[rlang:try_fetch]{rlang::try_fetch()}}. See \code{\link[rlang:abort]{rlang::abort()}} for additional information.} + \item{call}{(\code{environment}) The caller environment for error messages.} \item{message_env}{(\code{environment}) The execution environment to use to evaluate variables in error messages.} + +\item{...}{Additional parameters passed to \code{\link[cli:cli_abort]{cli::cli_abort()}} and on to +\code{\link[rlang:abort]{rlang::abort()}}.} } \value{ Does not return. } \description{ -Raise a package-scoped error +Signal a package-scoped error } \keyword{internal} diff --git a/man/dot-pkg_inform.Rd b/man/dot-pkg_inform.Rd new file mode 100644 index 0000000..ba6a0f1 --- /dev/null +++ b/man/dot-pkg_inform.Rd @@ -0,0 +1,40 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/aaa-conditions.R +\name{.pkg_inform} +\alias{.pkg_inform} +\title{Signal a package-scoped message} +\usage{ +.pkg_inform( + message, + subclass, + parent = NULL, + call = caller_env(), + message_env = caller_env(), + ... +) +} +\arguments{ +\item{message}{(\code{character}) The message for the new message condition. +Messages will be formatted with \code{\link[cli:cli_bullets]{cli::cli_bullets()}}.} + +\item{subclass}{(\code{character}) Class(es) to assign to the message. Will be +prefixed by "\{package\}-message-".} + +\item{parent}{A parent condition, as you might create during a +\code{\link[rlang:try_fetch]{rlang::try_fetch()}}. See \code{\link[rlang:abort]{rlang::abort()}} for additional information.} + +\item{call}{(\code{environment}) The caller environment for error messages.} + +\item{message_env}{(\code{environment}) The execution environment to use to +evaluate variables in error messages.} + +\item{...}{Additional parameters passed to \code{\link[cli:cli_abort]{cli::cli_inform()}} and on to +\code{\link[rlang:abort]{rlang::inform()}}.} +} +\value{ +Does not return. +} +\description{ +Signal a package-scoped message +} +\keyword{internal} diff --git a/man/dot-pkg_warn.Rd b/man/dot-pkg_warn.Rd new file mode 100644 index 0000000..8e62c3a --- /dev/null +++ b/man/dot-pkg_warn.Rd @@ -0,0 +1,40 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/aaa-conditions.R +\name{.pkg_warn} +\alias{.pkg_warn} +\title{Signal a package-scoped warning} +\usage{ +.pkg_warn( + message, + subclass, + parent = NULL, + call = caller_env(), + message_env = caller_env(), + ... +) +} +\arguments{ +\item{message}{(\code{character}) The message for the new warning. Messages will +be formatted with \code{\link[cli:cli_bullets]{cli::cli_bullets()}}.} + +\item{subclass}{(\code{character}) Class(es) to assign to the warning. Will be +prefixed by "\{package\}-warning-".} + +\item{parent}{A parent condition, as you might create during a +\code{\link[rlang:try_fetch]{rlang::try_fetch()}}. See \code{\link[rlang:abort]{rlang::abort()}} for additional information.} + +\item{call}{(\code{environment}) The caller environment for error messages.} + +\item{message_env}{(\code{environment}) The execution environment to use to +evaluate variables in error messages.} + +\item{...}{Additional parameters passed to \code{\link[cli:cli_abort]{cli::cli_warn()}} and on to +\code{\link[rlang:abort]{rlang::warn()}}.} +} +\value{ +Does not return. +} +\description{ +Signal a package-scoped warning +} +\keyword{internal} diff --git a/man/pkgskills-package.Rd b/man/pkgskills-package.Rd index 4cc6c2a..0d78da8 100644 --- a/man/pkgskills-package.Rd +++ b/man/pkgskills-package.Rd @@ -22,5 +22,10 @@ Useful links: \author{ \strong{Maintainer}: Jon Harmon \email{jonthegeek@gmail.com} (\href{https://orcid.org/0000-0003-4781-4346}{ORCID}) +Authors: +\itemize{ + \item Jon Harmon \email{jonthegeek@gmail.com} (\href{https://orcid.org/0000-0003-4781-4346}{ORCID}) +} + } \keyword{internal} diff --git a/tests/testthat/_snaps/use_agent.md b/tests/testthat/_snaps/use_agent.md index 17843e4..785671e 100644 --- a/tests/testthat/_snaps/use_agent.md +++ b/tests/testthat/_snaps/use_agent.md @@ -1,8 +1,12 @@ -# use_agent() emits an informational message after writing (#2) +# use_agent() emits an informational message after writing (#2, #81) Code - use_agent(open = FALSE) - Message + (expect_pkg_message_classes({ + use_agent(open = FALSE) + }, "pkgskills", "ai_implementation", "agent")) + Output + + Message in `use_agent()`: 'AGENTS.md' created. i To tailor it to your project, tell your AI agent this: "Tailor @AGENTS.md to reflect this repository's actual structure. Focus on the **Repository overview** and the **Key files** table." diff --git a/tests/testthat/_snaps/use_skill.md b/tests/testthat/_snaps/use_skill.md index 4fd21b1..9f6e8bb 100644 --- a/tests/testthat/_snaps/use_skill.md +++ b/tests/testthat/_snaps/use_skill.md @@ -1,9 +1,13 @@ -# .use_skill() emits a cli_inform message (#6) +# .use_skill() emits a message (#6, #81) Code - .use_skill("create-issue", data = list(owner = "testowner", repo = "testrepo", - repo_id = "R_test", issue_types = list()), open = FALSE) - Message + (expect_pkg_message_classes({ + .use_skill("create-issue", data = list(owner = "testowner", repo = "testrepo", + repo_id = "R_test", issue_types = list()), open = FALSE) + }, "pkgskills", "ai_implementation", "skill")) + Output + + Message in `.use_skill()`: Skill '.github/skills/create-issue/SKILL.md' installed. # .use_skill() errors when overwrite = FALSE and file exists (#6) diff --git a/tests/testthat/_snaps/use_skill_create_issue.md b/tests/testthat/_snaps/use_skill_create_issue.md index 9e63206..9c7c7ba 100644 --- a/tests/testthat/_snaps/use_skill_create_issue.md +++ b/tests/testthat/_snaps/use_skill_create_issue.md @@ -9,6 +9,17 @@ ! No BugReports field found in 'DESCRIPTION'. i Run `usethis::use_github()` to set one up. +# .bug_reports_from_remote() informs about writes (#81, #82) + + Code + (expect_pkg_message_classes({ + .extract_repo_from_desc() + }, "pkgskills", "description_update", "bugreports")) + Output + + Message in `.bug_reports_from_remote()`: + Added BugReports to 'DESCRIPTION': + # use_skill_create_issue() errors when BugReports is not a GitHub URL (#6) Code diff --git a/tests/testthat/_snaps/use_skill_document.md b/tests/testthat/_snaps/use_skill_document.md index 2621a88..e82180a 100644 --- a/tests/testthat/_snaps/use_skill_document.md +++ b/tests/testthat/_snaps/use_skill_document.md @@ -1,9 +1,12 @@ -# use_skill_document() emits message when aaa-shared_params.R is created (#9) +# use_skill_document() emits message when aaa-shared_params.R is created (#9, #81) Code - use_skill_document(open = FALSE) - Message - Skill '.github/skills/document/SKILL.md' installed. + (expect_pkg_message_classes({ + expect_message(use_skill_document(open = FALSE), class = "pkgskills-message-ai_implementation-skill") + }, "pkgskills", "shared_file", "params")) + Output + + Message in `use_skill_document()`: 'R/aaa-shared_params.R' created. i Add shared parameters to it as your package grows. Functions can inherit shared parameters with `@inheritParams .shared-params`. diff --git a/tests/testthat/_snaps/use_skill_r_code.md b/tests/testthat/_snaps/use_skill_r_code.md index 47a5c75..d84d707 100644 --- a/tests/testthat/_snaps/use_skill_r_code.md +++ b/tests/testthat/_snaps/use_skill_r_code.md @@ -1,9 +1,12 @@ -# use_skill_r_code() emits install message (#19) +# use_skill_r_code() emits install message (#19, #81) Code - use_skill_r_code(open = FALSE) - Message - Skill '.github/skills/r-code/SKILL.md' installed. + (expect_pkg_message_classes({ + expect_message(use_skill_r_code(open = FALSE), class = "pkgskills-message-ai_implementation-skill") + }, "pkgskills", "shared_file", "conditions")) + Output + + Message in `use_skill_r_code()`: 'R/aaa-conditions.R' created. i Use `.pkg_abort()` for package errors. Add more error helpers here as your package grows. diff --git a/tests/testthat/test-use_agent.R b/tests/testthat/test-use_agent.R index b5275fb..b990199 100644 --- a/tests/testthat/test-use_agent.R +++ b/tests/testthat/test-use_agent.R @@ -35,11 +35,16 @@ test_that("use_agent() does not insert 'NA' when Description or URL is absent (# expect_no_match(content, "^NA$", all = FALSE) }) -test_that("use_agent() emits an informational message after writing (#2)", { +test_that("use_agent() emits an informational message after writing (#2, #81)", { local_pkg() - expect_snapshot({ - use_agent(open = FALSE) - }) + stbl::expect_pkg_message_snapshot( + { + use_agent(open = FALSE) + }, + "pkgskills", + "ai_implementation", + "agent" + ) }) test_that("use_agent() errors on non-scalar save_as (#2)", { diff --git a/tests/testthat/test-use_github_copilot_whitelist.R b/tests/testthat/test-use_github_copilot_whitelist.R index 5607108..ca51d01 100644 --- a/tests/testthat/test-use_github_copilot_whitelist.R +++ b/tests/testthat/test-use_github_copilot_whitelist.R @@ -1,11 +1,13 @@ -test_that("use_github_copilot_whitelist() warns and informs user (#79)", { +test_that("use_github_copilot_whitelist() warns and informs user (#79, #81)", { local_pkg() expect_warning( expect_message( use_github_copilot_whitelist(gh_token = "test-token"), - "myorg/mypkg" + "myorg/mypkg", + class = "pkgskills-message-ai_implementation-github_copilot-whitelist" ), - "cannot be updated through the api" + "cannot be updated through the api", + class = "pkgskills-warning-allowlist_api_error" ) }) diff --git a/tests/testthat/test-use_skill.R b/tests/testthat/test-use_skill.R index 95f29df..ff23cae 100644 --- a/tests/testthat/test-use_skill.R +++ b/tests/testthat/test-use_skill.R @@ -100,19 +100,24 @@ test_that(".use_skill() renders template variables into skill file (#6)", { expect_true(any(grepl("IT_bug", content))) }) -test_that(".use_skill() emits a cli_inform message (#6)", { +test_that(".use_skill() emits a message (#6, #81)", { local_pkg() - expect_snapshot( - .use_skill( - "create-issue", - data = list( - owner = "testowner", - repo = "testrepo", - repo_id = "R_test", - issue_types = list() - ), - open = FALSE - ) + stbl::expect_pkg_message_snapshot( + { + .use_skill( + "create-issue", + data = list( + owner = "testowner", + repo = "testrepo", + repo_id = "R_test", + issue_types = list() + ), + open = FALSE + ) + }, + "pkgskills", + "ai_implementation", + "skill" ) }) diff --git a/tests/testthat/test-use_skill_create_issue.R b/tests/testthat/test-use_skill_create_issue.R index c5ba0db..ab70a41 100644 --- a/tests/testthat/test-use_skill_create_issue.R +++ b/tests/testthat/test-use_skill_create_issue.R @@ -33,6 +33,36 @@ test_that("use_skill_create_issue() errors when BugReports is absent (#6)", { ) }) +test_that(".bug_reports_from_remote() informs about writes (#81, #82)", { + proj_dir <- local_pkg( + DESCRIPTION = c( + "Package: mypkg", + "Title: My Package", + "Version: 0.1.0" + ) + ) + local_mocked_bindings( + git_remote_list = function(...) { + data.frame( + name = "origin", + url = "https://github.com/myorg/mypkg.git", + fetch = "+refs/heads/*:refs/remotes/origin/*", + push = "https://github.com/myorg/mypkg.git", + stringsAsFactors = FALSE + ) + }, + .package = "gert" + ) + stbl::expect_pkg_message_snapshot( + { + .extract_repo_from_desc() + }, + "pkgskills", + "description_update", + "bugreports" + ) +}) + test_that(".bug_reports_from_remote() falls back to origin GitHub remote (#82)", { proj_dir <- local_pkg( DESCRIPTION = c( diff --git a/tests/testthat/test-use_skill_document.R b/tests/testthat/test-use_skill_document.R index 9376a64..378f6bc 100644 --- a/tests/testthat/test-use_skill_document.R +++ b/tests/testthat/test-use_skill_document.R @@ -29,9 +29,19 @@ test_that("use_skill_document() does not overwrite existing R/aaa-shared_params. expect_equal(content, "# custom content") }) -test_that("use_skill_document() emits message when aaa-shared_params.R is created (#9)", { +test_that("use_skill_document() emits message when aaa-shared_params.R is created (#9, #81)", { local_pkg() - expect_snapshot(use_skill_document(open = FALSE)) + stbl::expect_pkg_message_snapshot( + { + expect_message( + use_skill_document(open = FALSE), + class = "pkgskills-message-ai_implementation-skill" + ) + }, + "pkgskills", + "shared_file", + "params" + ) }) test_that("use_skill_document() does not emit shared_params message when file exists (#9)", { diff --git a/tests/testthat/test-use_skill_r_code.R b/tests/testthat/test-use_skill_r_code.R index ccfe294..5e0c817 100644 --- a/tests/testthat/test-use_skill_r_code.R +++ b/tests/testthat/test-use_skill_r_code.R @@ -16,9 +16,19 @@ test_that("use_skill_r_code() returns path invisibly (#19)", { ) }) -test_that("use_skill_r_code() emits install message (#19)", { +test_that("use_skill_r_code() emits install message (#19, #81)", { local_pkg() - expect_snapshot(use_skill_r_code(open = FALSE)) + stbl::expect_pkg_message_snapshot( + { + expect_message( + use_skill_r_code(open = FALSE), + class = "pkgskills-message-ai_implementation-skill" + ) + }, + "pkgskills", + "shared_file", + "conditions" + ) }) test_that("use_skill_r_code() creates R/aaa-conditions.R when absent (#48)", { From f08cece4014604bade070e9a1820b26343984b27 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 12:05:19 +0000 Subject: [PATCH 2/4] docs: update condition handling guidance in skills Agent-Logs-Url: https://github.com/api2r/pkgskills/sessions/7613b6fc-6da3-422c-8fe9-8abf9430b609 Co-authored-by: jonthegeek <33983824+jonthegeek@users.noreply.github.com> --- .github/skills/r-code/SKILL.md | 6 ++--- .github/skills/tdd-workflow/SKILL.md | 26 +++++++++++++++++++++ inst/templates/skills/r-code/SKILL.md | 6 ++--- inst/templates/skills/tdd-workflow/SKILL.md | 26 +++++++++++++++++++++ 4 files changed, 58 insertions(+), 6 deletions(-) diff --git a/.github/skills/r-code/SKILL.md b/.github/skills/r-code/SKILL.md index 70350d8..707e335 100644 --- a/.github/skills/r-code/SKILL.md +++ b/.github/skills/r-code/SKILL.md @@ -6,7 +6,7 @@ description: Guide for writing R code. Use when writing new functions, designing # R code -This skill covers how to design and write R functions — including naming conventions, signatures, API conventions, input validation, error handling, and common pitfalls. For documenting functions, use the `document` skill. For tests, use the `tdd-workflow` skill. +This skill covers how to design and write R functions — including naming conventions, signatures, API conventions, input validation, condition handling, and common pitfalls. For documenting functions, use the `document` skill. For tests, use the `tdd-workflow` skill. ## Naming conventions @@ -184,9 +184,9 @@ Keep a function internal when: Internal helpers use a dot prefix (e.g. `.parse_response()`). -## Error handling +## Condition handling -Use `.pkg_abort()` (defined in `R/aaa-conditions.R`) rather than calling `cli::cli_abort()` directly. This wraps `stbl::pkg_abort()` and ensures consistent error class formatting: +Use `.pkg_abort()`, `.pkg_warn()`, and `.pkg_inform()` (defined in `R/aaa-conditions.R`) rather than calling `cli::cli_abort()`, `cli::cli_warn()`, or `cli::cli_inform()` directly. These wrap `stbl` condition helpers and ensure consistent class formatting: ```r .pkg_abort( diff --git a/.github/skills/tdd-workflow/SKILL.md b/.github/skills/tdd-workflow/SKILL.md index 0ddf177..bf07319 100644 --- a/.github/skills/tdd-workflow/SKILL.md +++ b/.github/skills/tdd-workflow/SKILL.md @@ -136,6 +136,32 @@ test_that("process_data() errors on empty input (#42)", { Pass `transform = .transform_path(path)` to scrub volatile values (e.g. temp paths) from the snapshot before comparison. +**Warnings thrown by this package** (via `.pkg_warn()`) should be tested with +`stbl::expect_pkg_warning_snapshot()`: + +```r +test_that("process_data() warns on dropped rows (#42)", { + stbl::expect_pkg_warning_snapshot( + process_data(data.frame(value = c(1, NA))), + "pkgskills", + "dropped_rows" + ) +}) +``` + +**Messages thrown by this package** (via `.pkg_inform()`) should be tested with +`stbl::expect_pkg_message_snapshot()`: + +```r +test_that("process_data() informs on defaults used (#42)", { + stbl::expect_pkg_message_snapshot( + process_data(data.frame(value = 1)), + "pkgskills", + "used_defaults" + ) +}) +``` + **Errors thrown by `stbl`** (via `stbl::to_*()` / `stbl::stabilize_*()`) should be tested with `stbl::expect_pkg_error_classes()`. Since the message text is controlled by `stbl`, only the class hierarchy needs to be asserted: diff --git a/inst/templates/skills/r-code/SKILL.md b/inst/templates/skills/r-code/SKILL.md index 70350d8..707e335 100644 --- a/inst/templates/skills/r-code/SKILL.md +++ b/inst/templates/skills/r-code/SKILL.md @@ -6,7 +6,7 @@ description: Guide for writing R code. Use when writing new functions, designing # R code -This skill covers how to design and write R functions — including naming conventions, signatures, API conventions, input validation, error handling, and common pitfalls. For documenting functions, use the `document` skill. For tests, use the `tdd-workflow` skill. +This skill covers how to design and write R functions — including naming conventions, signatures, API conventions, input validation, condition handling, and common pitfalls. For documenting functions, use the `document` skill. For tests, use the `tdd-workflow` skill. ## Naming conventions @@ -184,9 +184,9 @@ Keep a function internal when: Internal helpers use a dot prefix (e.g. `.parse_response()`). -## Error handling +## Condition handling -Use `.pkg_abort()` (defined in `R/aaa-conditions.R`) rather than calling `cli::cli_abort()` directly. This wraps `stbl::pkg_abort()` and ensures consistent error class formatting: +Use `.pkg_abort()`, `.pkg_warn()`, and `.pkg_inform()` (defined in `R/aaa-conditions.R`) rather than calling `cli::cli_abort()`, `cli::cli_warn()`, or `cli::cli_inform()` directly. These wrap `stbl` condition helpers and ensure consistent class formatting: ```r .pkg_abort( diff --git a/inst/templates/skills/tdd-workflow/SKILL.md b/inst/templates/skills/tdd-workflow/SKILL.md index be146f4..a46e353 100644 --- a/inst/templates/skills/tdd-workflow/SKILL.md +++ b/inst/templates/skills/tdd-workflow/SKILL.md @@ -136,6 +136,32 @@ test_that("process_data() errors on empty input (#42)", { Pass `transform = stbl::.transform_path(path)` to scrub volatile values (e.g. temp paths) from the snapshot before comparison. +**Warnings thrown by this package** (via `.pkg_warn()`) should be tested with +`stbl::expect_pkg_warning_snapshot()`: + +```r +test_that("process_data() warns on dropped rows (#42)", { + stbl::expect_pkg_warning_snapshot( + process_data(data.frame(value = c(1, NA))), + "{{{package}}}", + "dropped_rows" + ) +}) +``` + +**Messages thrown by this package** (via `.pkg_inform()`) should be tested with +`stbl::expect_pkg_message_snapshot()`: + +```r +test_that("process_data() informs on defaults used (#42)", { + stbl::expect_pkg_message_snapshot( + process_data(data.frame(value = 1)), + "{{{package}}}", + "used_defaults" + ) +}) +``` + **Errors thrown by `stbl`** (via `stbl::to_*()` / `stbl::stabilize_*()`) should be tested with `stbl::expect_pkg_error_classes()`. Since the message text is controlled by `stbl`, only the class hierarchy needs to be asserted: From 929f6433048f19944d970e22ac40ecb8a49971c9 Mon Sep 17 00:00:00 2001 From: Jon Harmon Date: Fri, 15 May 2026 07:56:49 -0500 Subject: [PATCH 3/4] Better messaging in `use_github_copilot_whitelist()` --- .github/skills/create-issue/SKILL.md | 12 ++++++------ R/use_github_copilot_whitelist.R | 16 ++++++++++++---- inst/templates/skills/tdd-workflow/SKILL.md | 3 --- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/.github/skills/create-issue/SKILL.md b/.github/skills/create-issue/SKILL.md index f57e7ef..2da4aaa 100644 --- a/.github/skills/create-issue/SKILL.md +++ b/.github/skills/create-issue/SKILL.md @@ -13,7 +13,7 @@ If `gh` is not authenticated, stop and ask the user to authenticate before conti ## Looking up IDs -The hardcoded IDs below are correct for this repo as of 2026-03-11. If they ever change, or if you're working in a fork, re-run these queries to get fresh values: +The hardcoded IDs below are correct for this repo as of 2026-05-15 12:52:35 UTC. If they ever change, or if you're working in a fork, re-run these queries to get fresh values: ```bash # Repository node ID @@ -29,11 +29,11 @@ Choose the type that best fits the issue: | Type | ID | Use for | |---|---|---| -| Feature | `IT_kwDOCPuMJs4BPtRe` | New exported functions or capabilities | -| Bug | `IT_kwDOCPuMJs4BPtRc` | Something broken or incorrect | -| Documentation | `IT_kwDOCPuMJs4B5OL_` | Docs-only changes | -| Task | `IT_kwDOCPuMJs4BPtRZ` | Maintenance, refactoring, chores | -| Infrastructure | `IT_kwDOCPuMJs4B5OMn` | CI, tooling, repo configuration | +| Task | `IT_kwDOCPuMJs4BPtRZ` | A specific piece of work | +| Bug | `IT_kwDOCPuMJs4BPtRc` | An unexpected problem or behavior | +| Feature | `IT_kwDOCPuMJs4BPtRe` | A request, idea, or new functionality | +| Documentation | `IT_kwDOCPuMJs4B5OL_` | Explanations of how or why to do things | +| Infrastructure | `IT_kwDOCPuMJs4B5OMn` | Infrastructure of a project, like GitHub Actions | ## Issue title diff --git a/R/use_github_copilot_whitelist.R b/R/use_github_copilot_whitelist.R index 2ed09c7..75da31b 100644 --- a/R/use_github_copilot_whitelist.R +++ b/R/use_github_copilot_whitelist.R @@ -19,15 +19,17 @@ use_github_copilot_whitelist <- function( repo_parts <- .extract_repo_from_desc() owner <- repo_parts[["owner"]] repo <- repo_parts[["repo"]] + call <- rlang::current_env() rlang::try_fetch( .set_copilot_allowlist(owner, repo, allowlist, gh_token), error = function(cnd) { .pkg_warn( rlang::cnd_message(cnd), - c("allowlist_api_error") + c("allowlist_api_error"), + call = call ) - .inform_copilot_allowlist(owner, repo, allowlist) + .inform_copilot_allowlist(owner, repo, allowlist, call = call) } ) @@ -84,7 +86,12 @@ default_allowlist <- function() { #' @inheritParams .shared-params #' @returns `NULL`, invisibly. #' @keywords internal -.inform_copilot_allowlist <- function(owner, repo, allowlist) { +.inform_copilot_allowlist <- function( + owner, + repo, + allowlist, + call = caller_env() +) { url <- glue::glue( "https://github.com/{owner}/{repo}/settings/copilot/coding_agent/allowlist" ) @@ -93,7 +100,8 @@ default_allowlist <- function() { "Add the following hosts to the Copilot coding agent firewall allowlist at {.url {url}}:", allowlist ), - c("ai_implementation", "github_copilot", "whitelist") + c("ai_implementation", "github_copilot", "whitelist"), + call = call ) invisible(NULL) } diff --git a/inst/templates/skills/tdd-workflow/SKILL.md b/inst/templates/skills/tdd-workflow/SKILL.md index a46e353..fa53b43 100644 --- a/inst/templates/skills/tdd-workflow/SKILL.md +++ b/inst/templates/skills/tdd-workflow/SKILL.md @@ -133,9 +133,6 @@ test_that("process_data() errors on empty input (#42)", { }) ``` -Pass `transform = stbl::.transform_path(path)` to scrub volatile values (e.g. temp -paths) from the snapshot before comparison. - **Warnings thrown by this package** (via `.pkg_warn()`) should be tested with `stbl::expect_pkg_warning_snapshot()`: From 357c1477a29bd541996b517173a846862903af3b Mon Sep 17 00:00:00 2001 From: Jon Harmon Date: Fri, 15 May 2026 07:59:50 -0500 Subject: [PATCH 4/4] Document. --- man/dot-inform_copilot_allowlist.Rd | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/man/dot-inform_copilot_allowlist.Rd b/man/dot-inform_copilot_allowlist.Rd index f98efa3..b0d6fac 100644 --- a/man/dot-inform_copilot_allowlist.Rd +++ b/man/dot-inform_copilot_allowlist.Rd @@ -4,7 +4,7 @@ \alias{.inform_copilot_allowlist} \title{Inform the user of the Copilot allowlist URL and entries} \usage{ -.inform_copilot_allowlist(owner, repo, allowlist) +.inform_copilot_allowlist(owner, repo, allowlist, call = caller_env()) } \arguments{ \item{owner}{(\code{character(1)}) GitHub repository owner (user or organization).} @@ -14,6 +14,8 @@ \item{allowlist}{(\code{character}) Hostnames to add to the GitHub Copilot coding agent firewall allowlist. Defaults to \code{\link[=default_allowlist]{default_allowlist()}}, a curated set of R and GitHub domains.} + +\item{call}{(\code{environment}) The caller environment for error messages.} } \value{ \code{NULL}, invisibly.