From 796e6e9b2687abc072b54cabc427f5343455b227 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 07:49:53 +0100
Subject: [PATCH 01/10] Move check of active device to watchout

---
 R/eval.R    | 3 ---
 R/watcher.R | 6 ++++++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/R/eval.R b/R/eval.R
index 734b2ad..a9dff17 100644
--- a/R/eval.R
+++ b/R/eval.R
@@ -154,10 +154,7 @@ evaluate_top_level_expression <- function(exprs,
   source <- new_source(src, exprs[[1]], output_handler$source)
   output <- list(source)
 
-  dev <- dev.cur()
   handle_output <- function(plot = TRUE, incomplete_plots = FALSE) {
-    # if dev.cur() has changed, we should not record plots any more
-    plot <- plot && identical(dev, dev.cur())
     out <- watcher(plot, incomplete_plots)
     output <<- c(output, out)
   }
diff --git a/R/watcher.R b/R/watcher.R
index 19c0ca1..68708d4 100644
--- a/R/watcher.R
+++ b/R/watcher.R
@@ -10,6 +10,9 @@
 watchout <- function(handler = new_output_handler(),
                      debug = FALSE,
                      frame = parent.frame()) {
+  
+  dev <- dev.cur()
+
   con <- file("", "w+b")
   defer(frame = frame, {
     if (!test_con(con, isOpen)) {
@@ -25,6 +28,9 @@ watchout <- function(handler = new_output_handler(),
   defer(options(old), frame = frame)
 
   function(plot = TRUE, incomplete_plots = FALSE) {
+    # if dev.cur() has changed, we should not record plots any more
+    plot <- plot && identical(dev, dev.cur())
+
     out <- list(
       if (plot) plot_snapshot(incomplete_plots),
       read_con(con)

From 36628ba54172cdce770f1f9f0fe26e2c3368440e Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 07:57:48 +0100
Subject: [PATCH 02/10] Separate output and plot capture

---
 R/eval.R    |  7 +++++--
 R/watcher.R | 36 +++++++++++++++++++++---------------
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/R/eval.R b/R/eval.R
index a9dff17..c9c4e34 100644
--- a/R/eval.R
+++ b/R/eval.R
@@ -155,8 +155,11 @@ evaluate_top_level_expression <- function(exprs,
   output <- list(source)
 
   handle_output <- function(plot = TRUE, incomplete_plots = FALSE) {
-    out <- watcher(plot, incomplete_plots)
-    output <<- c(output, out)
+    out <- list(
+      if (plot) watcher$capture_plot(incomplete_plots),
+      watcher$capture_output()
+    )
+    output <<- c(output, compact(out))
   }
 
   local_output_handler(function() handle_output(FALSE))
diff --git a/R/watcher.R b/R/watcher.R
index 68708d4..dc484c4 100644
--- a/R/watcher.R
+++ b/R/watcher.R
@@ -1,16 +1,14 @@
 #' Watch for changes in output, text and graphics
 #'
-#' @param debug activate debug mode where output will be both printed to
-#'   screen and captured.
 #' @param handler An ouptut handler object.
 #' @param frame When this frame terminates, the watcher will automatically close.` 
 #' @return list containing four functions: `get_new`, `pause`,
 #'  `unpause`, `close`.
+#' @inheritParams evaluate
 #' @keywords internal
 watchout <- function(handler = new_output_handler(),
                      debug = FALSE,
                      frame = parent.frame()) {
-  
   dev <- dev.cur()
 
   con <- file("", "w+b")
@@ -27,23 +25,31 @@ watchout <- function(handler = new_output_handler(),
   old <- options(try.outFile = con)
   defer(options(old), frame = frame)
 
-  function(plot = TRUE, incomplete_plots = FALSE) {
+  capture_plot <- function(incomplete = FALSE) {
     # if dev.cur() has changed, we should not record plots any more
-    plot <- plot && identical(dev, dev.cur())
+    if (!identical(dev, dev.cur())) {
+      return()
+    }
 
-    out <- list(
-      if (plot) plot_snapshot(incomplete_plots),
-      read_con(con)
-    )
-    if (!is.null(out[[1]])) {
-      handler$graphics(out[[1]])
+    out <- plot_snapshot(incomplete)
+    if (!is.null(out)) {
+      handler$graphics(out)
     }
-    if (!is.null(out[[2]])) {
-      handler$text(out[[2]])
+    out
+  }
+
+  capture_output <- function() {
+    out <- read_con(con)
+    if (!is.null(out)) {
+      handler$text(out)
     }
-    
-    compact(out)
+    out
   }
+  
+  list(
+    capture_plot = capture_plot,
+    capture_output = capture_output
+  )
 }
 
 read_con <- function(con, buffer = 32 * 1024) {

From be153f8b27a4e3851b3aa4fecfb3b9a91a66e185 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 08:34:52 +0100
Subject: [PATCH 03/10] Record plot state in watcher

---
 R/eval.R     |  3 ---
 R/graphics.R | 33 ---------------------------------
 R/watcher.R  | 38 ++++++++++++++++++++++++++++++++++----
 3 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/R/eval.R b/R/eval.R
index c9c4e34..9d08e1b 100644
--- a/R/eval.R
+++ b/R/eval.R
@@ -82,9 +82,6 @@ evaluate <- function(input,
   devn <- length(.env$dev_list <- dev.list())
   dev <- dev.cur()
 
-  # clean up the last_plot object after an evaluate() call (cf yihui/knitr#722)
-  on.exit(assign("last_plot", NULL, envir = environment(plot_snapshot)), add = TRUE)
-
   # if this env var is set to true, always bypass messages
   if (tolower(Sys.getenv('R_EVALUATE_BYPASS_MESSAGES')) == 'true')
     keep_message = keep_warning = NA
diff --git a/R/graphics.R b/R/graphics.R
index a1a8734..669be1a 100644
--- a/R/graphics.R
+++ b/R/graphics.R
@@ -6,39 +6,6 @@
 #"
 #" @return \code{NULL} if plot is blank or unchanged, otherwise the output of
 #"   \code{\link{recordPlot}}.
-plot_snapshot <- local({
-  last_plot <- NULL
-
-  function(incomplete = FALSE) {
-    devs <- dev.list()
-    # No graphics devices 
-    if (is.null(devs)) {
-      return()
-    }
-
-    # Current graphics device changed since evaluate started
-    if (!identical(devs, .env$dev_list)) {
-      return()
-    }
-
-    # current page is incomplete
-    if (!par("page") && !incomplete) {
-      return()
-    }
-
-    plot <- recordPlot()
-    if (!makes_visual_change(plot[[1]])) {
-      return()
-    }
-
-    if (!looks_different(last_plot[[1]], plot[[1]])) {
-      return()
-    }
-
-    last_plot <<- plot
-    plot
-  }
-})
 
 looks_different <- function(old_dl, new_dl) {
   if (identical(old_dl, new_dl)) {
diff --git a/R/watcher.R b/R/watcher.R
index dc484c4..3047766 100644
--- a/R/watcher.R
+++ b/R/watcher.R
@@ -10,6 +10,7 @@ watchout <- function(handler = new_output_handler(),
                      debug = FALSE,
                      frame = parent.frame()) {
   dev <- dev.cur()
+  last_plot <- NULL
 
   con <- file("", "w+b")
   defer(frame = frame, {
@@ -31,11 +32,12 @@ watchout <- function(handler = new_output_handler(),
       return()
     }
 
-    out <- plot_snapshot(incomplete)
-    if (!is.null(out)) {
-      handler$graphics(out)
+    new_plot <- plot_snapshot(last_plot, incomplete)
+    if (!is.null(new_plot)) {
+      last_plot <<- new_plot
+      handler$graphics(new_plot)
     }
-    out
+    new_plot
   }
 
   capture_output <- function() {
@@ -73,3 +75,31 @@ test_con = function(con, test) {
 con_error = function(x) stop(
   x, '... Please make sure not to call closeAllConnections().', call. = FALSE
 )
+
+plot_snapshot <- function(last_plot, incomplete = FALSE) {
+  devs <- dev.list()
+  # No graphics devices 
+  if (is.null(devs)) {
+    return()
+  }
+
+  # Current graphics device changed since evaluate started
+  if (!identical(devs, .env$dev_list)) {
+    return()
+  }
+
+  # current page is incomplete
+  if (!par("page") && !incomplete) {
+    return()
+  }
+
+  plot <- recordPlot()
+  if (!makes_visual_change(plot[[1]])) {
+    return()
+  }
+
+  if (!looks_different(last_plot[[1]], plot[[1]])) {
+    return()
+  }
+  plot
+}

From 1754e2429dcb546287d6883257ad24bb917507ad Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 08:35:46 +0100
Subject: [PATCH 04/10] Move devices check up

---
 R/watcher.R | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/R/watcher.R b/R/watcher.R
index 3047766..de7f3f6 100644
--- a/R/watcher.R
+++ b/R/watcher.R
@@ -31,6 +31,15 @@ watchout <- function(handler = new_output_handler(),
     if (!identical(dev, dev.cur())) {
       return()
     }
+    devs <- dev.list()
+    # No graphics devices 
+    if (is.null(devs)) {
+      return()
+    }
+    # Current graphics device changed since evaluate started
+    if (!identical(devs, .env$dev_list)) {
+      return()
+    }  
 
     new_plot <- plot_snapshot(last_plot, incomplete)
     if (!is.null(new_plot)) {
@@ -77,17 +86,6 @@ con_error = function(x) stop(
 )
 
 plot_snapshot <- function(last_plot, incomplete = FALSE) {
-  devs <- dev.list()
-  # No graphics devices 
-  if (is.null(devs)) {
-    return()
-  }
-
-  # Current graphics device changed since evaluate started
-  if (!identical(devs, .env$dev_list)) {
-    return()
-  }
-
   # current page is incomplete
   if (!par("page") && !incomplete) {
     return()

From 0748e3c3347b6c4892ea2b4c8fef748360cd6d5b Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 08:40:06 +0100
Subject: [PATCH 05/10] Record current devices in watchout

---
 R/eval.R    | 11 +----------
 R/watcher.R | 27 +++++++++++++++++++++------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/R/eval.R b/R/eval.R
index 9d08e1b..9ff0b9c 100644
--- a/R/eval.R
+++ b/R/eval.R
@@ -77,10 +77,6 @@ evaluate <- function(input,
     dev <- dev.cur()
     on.exit(dev.off(dev))
   }
-  # record the list of current devices
-  devs <- .env$dev_list; on.exit(.env$dev_list <- devs, add = TRUE)
-  devn <- length(.env$dev_list <- dev.list())
-  dev <- dev.cur()
 
   # if this env var is set to true, always bypass messages
   if (tolower(Sys.getenv('R_EVALUATE_BYPASS_MESSAGES')) == 'true')
@@ -94,12 +90,6 @@ evaluate <- function(input,
     if (debug) {
       message(parsed$src[[i]])
     }
-
-    # if dev.off() was called, make sure to restore device to the one opened by
-    # evaluate() or existed before evaluate()
-    if (length(dev.list()) < devn) dev.set(dev)
-    devn <- length(dev.list())
-
     out[[i]] <- evaluate_top_level_expression(
       exprs = parsed$expr[[i]],
       src = parsed$src[[i]],
@@ -114,6 +104,7 @@ evaluate <- function(input,
       output_handler = output_handler,
       include_timing = include_timing
     )
+    watcher$check_devices()
 
     if (stop_on_error > 0L) {
       errs <- vapply(out[[i]], is.error, logical(1))
diff --git a/R/watcher.R b/R/watcher.R
index de7f3f6..d71314c 100644
--- a/R/watcher.R
+++ b/R/watcher.R
@@ -9,9 +9,13 @@
 watchout <- function(handler = new_output_handler(),
                      debug = FALSE,
                      frame = parent.frame()) {
-  dev <- dev.cur()
   last_plot <- NULL
 
+  # record current devices
+  devs <- dev.list()
+  devn <- length(devs)
+  dev <- dev.cur()
+
   con <- file("", "w+b")
   defer(frame = frame, {
     if (!test_con(con, isOpen)) {
@@ -31,15 +35,15 @@ watchout <- function(handler = new_output_handler(),
     if (!identical(dev, dev.cur())) {
       return()
     }
-    devs <- dev.list()
+    cur_devs <- dev.list()
     # No graphics devices 
-    if (is.null(devs)) {
+    if (is.null(cur_devs)) {
       return()
     }
     # Current graphics device changed since evaluate started
-    if (!identical(devs, .env$dev_list)) {
+    if (!identical(cur_devs, devs)) {
       return()
-    }  
+    }
 
     new_plot <- plot_snapshot(last_plot, incomplete)
     if (!is.null(new_plot)) {
@@ -56,10 +60,21 @@ watchout <- function(handler = new_output_handler(),
     }
     out
   }
+
+  check_devices <- function() {
+    # if dev.off() was called, make sure to restore device to the one opened by
+    # evaluate() or existed before evaluate()
+    if (length(dev.list()) < devn) {
+      dev.set(dev)
+    }
+    devn <<- length(dev.list())
+    invisible()
+  }
   
   list(
     capture_plot = capture_plot,
-    capture_output = capture_output
+    capture_output = capture_output,
+    check_devices = check_devices
   )
 }
 

From a7bd64335a4493e5a2e5ee57d1bc2f15713742db Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 08:46:35 +0100
Subject: [PATCH 06/10] Inline plot snapshot

---
 NEWS.md         |  1 +
 R/watcher.R     | 66 +++++++++++++++++++------------------------------
 man/watchout.Rd | 24 ------------------
 3 files changed, 27 insertions(+), 64 deletions(-)
 delete mode 100644 man/watchout.Rd

diff --git a/NEWS.md b/NEWS.md
index 427b746..aceb1f5 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,6 @@
 # evaluate (development version)
 
+* `watchout()` is no longer exported; it's really an implementation detail that should never have been leaked to the public interface.
 * `evaluate()` gains an output class (`evaluate_evaluation`/`list`) and a basic print method.
 * `evaluate()` now correctly captures plots created before messages/warnings/errors (#28).
 
diff --git a/R/watcher.R b/R/watcher.R
index d71314c..abe4889 100644
--- a/R/watcher.R
+++ b/R/watcher.R
@@ -1,11 +1,3 @@
-#' Watch for changes in output, text and graphics
-#'
-#' @param handler An ouptut handler object.
-#' @param frame When this frame terminates, the watcher will automatically close.` 
-#' @return list containing four functions: `get_new`, `pause`,
-#'  `unpause`, `close`.
-#' @inheritParams evaluate
-#' @keywords internal
 watchout <- function(handler = new_output_handler(),
                      debug = FALSE,
                      frame = parent.frame()) {
@@ -31,26 +23,37 @@ watchout <- function(handler = new_output_handler(),
   defer(options(old), frame = frame)
 
   capture_plot <- function(incomplete = FALSE) {
-    # if dev.cur() has changed, we should not record plots any more
-    if (!identical(dev, dev.cur())) {
+    # only record plots for our graphics device
+    if (!identical(dev.cur(), dev)) {
       return()
     }
-    cur_devs <- dev.list()
-    # No graphics devices 
-    if (is.null(cur_devs)) {
+    # cur_devs <- dev.list()
+    # # No graphics devices 
+    # if (is.null(cur_devs)) {
+    #   return()
+    # }
+    # # Current graphics device changed since evaluate started
+    # if (!identical(cur_devs, devs)) {
+    #   return()
+    # }
+
+    # current page is incomplete
+    if (!par("page") && !incomplete) {
       return()
     }
-    # Current graphics device changed since evaluate started
-    if (!identical(cur_devs, devs)) {
+
+    plot <- recordPlot()
+    if (!makes_visual_change(plot[[1]])) {
       return()
     }
-
-    new_plot <- plot_snapshot(last_plot, incomplete)
-    if (!is.null(new_plot)) {
-      last_plot <<- new_plot
-      handler$graphics(new_plot)
+  
+    if (!looks_different(last_plot[[1]], plot[[1]])) {
+      return()
     }
-    new_plot
+
+    last_plot <<- plot
+    handler$graphics(plot)
+    plot
   }
 
   capture_output <- function() {
@@ -62,8 +65,8 @@ watchout <- function(handler = new_output_handler(),
   }
 
   check_devices <- function() {
-    # if dev.off() was called, make sure to restore device to the one opened by
-    # evaluate() or existed before evaluate()
+    # if dev.off() was called, make sure to restore device to the one opened 
+    # when watchout() was called
     if (length(dev.list()) < devn) {
       dev.set(dev)
     }
@@ -99,20 +102,3 @@ test_con = function(con, test) {
 con_error = function(x) stop(
   x, '... Please make sure not to call closeAllConnections().', call. = FALSE
 )
-
-plot_snapshot <- function(last_plot, incomplete = FALSE) {
-  # current page is incomplete
-  if (!par("page") && !incomplete) {
-    return()
-  }
-
-  plot <- recordPlot()
-  if (!makes_visual_change(plot[[1]])) {
-    return()
-  }
-
-  if (!looks_different(last_plot[[1]], plot[[1]])) {
-    return()
-  }
-  plot
-}
diff --git a/man/watchout.Rd b/man/watchout.Rd
deleted file mode 100644
index 75dab40..0000000
--- a/man/watchout.Rd
+++ /dev/null
@@ -1,24 +0,0 @@
-% Generated by roxygen2: do not edit by hand
-% Please edit documentation in R/watcher.R
-\name{watchout}
-\alias{watchout}
-\title{Watch for changes in output, text and graphics}
-\usage{
-watchout(handler = new_output_handler(), debug = FALSE, frame = parent.frame())
-}
-\arguments{
-\item{handler}{An ouptut handler object.}
-
-\item{debug}{activate debug mode where output will be both printed to
-screen and captured.}
-
-\item{frame}{When this frame terminates, the watcher will automatically close.`}
-}
-\value{
-list containing four functions: \code{get_new}, \code{pause},
-\code{unpause}, \code{close}.
-}
-\description{
-Watch for changes in output, text and graphics
-}
-\keyword{internal}

From 07a4f7af1996ca213d2071d4630cc40b4b599e83 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 08:48:10 +0100
Subject: [PATCH 07/10] Also handle creation of graphics device

---
 R/eval.R    | 11 +----------
 R/watcher.R | 10 ++++++++++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/R/eval.R b/R/eval.R
index 9ff0b9c..dc0013d 100644
--- a/R/eval.R
+++ b/R/eval.R
@@ -69,21 +69,12 @@ evaluate <- function(input,
   }
   local_inject_funs(envir)
 
-  if (new_device) {
-    # Ensure we have a graphics device available for recording, but choose
-    # one that's available on all platforms and doesn't write to disk
-    pdf(file = NULL)
-    dev.control(displaylist = "enable")
-    dev <- dev.cur()
-    on.exit(dev.off(dev))
-  }
-
   # if this env var is set to true, always bypass messages
   if (tolower(Sys.getenv('R_EVALUATE_BYPASS_MESSAGES')) == 'true')
     keep_message = keep_warning = NA
 
   # Capture output
-  watcher <- watchout(output_handler, debug = debug)
+  watcher <- watchout(output_handler, new_device = new_device, debug = debug)
 
   out <- vector("list", nrow(parsed))
   for (i in seq_along(out)) {
diff --git a/R/watcher.R b/R/watcher.R
index abe4889..72ee3a4 100644
--- a/R/watcher.R
+++ b/R/watcher.R
@@ -1,8 +1,18 @@
 watchout <- function(handler = new_output_handler(),
+                     new_device = TRUE,
                      debug = FALSE,
                      frame = parent.frame()) {
   last_plot <- NULL
 
+  if (new_device) {
+    # Ensure we have a graphics device available for recording, but choose
+    # one that's available on all platforms and doesn't write to disk
+    pdf(file = NULL)
+    dev.control(displaylist = "enable")
+    dev <- dev.cur()
+    defer(dev.off(dev), frame)
+  }
+
   # record current devices
   devs <- dev.list()
   devn <- length(devs)

From d0ac039b8f5bea361c7cff4547e8bff4d406d6b7 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 08:52:42 +0100
Subject: [PATCH 08/10] Remove outdated comment

---
 R/graphics.R | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/R/graphics.R b/R/graphics.R
index 669be1a..1575342 100644
--- a/R/graphics.R
+++ b/R/graphics.R
@@ -1,12 +1,3 @@
-#" Capture snapshot of current device.
-#"
-#" There's currently no way to capture when a graphics device changes,
-#" except to check its contents after the evaluation of every expression.
-#" This means that only the last plot of a series will be captured.
-#"
-#" @return \code{NULL} if plot is blank or unchanged, otherwise the output of
-#"   \code{\link{recordPlot}}.
-
 looks_different <- function(old_dl, new_dl) {
   if (identical(old_dl, new_dl)) {
     return(FALSE)

From 27e3983d2d2bee9ca1679a74f8cff88be4e3dd56 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 10:09:54 +0100
Subject: [PATCH 09/10] Simplify test

---
 tests/testthat/test-graphics.R | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/testthat/test-graphics.R b/tests/testthat/test-graphics.R
index aefacf2..67201b3 100644
--- a/tests/testthat/test-graphics.R
+++ b/tests/testthat/test-graphics.R
@@ -185,8 +185,10 @@ test_that("an incomplete plot with a comment in the end is also captured", {
 })
 
 # a bug report yihui/knitr#722
-test_that("repeatedly drawing the same plot does not omit plots randomly", {
-  expect_true(all(replicate(100, length(evaluate("plot(1:10)"))) == 2))
+test_that("plot state doesn't persist over evaluate calls", {
+  expect_output_types(evaluate("plot(1)"), c("source", "plot"))
+  expect_output_types(evaluate("plot(1)"), c("source", "plot"))
+  expect_output_types(evaluate("plot(1)"), c("source", "plot"))
 })
 
 # test_that("no plot windows open", {

From f39fbcd5e9e0627b8479ff21d22ebf905594229a Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 19 Jun 2024 11:43:59 +0100
Subject: [PATCH 10/10] Drop unneeded code

---
 R/watcher.R | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/R/watcher.R b/R/watcher.R
index 72ee3a4..e2d6da5 100644
--- a/R/watcher.R
+++ b/R/watcher.R
@@ -37,15 +37,6 @@ watchout <- function(handler = new_output_handler(),
     if (!identical(dev.cur(), dev)) {
       return()
     }
-    # cur_devs <- dev.list()
-    # # No graphics devices 
-    # if (is.null(cur_devs)) {
-    #   return()
-    # }
-    # # Current graphics device changed since evaluate started
-    # if (!identical(cur_devs, devs)) {
-    #   return()
-    # }
 
     # current page is incomplete
     if (!par("page") && !incomplete) {