fixed knit_child when knitting chunks with blocks that produce multiple ...

[kforner]

fixed knit_child when knitting chunks with blocks that produce multiple plots.
cf #824

It was difficult writing a test because I could not manage to run your testit tests without running R CMD check, and you do not seem to use sample files in your tests (is there any reason not to use testthat ?)

To test it manually:

library(devtools)
load_all('knitr')
res <- knit('parent.Rnw', quiet = TRUE)
lines <- readLines(res)
nb1 <- sum(grepl('includegraphics', lines, ignore.case = TRUE))
cat('nb plots=', nb1, '\n')

with
parent.Rnw

\documentclass[letter, margin=1.90cm, 12pt]{scrartcl}
\begin{document}

<<parent_call_child, message = FALSE, include = FALSE, results = 'asis', eval = TRUE>>=
library(ggplot2)
out <- knitr::knit_child('child.Rnw', options = list(fig.keep='all'))
@
\Sexpr{paste(out, collapse="\n")}
\end{document}

child.Rnw

\section{PLOTS}
<<plots_for_loop, message = FALSE, echo = FALSE, results = 'asis', fig.keep = 'all', fig.show = 'asis', eval = TRUE>>=
{
  print(ggplot(iris, aes(x = Species)) + geom_histogram(fill = 'green') + ggtitle(paste('PLOT CHILD 1')))
  print(ggplot(iris, aes(x = Species)) + geom_histogram(fill = 'green') + ggtitle(paste('PLOT CHILD 2')))
}
@

The old knitr would output 1, the fixed one 2.

[kforner]

Hi. Any chance that this fix gets merged into master soon ?

[yihui]

Merged. Thanks!

Regarding the tests, you can just run test-all.R, e.g.

cd tests
Rscript run-all.R

or in R:

setwd('tests')
source('run-all.R')

I do not use testthat because I'm not interested in its new vocabulary (I'm familiar with R's native vocabulary such as identical() and all.equal(), and I do not want to memorize what expect_identical() and expect_equal() exactly mean). More here: http://yihui.name/en/2013/09/testing-r-packages/

[yihui]

Well, this is not really a safe fix. The existing hooks should not be set to NULL. We should only remove the previous hooks added by the previous evaluate() call. I'll fix it.

[kforner]

We should only remove the previous hooks added by the previous evaluate() call

You mean that for each hook, you will remove the last function, assuming that it has been added by evaluate ?
What about multiple levels of evaluate recursion ? We are actually using at least 3 levels.

[yihui]

Yes. It can be complicated. I have fixed the problem in the evaluate package.

[kforner]

Yes and indeed it seems to work pretty well. thanks !!