search

r-lib / devtools

Tools to make an R developer's life easier
https://devtools.r-lib.org
Other
2.37k stars 755 forks source link

`build_rmd()` fails with some output format #2510

Open cderv opened 1 year ago

cderv commented 1 year ago

devtools::build_rmd() is supposed to be a generic function to build any rmd

Currently it will error on some format like rmardown::pdf_document() (which could be used in vignettes too) because of https://github.com/r-lib/devtools/blob/daca0cf04413a9d251b3e55f80091afa07a60ab2/R/build-readme.R#L33-L34

> devtools::build_rmd("vignettes/demo.Rmd")
ℹ Installing dummyPkg in temporary library
ℹ Building C:/Users/chris/Documents/dummyPkg/vignettes/demo.Rmd
Error in rmarkdown::pdf_document(..., template = template) : 
  argument inutilisé (html_preview = FALSE)
Error:
! ! in callr subprocess.
Caused by error in `rmarkdown::pdf_document(..., template = template)`:
! argument inutilisé (html_preview = FALSE)

This is because html_preview is not part of pdf_document() arguments, and pdf_document() does not have an ignore ... where those unused argument can go.

We could add this ... argument, like rmarkdown::html_document_base() has, but really this option html_preview is only to be used with github_document() IMO. So maybe setting this option was supposed to be part of build_readme() and not build_rmd() ?

jennybc commented 8 months ago

I'm not so sure that build_rmd() was intended to be a generic function to build any .Rmd. I think the origin story is more that we like having build_readme() and it made sense to implement that as a special case of something slightly more generic. I suspect that the way things are is what's best, for the devtools use cases, than trying to make build_rmd() more general.

That being said, if you want to make a PR with your proposed change, we can discuss further. How is this coming up for you?