search

jhelvy / renderthis

An R package for building xaringan slides into multiple outputs, including html, pdf, png, gif, pptx, and mp4.
https://jhelvy.github.io/renderthis
Other
174 stars 12 forks source link

Add `rstudio/chromote` to dependency? #17

Closed boltomli closed 3 years ago

boltomli commented 3 years ago

chromote is used and I need to set CHROMOTE_CHROME and PAGEDOWN_CHROME for build_pdf() to work. Perhaps add it to dependency? Also mention the PATH setting so user won't have to install chrome (chromium, edge, etc. will work).

jhelvy commented 3 years ago

I've been hesitant to add chromote as a dependency since it isn't required for most functions / options. You should still be able to use build_pdf() without chromote, but you won't be able to use the complex_slides = TRUE or partial_slides = TRUE arguments (those are the options that require chromote). Also chromote isn't on CRAN yet, so the user will have to install it from Github.

For now I've put chromote under Suggests rather than Imports, along with a message inside build_pdf() that pops up with instructions for how to install it if you are using complex_slides = TRUE or partial_slides = TRUE. Hopefully that is a clear enough message to the user for now.

I will try to add additional warnings though for setting the paths for CHROMOTE_CHROME and PAGEDOWN_CHROME. I haven't had to do this, but my guess is you did something like this to get it to work: 

Sys.setenv(CHROMOTE_CHROME = "/path/to/browser")
Sys.setenv(PAGEDOWN_CHROME = "/path/to/browser")

Is that correct?

boltomli commented 3 years ago

Yes, these are exactly what I did :)

Sys.setenv(CHROMOTE_CHROME = "/path/to/browser")
Sys.setenv(PAGEDOWN_CHROME = "/path/to/browser")
gadenbuie commented 3 years ago

FWIW, @jhelvy I think you've handled this well and I'm not sure if there's much more to be done with this issue. I think Suggests is the right place for chromote even if it were on CRAN, since it introduces system requirements and isn't strictly required for xaringanBuilder to still be useful. I also think xaringanBuilder does a good job handling the Chrome path now that find_chrome() is exported from chromote and used in assert_chome_installed().

jhelvy commented 3 years ago

Okay, I'll close this.