-
Notifications
You must be signed in to change notification settings - Fork 35
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
Ensure on.exit() runs at end of chunk #204
Conversation
This also changes the behaviour of evaluate::evaluate(c(
"1 + 1",
"return()",
"2 + 2"
))
#> <evaluation>
#> Source code:
#> 1 + 1
#> Text output:
#> [1] 2
#> Source code:
#> return()
#> Text output:
#> NULL
#> Source code:
#> 2 + 2
#> Text output:
#> [1] 4 Now: evaluate::evaluate(c(
"1 + 1",
"return()",
"2 + 2"
))
#> <evaluation>
#> Source code:
#> 1 + 1
#> Text output:
#> [1] 2
#> Source code:
#> return() That seems like better behaviour to me, but it also has the possibility of affecting existing code. |
Also worth noting we'll need to update withr to match this behaviour. |
Just noticed that with this PR,
returns
|
Could we zap all error calls where the expression is |
@lionel- we're already doing that and it works fine within evaluate itself, but it some how leaks out when used in knitr. |
@lionel- now fixed; I don't understand why |
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 agree with the change, it makes on.exit useful, and I don't think it was for now.
Fixes #201
@cderv this has the potential to affect existing Rmds, but I think it's low risk as currently
on.exit()
runs immediately making it effectively useless, so I can't see why anyone would have called it.