Skip to content

Commit

Permalink
Ensure on.exit() runs at end of chunk (#204)
Browse files Browse the repository at this point in the history
Fixes #201
  • Loading branch information
hadley authored Jul 11, 2024
1 parent eee3073 commit 2c533a9
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 26 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# evaluate (development version)

* evaluation "chunks" now provide a function-like scope. This means that `on.exit()` will now run at the end of the evaluate code, rather than immediately and `return()` will cause the evaluation to finish (#201).
* The default `value` handler now evaluates print in a child environment of the evaluation environment. This largely makes evaluate easier to test, but should make defining S3 methods for print a little easier (#192).
* `parse_all()` adds a `\n` to the end of every line, even the last one if it didn't have one in the input.
* Setting `ACTIONS_STEP_DEBUG=1` (as in a failing GHA workflow) will automatically set `log_echo` and `log_warning` to `TRUE` (#175).
Expand Down
6 changes: 5 additions & 1 deletion R/conditions.R
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,12 @@ with_handlers <- function(code, handlers) {
}

sanitize_call <- function(cnd) {
if (identical(cnd$call, quote(eval(expr, envir)))) {
if (identical(cnd$call, quote(withVisible(do)))) {
cnd$call <- NULL
}
if (identical(cnd$call, quote(eval(as.call(list(cb)), envir)))) {
cnd$call <- NULL
}

cnd
}
65 changes: 40 additions & 25 deletions R/evaluate.R
Original file line number Diff line number Diff line change
Expand Up @@ -134,34 +134,49 @@ evaluate <- function(input,
# The user's condition handlers have priority over ours
handlers <- c(user_handlers, evaluate_handlers)

for (tle in tles) {
watcher$push_source(tle$src, tle$exprs)
if (debug || log_echo) {
cat_line(tle$src, file = stderr())
}
cb <- function() {
do <- NULL # silence R CMD check note

continue <- withRestarts(
with_handlers(
{
for (expr in tle$exprs) {
ev <- withVisible(eval(expr, envir))
watcher$capture_plot_and_output()
watcher$print_value(ev$value, ev$visible, envir)
}
TRUE
},
handlers
),
eval_continue = function() TRUE,
eval_stop = function() FALSE,
eval_error = function(cnd) stop(cnd)
)
watcher$check_devices()

if (!continue) {
break
for (tle in tles) {
watcher$push_source(tle$src, tle$exprs)
if (debug || log_echo) {
cat_line(tle$src, file = stderr())
}

continue <- withRestarts(
with_handlers(
{
for (expr in tle$exprs) {
# Using `delayedAssign()` as an interface to the C-level function
# `Rf_eval()`. Unlike the R-level `eval()`, this doesn't create
# an unwinding scope.
eval(bquote(delayedAssign("do", .(expr), eval.env = envir)))

ev <- withVisible(do)
watcher$capture_plot_and_output()
watcher$print_value(ev$value, ev$visible, envir)
}
TRUE
},
handlers
),
eval_continue = function() TRUE,
eval_stop = function() FALSE,
eval_error = function(cnd) stop(cnd)
)
watcher$check_devices()

if (!continue) {
break
}
}
}

# Here we use `eval()` to create an unwinding scope for `envir`.
# We call ourselves back immediately once the scope is created.
eval(as.call(list(cb)), envir)
watcher$capture_output()

# Always capture last plot, even if incomplete
watcher$capture_plot(TRUE)

Expand Down
4 changes: 4 additions & 0 deletions tests/testthat/test-conditions.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ test_that("all condition handlers first capture output", {
test_that("conditions get calls stripped", {
expect_equal(evaluate("warning('x')")[[2]]$call, NULL)
expect_equal(evaluate("stop('x')")[[2]]$call, NULL)

# including errors emitted by C
expect_equal(evaluate("mpg")[[2]]$call, NULL)
expect_equal(evaluate("3()")[[2]]$call, NULL)
})

test_that("envvar overrides keep_* arguments", {
Expand Down
18 changes: 18 additions & 0 deletions tests/testthat/test-evaluate.R
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,24 @@ test_that("multiple lines of comments do not lose the terminating \\n", {
expect_equal(ev[[1]]$src, "# foo\n")
})

test_that("on.exit is evaluated at end of code", {
ev <- evaluate::evaluate(c(
"on.exit(print('bye'))",
"print('hi')"
))
expect_output_types(ev, c("source", "source", "text", "text"))
})

test_that("return causes an early return", {
ev <- evaluate::evaluate(c(
"1 + 1",
"return()",
"2 + 2"
))
expect_output_types(ev, c("source", "text", "source"))
})


test_that("check_stop_on_error converts integer to enum", {
expect_equal(check_stop_on_error(0), "continue")
expect_equal(check_stop_on_error(1), "stop")
Expand Down

0 comments on commit 2c533a9

Please sign in to comment.