Revert change to using manual context

Closes #210. Fixes #214. Closes #219. Fixes #220. Closes #221.

NEWS.md

@@ -1,7 +1,6 @@
 # evaluate (development version)
 
 * New `trim_intermediate_plots()` drops intermediate plots to reveal the complete/final plot (#206).
-* 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).

R/conditions.R

@@ -51,10 +51,7 @@ with_handlers <- function(code, handlers) {
 }
 
 sanitize_call <- function(cnd) {
-  if (identical(cnd$call, quote(withVisible(do)))) {
-    cnd$call <- NULL
-  }
-  if (identical(cnd$call, quote(eval(as.call(list(context)), envir)))) {
+  if (identical(cnd$call, quote(eval(expr, envir)))) {
     cnd$call <- NULL
   }
 

R/evaluate.R

@@ -132,48 +132,34 @@ evaluate <- function(input,
   # The user's condition handlers have priority over ours
   handlers <- c(user_handlers, evaluate_handlers)
 
-  context <- function() {
-    do <- NULL # silence R CMD check note
-
-    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
-      }
+  for (tle in tles) {
+    watcher$push_source(tle$src, tle$exprs)
+    if (debug || log_echo) {
+      cat_line(tle$src, file = stderr())
     }
-  }
 
-  # 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(context)), envir)
-  watcher$capture_output()
+    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) signalCondition(cnd)
+    )
+    watcher$check_devices()
+
+    if (!continue) {
+      break
+    }
+  }
 
   # Always capture last plot, even if incomplete
   watcher$capture_plot(TRUE)

tests/testthat/test-evaluate.R

@@ -96,24 +96,6 @@ 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")