From 260b3fffb3f532b6ce6c519227e8e70a518adacc Mon Sep 17 00:00:00 2001 From: Yihui Xie <xie@yihui.name> Date: Wed, 15 Jan 2020 13:47:55 -0600 Subject: [PATCH] fix #1766: consider the case of multiple devices per chunk when calculating all_figs() (need to iterate over devices, instead of assuming a single device) --- NEWS.md | 2 ++ R/utils.R | 2 +- tests/testit/test-utils.R | 10 ++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index d826bb4ef1..9d8b5284db 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,6 +12,8 @@ - When the chunk option `dev = 'tikz'` and a plot in the code chunk is generated by **ggplot2** with a legend on a continuous scale, **tikzDevice** will create a raster image for the legend. Now the path to the raster image in the `.tex` file is tweaked to include the dir name of the `.tex` file, so that the `.tex` file can be correctly compiled to PDF via LaTeX (thanks to @rstub for the debugging at https://stackoverflow.com/a/58410965/559676). +- `hook_pdfcrop()` does not crop all plots when multiple graphical devices are used in a code chunk (i.e., the chunk option `dev` takes a vector of devices) due to a bug in the internal function `all_figs()` identified by @bastistician in #1766, who also proposed a fix. + ## MAJOR CHANGES - `options('width')` no longer affects caching. With this change, your previous cache will be invalidated if you update **knitr** from 1.26 to 1.27. If you prefer that changes in the R global option `width` invalidate cache (as in previous versions of **knitr**), you may associate it with a chunk option, e.g., `cache.extra = getOption('width')`. The reason for this change is that it is too costly to invalidate the cache when the `width` option changes---the effect of this option is only cosmetic and the code chunk output may not really rely on this option (thanks, @jaburgoyne, #1781). diff --git a/R/utils.R b/R/utils.R index 3a24ee3b48..5e20422c68 100644 --- a/R/utils.R +++ b/R/utils.R @@ -539,7 +539,7 @@ merge_list = function(x, y) { # paths of all figures all_figs = function(options, ext = options$fig.ext, num = options$fig.num) { - fig_path(ext, options, number = seq_len(num)) + unlist(lapply(ext, fig_path, options = options, number = seq_len(num))) } # evaluate an expression in a diretory and restore wd after that diff --git a/tests/testit/test-utils.R b/tests/testit/test-utils.R index 2b78a8caee..4f80df8ab9 100644 --- a/tests/testit/test-utils.R +++ b/tests/testit/test-utils.R @@ -101,6 +101,16 @@ assert( identical(fig_chunk('foo', '.pdf'), 'figure/foo-1.pdf') ) +assert('all_figs() generates all figure paths for a code chunk', { + opts = list(fig.path = 'abc/', label = 'foo', fig.num = 3) + (all_figs(opts, '.svg') %==% sprintf('abc/foo-%d.svg', 1:3)) + (all_figs(opts, c('png', 'pdf')) %==% apply( + expand.grid(1:3, c('.png', '.pdf')), 1, function(x) { + paste0(c('abc/foo-', x), collapse = '') + } + )) +}) + f = file.path(R.home('doc'), 'html', 'logo.jpg') assert( 'base64_encode() gets the same result as markdown:::.b64EncodeFile',