fix(#135): evaluate @examples extraction without inline-R side effects#136
Merged
Conversation
`att_from_examples()` delegated example extraction to
`roxygen2::parse_file()`, which unconditionally evaluates inline R
embedded in roxygen markdown tags such as ``@param x `r helper("x")` ``.
roxygen2 evaluates that R in the env returned by `roxy_meta_get("env")`,
which falls back to `baseenv()` outside a full `roxygenise()` context;
any package-local helper is therefore unresolvable, producing a cascade
of "could not find function" messages on every affected tag.
Replace the parse_file call with a focused regex extractor that pulls
`@examples` / `@examplesIf` blocks straight from the source. Example
detection no longer touches roxygen2's tokenizer, so inline R inside
unrelated tags is left strictly alone.
NAMESPACE auto-regen drops the now-unused `roxygen2::parse_file` and
`roxygen2::block_get_tag_value` imports.
Bumps version to 1.0.1 with a NEWS entry.
4 tasks
There was a problem hiding this comment.
Pull request overview
Fixes att_from_examples() / att_amend_desc() failures caused by roxygen2 inline-R evaluation side effects during @examples extraction (issue #135), by avoiding roxygen2::parse_file() and extracting example blocks directly from source.
Changes:
- Replace
roxygen2::parse_file()-based example extraction with a regex-based@examples/@examplesIfblock extractor. - Add a regression test ensuring inline R in
@paramcan call a package-local helper without causing extraction failures. - Bump version to 1.0.1 and update NEWS + NAMESPACE imports accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
R/add_from_examples.R |
Switches example extraction away from roxygen2 parsing to avoid inline-R evaluation side effects. |
tests/testthat/test-att_from_namespace.R |
Adds regression coverage for issue #135 via a dummy package scenario. |
NEWS.md |
Documents the fix in the 1.0.1 changelog. |
NAMESPACE |
Removes now-unused roxygen2 imports. |
DESCRIPTION |
Bumps package version to 1.0.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on PR #136: - `extract_examples_lines()` was hard-coding `encoding = "UTF-8"` and bare-calling `readLines()`. The sibling `att_from_rscript()` uses `getOption("encoding")` and a `tryCatch()` that warns + returns `character(0)` on read failure. Mirror that pattern so a single unreadable source file no longer aborts the whole `att_from_examples()` scan, and so non-UTF8 locales (Windows) are respected by default. - Thread an `encoding` argument through `att_from_examples()` to expose the same knob users already have on `att_from_rscript()`. - NEWS: clarify that roxygen2's inline-R fallback env is `baseenv()` — base R is reachable, only package-local helpers are not.
There was a problem hiding this comment.
Pull request overview
This PR fixes a failure mode in att_from_examples()/att_amend_desc() where roxygen2 inline-R evaluation in unrelated tags (e.g. @param) can produce errors and lead to incorrect .Rd output when processed outside a full roxygenise() context.
Changes:
- Replaced
roxygen2::parse_file()-based example extraction with a source-scanning extractor to avoid inline-R evaluation side effects. - Added an
encodingparameter toatt_from_examples()(defaulting togetOption("encoding")) and documented it. - Added a regression test covering inline-R in
@paramresolving package-local helpers; bumped version and added NEWS entry.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
R/add_from_examples.R |
Switches example extraction away from roxygen2 parsing; adds encoding arg and a new internal extractor. |
man/att_from_examples.Rd |
Documents the new encoding parameter in the public API. |
tests/testthat/test-att_from_namespace.R |
Adds regression test for issue #135 using a modified dummy package. |
NEWS.md |
Adds 1.0.1 release notes describing the fix for #135. |
NAMESPACE |
Removes now-unused roxygen2 imports. |
DESCRIPTION |
Bumps package version to 1.0.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot's second review on PR #136: - `extract_examples_lines()` was including the payload of `@examplesIf` (a guard condition like `requireNamespace("foo")`) in the extracted example text, so any package mentioned only in the guard was falsely detected as a dependency. Split the tag detection so `@examplesIf` enters example mode but discards the condition; only the body lines that follow are captured. - Add a focused test asserting the @examplesIf path: the body's deps must be detected, the guard's must not. - The #135 regression test appended `Roxygen: list(markdown = TRUE)` to the dummypackage DESCRIPTION via `c(readLines(...), new_line)`, which preserved the file's trailing blank line and inserted the new field after a blank record separator. DCF parsers may then ignore it. Trim trailing empty lines first.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #135.
Problem
att_amend_desc()(viaatt_from_examples()) chokes on roxygen2 markdown that contains inline R referencing a package-local function, e.g.#' @param x `r helper("x")`Result is a cascade per affected tag:
The doc finishes writing, but every inline-R
@paramis silently skipped — output.Rdends up wrong on a cold run.Root cause
att_from_examples()delegates example extraction toroxygen2::parse_file(). Inside roxygen2,parse_tags()evaluates inline R usingroxy_meta_get("env") %||% baseenv()— and outside a fullroxygenise()call noenvis set, so the lookup falls back tobaseenv(). Any helper defined in the package being processed is unresolvable frombaseenv().Passing
env=toparse_file()does not fix it:parse_file()only sets the env on each block viablock_set_env(), but inline R evaluation readsroxy_meta_get("env")before that — the API surface roxygen2 exposes simply isn't enough to redirect inline-R lookup from outside.Fix
Replace the
parse_file()+block_get_tag_value(tag = "examples")round-trip with a focused regex extractor that pulls@examples/@examplesIfblocks straight from the source. Example detection no longer touches roxygen2's tokenizer at all, so inline R inside unrelated tags (@param,@description, …) is left strictly alone.Side benefits:
roxygen2::parse_fileandroxygen2::block_get_tag_valuedisappear fromNAMESPACE(auto-regen).Coverage
Added
tests/testthat/test-att_from_namespace.R::"inline R in @param resolves package-local functions (#135)"— fails onmain, green with this fix.Test plan
could not find function "helper")devtools::test()full suite:[ FAIL 0 | WARN 0 | SKIP 1 | PASS 392 ](modulo threetest-renv_create.Rpre-existing fails fixed in a separate PR — independent of this work)R CMD check --as-cran: 0 ERROR, 2 WARNING (vignettes/inst/doc, unrelated build artefact), 1 NOTE (CRAN incoming for version bump).Rdnow contains the evaluated inline-R value, no stderr noiseVersion
Bumps to 1.0.1 with a NEWS entry citing #135.