-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix deferred_run()
in knitr
#251
Conversation
Should we include this in a vignette or test it some other way? |
6abfb1c
to
40a6fd9
Compare
I added a snapshot test before and now I've also turned on evaluation for the |
@@ -311,28 +311,21 @@ It's hard to develop with functions that work one way inside a function, but ano | |||
|
|||
Here's how `defer()` (and all functions based on it) works in an interactive session. | |||
|
|||
```{r eval = FALSE} | |||
```{r} | |||
library(withr) | |||
|
|||
defer(print("hi")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to add this comment back as this example is about running in the global environment https://withr.r-lib.org/dev/articles/changing-and-restoring-state.html#deferring-events-on-the-global-environment, and we should show the user what they get. I think you could add that in a separate chunk that said "because this code is running in a vignette it doesn't look exactly the same as what you'll see. When you run it interactively, you'll also see this message: ..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, done in 546d0c2
Branched from #250.
Closes #235.
Throwing an error causes a testthat vignette to fail (see r-lib/testthat#1920). And now that we no longer use
exit_frame()
as the public interface for handling environments specially, it's easier to introduce special-casing for knitr. We now use the old approach of attaching a list of handlers to the knitr exit frame. This way our handlers are not mixed with knitr's own handlers and we can flush them separately.Fixes the testthat failure in revdeps.