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

Allow to specify LaTeX dependencies in knitr_meta #647

Merged
merged 6 commits into from Mar 22, 2016

Conversation

Projects
None yet
4 participants
@zeehio
Copy link
Contributor

zeehio commented Mar 21, 2016

This patch allows third party packages to specify LaTeX package names and options to be inserted in the final LaTeX header.

Why is it needed

At least the condformat package needs to add \usepackage[table]{xcolor} to the .tex preamble. Currently the only way to do that is to add includes: in_header: filename in the yaml header of each of the files to be rendered. A package can not add by itself LaTeX dependencies.

Proposed solution

Based on the discussion started here, I wrote this patch. Summarizing:

  • It exports a new function latex_dependency that can be used by the meta argument in knit_print as described here to add LaTeX package dependencies. A use case is found here.
  • In the pdf_preprocessor, it compiles the dependency list and generates a .tex file that is added to the args passed to pandoc using includes_to_pandoc_args. The dependency list compilation is based on how it is done for the HTML dependencies.

Contributor agreement

  • Just signed and emailed

Comments

  • There is a github note in R/pdf_document.R asking for the best way to create a temp file in rmarkdown.
if (has_latex_dependencies(knit_meta)) {
all_dependencies <- if (is.null(format_deps)) list() else format_deps
all_dependencies <- append(all_dependencies, flatten_latex_dependencies(knit_meta))
filename <- tempfile()

This comment has been minimized.

@zeehio

zeehio Mar 21, 2016

Author Contributor

Is there a specific rmarkdown way of creating a temporary file?

This comment has been minimized.

@jjallaire

jjallaire Mar 21, 2016

Member

No there isn't anything special required here.

@jjallaire

This comment has been minimized.

Copy link
Member

jjallaire commented Mar 21, 2016

Could you add a NEWS item for this feature? Thanks!

@jjallaire

This comment has been minimized.

Copy link
Member

jjallaire commented Mar 21, 2016

@yihui This looks good to me, could you review as well?

@@ -56,6 +56,9 @@ default definition or R Markdown. See the \code{\link{rmarkdown_format}} for
additional details.}

\item{pandoc_args}{Additional command line options to pass to pandoc}

\item{extra_dependencies}{Additional function arguments to pass to the
base R Markdown HTML output formatter \code{\link{html_document_base}}}

This comment has been minimized.

@yihui

yihui Mar 21, 2016

Member

You need to document this argument explicitly in roxygen instead of relying on @inheritParams html_document

R/render.R Outdated
@@ -367,6 +367,15 @@ render <- function(input,
"of this document to HTML.", call. = FALSE)
}
}
# if this isn't latex and there are latex dependencies then flag an error
if (!(identical(output_format$pandoc$to, "pdf") ||

This comment has been minimized.

@yihui

yihui Mar 21, 2016

Member

to cannot be pdf: it can only be latex or beamer (there may be other LaTeX formats in the future in Pandoc)

I'd say let's get rid of the check here for now. It makes sense but it can be messy to do it completely right, e.g. the file extension could be .tex, which is legitimate to have LaTeX dependencies.

@@ -201,7 +202,7 @@ pdf_document <- function(toc = FALSE,
# Use filter to set pdf geometry defaults (while making sure we don't override
# any geometry settings already specified by the user)
pdf_pre_processor <- function(metadata, input_file, runtime, knit_meta, files_dir,
output_dir) {
output_dir, extra_dependencies) {

This comment has been minimized.

@yihui

yihui Mar 21, 2016

Member

extra_dependencies should not be an argument of the pre-processor; you can safely leave it out here and it the extra_dependencies argument of pdf_document can be passed to the inside of this processor

This comment has been minimized.

@zeehio

zeehio Mar 21, 2016

Author Contributor

Then to prevent the RStudio IDE warning "extra_dependencies not in scope", I need to move the pdf_pre_processor function into the pdf_document function body, just like it is done for the html_document case.

This comment has been minimized.

@yihui

yihui Mar 22, 2016

Member

Okay, I didn't notice that it was a separate function. I thought it was inside pdf_document() like the pre-processor for html_document()

if (inherits(knit_meta, "latex_dependency"))
return(TRUE)

else if (is.list(knit_meta)) {

This comment has been minimized.

@yihui

yihui Mar 21, 2016

Member

else is unnecessary here due to the return() above


FALSE
} else {
FALSE

This comment has been minimized.

@yihui

yihui Mar 21, 2016

Member

The last two FALSE's can be combined into one and placed at the very end of this function

@yihui

This comment has been minimized.

Copy link
Member

yihui commented Mar 21, 2016

This looks good overall. My only concern is that unlike HTML, LaTeX is so very fragile and this approach could easily fail, e.g. you cannot \usepackage twice on the same package with different options. Even you make sure you deduplicate these LaTeX dependencies, you may still have conflicts with packages that have already been used in the Pandoc LaTeX template. That is mainly an issue with LaTeX instead of this PR, so I'm fine with getting this PR merged later.

Another possibly useful feature is like the head argument of htmlDependency(), i.e. make it possible to write arbitrary LaTeX into the preamble (e.g. think \hypersetup{}), although this can make the system even more fragile.

@yihui yihui added the Enhancement label Mar 21, 2016

@zeehio

This comment has been minimized.

Copy link
Contributor Author

zeehio commented Mar 21, 2016

I have addressed all the code comments. Regarding @yihui comment about LaTeX fragility, I totally agree with you, but there is little I can do. Other packages that want to use this feature will have to cooperate in order to avoid compatibility issues.

Given that it seems all right to you, I would rather offer a simple initial implementation than trying to address all possible issues without having practical use cases.

@jjallaire

This comment has been minimized.

Copy link
Member

jjallaire commented Mar 22, 2016

@yihui Feel free to merge once you've reviewed all of the follow up commits.

yihui added a commit that referenced this pull request Mar 22, 2016

Merge pull request #647 from zeehio/latex_dependencies
Allow to specify LaTeX dependencies in knitr_meta; closes yihui/knitr#1184

@yihui yihui merged commit 44ab159 into rstudio:master Mar 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Auburngrads

This comment has been minimized.

Copy link

Auburngrads commented Jul 29, 2016

Something is unclear about this.

I'm building a package with a custom thesis template and want to include a custom .sty file in the pdf_document. My function, afit_thesis.R, contains a call to pdf_document in which I've provided the argument extra_dependencies=latex_dependency('myPackage') where 'myPackage' has been the name of the package and the path to the package using system.file. Various attempts have so far been unsuccessful.

After reading this pull request, the rmarkdown documentation and the online documentation for pdf documents, it is not clear as to where the custom Latex package should be stored or how it should be called to ensure it is included in a .tex file.

@yihui

This comment has been minimized.

Copy link
Member

yihui commented Aug 1, 2016

@Auburngrads Typically I just put them in a .tex file, and include the .tex file in the preamble via the includes option.

output:
  pdf_document:
    includes:
      in_header: your-preamble.tex
@Auburngrads

This comment has been minimized.

Copy link

Auburngrads commented Aug 2, 2016

@yihui By 'them' I assume you mean relative paths to the .sty files.

Per the documentation for includes() the path to the .tex file must be relative to input (.Rmd) document. For custom templates I believe that this also means that the .tex & .sty files must be included in the newly created template directory.

Perhaps my problem is trying to reference an absolute file path with system.file but I want to be sure that I can push updates to template.tex and myPackage.sty without requiring the user to move files from one directory to another.

@zeehio zeehio deleted the zeehio:latex_dependencies branch Jan 9, 2017

@zeehio zeehio restored the zeehio:latex_dependencies branch Jan 9, 2017

@zeehio zeehio deleted the zeehio:latex_dependencies branch Jan 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment