block_exec_R: some problems with code as written

[abhsarma]

I see some problems with this bit of code as written:

https://github.com/MUCollective/multiverse/blob/f0d6452dddfb5c22554f43904b1319166d37b253/R/block_exec.R#L40-L51

1. It will not support code chunks that are neither R nor multiverse code chunks. This is because (1) non-R and non-multiverse code chunks have nowhere to go in this if statement and (2) for R code chunks you are calling down to your implementation of block_exec_R instead of calling down to the non-patched version in knitr, and your implementation of block_exec_R does not support non-R code chunks (while knitr::block_exec does).

2. If knitr changes block_exec internally somehow your patch effectively undoes those changes, creating a maintenance headache.

I would suggest changing the logic to something like:

if (options$engine != "multiverse") {
  # call down to whatever the original implementation of knitr::block_exec was in the loaded
  # version of the package. you will have to save that function somewhere before over-writing it
} else {
  # insert your code from above for handling multiverse code chunks
}

Then rename your block_exec_R to block_exec_multiverse and aim it solely at supporting the execution of multiverse code chunks. You shouldn't maintain a copy of the full implementation of block_exec in the multiverse package since it already exists in knitr and you can patch over and call back down to it as needed rather than copying all the code over.

Originally posted by @mjskay in https://github.com/MUCollective/multiverse/issues/58#issuecomment-660346673