From af96b1bbac4715384d9612a07fdcca8ed76b5579 Mon Sep 17 00:00:00 2001 From: atusy <30277794+atusy@users.noreply.github.com> Date: Fri, 26 May 2023 10:19:15 +0900 Subject: [PATCH 1/7] feat: implement merge_file_scope --- R/output_format.R | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/R/output_format.R b/R/output_format.R index 17a7bd4780..b3e12a6e4a 100644 --- a/R/output_format.R +++ b/R/output_format.R @@ -52,9 +52,11 @@ #' @param file_scope A function that will split markdown input to pandoc into #' multiple named files. This is useful when the caller has concatenated a set #' of Rmd files together (as \pkg{bookdown} does), and those files may need to -#' processed by pandoc using the \code{--file-scope} option. The function -#' should return a named list of files w/ \code{name} and \code{content} for -#' each file. +#' processed by pandoc using the \code{--file-scope} option. The first +#' argument is input file paths and the second is \code{NULL} or current file +#' scope which is a named list of files w/ \code{name} and \code{content} for +#' each file. The return is the new file scope. Also, the arguments should +#' include \code{...} for the future extensions. #' @param base_format An optional format to extend. #' @return An R Markdown output format definition that can be passed to #' \code{\link{render}}. @@ -157,6 +159,26 @@ merge_post_processors <- function(base, } } +merge_file_scope <- function(base, + overlay) { + if (is.null(overlay)) { + return(base) + } + has_ellipsis <- "..." %in% names(formals(overlay)) + if (!has_ellipsis) { + warning("file_scope lacks ... as an argument. ", + "Otherwise, file_scope replaces file_scope without merging.") + } + if (is.null(base) || !has_ellipsis) { + return(overlay) + } + if (has_ellipsis) { + return(function(x, current_scope = NULL, ...) { + overlay(x, base(x, current_scope, ...), ...) + }) + } +} + # merges two output formats merge_output_formats <- function(base, overlay) { @@ -181,6 +203,7 @@ merge_output_formats <- function(base, overlay$intermediates_generator, c), post_processor = merge_post_processors(base$post_processor, overlay$post_processor), + file_scope = merge_file_scope(base$file_scope, overlay$file_scope), on_exit = merge_on_exit(base$on_exit, overlay$on_exit) ), class = "rmarkdown_output_format") From 7b67febe5ca664f6bf10999331587a7800a0a293 Mon Sep 17 00:00:00 2001 From: atusy <30277794+atusy@users.noreply.github.com> Date: Fri, 26 May 2023 10:28:05 +0900 Subject: [PATCH 2/7] feat: extend file_scope_split to support merging file_scope --- R/render.R | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/R/render.R b/R/render.R index 9d73610d8c..0033e45e52 100644 --- a/R/render.R +++ b/R/render.R @@ -1200,7 +1200,11 @@ resolve_df_print <- function(df_print) { output_metadata = knitr:::new_defaults() file_scope_split <- function(input, fun) { - inputs <- fun(input) + inputs <- if (length(formals(fun)) == 1L) { + fun(input) # for the backward compatibility + } else { + fun(input, NULL) # the second argument implies current file scope + } # file_scope_fun should split the input file in several # do nothing if not and return input file unsplited From d6ca0d4980512778d09af6ad465925841684d0f5 Mon Sep 17 00:00:00 2001 From: atusy <30277794+atusy@users.noreply.github.com> Date: Fri, 26 May 2023 10:30:09 +0900 Subject: [PATCH 3/7] test: file_scope merging behaviors --- tests/testthat/test-output_format.R | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/testthat/test-output_format.R b/tests/testthat/test-output_format.R index f70a34f2d2..b870639194 100644 --- a/tests/testthat/test-output_format.R +++ b/tests/testthat/test-output_format.R @@ -7,6 +7,7 @@ test_that("all elements can be NULL", { test_that("inherits base format", { base_fmt <- html_document() + base_fmt$file_scope <- identity out_fmt <- output_format( knitr = NULL, pandoc = NULL, keep_md = NULL, clean_supporting = NULL, base_format = base_fmt @@ -17,6 +18,23 @@ test_that("inherits base format", { expect_identical(out_fmt[not_fun], base_fmt[not_fun]) }) +test_that("file_scope replaces base format", { + base_fmt <- html_document() + base_fmt$file_scope <- function(input_file, file_scope, ...) { + return(list("foo" = "foo")) + } + out_fmt <- output_format( + knitr = NULL, pandoc = NULL, keep_md = NULL, clean_supporting = NULL, + file_scope = function(input_file, file_scope, ...) { + file_scope$bar <- "bar" + return(file_scope) + }, + base_format = base_fmt + ) + expect_identical(out_fmt$file_scope("input", NULL), + list("foo" = "foo", "bar" = "bar")) +}) + test_that("clean_supporting is coerced to FALSE only if keep_md is TRUE", { args_combinations <- expand.grid( knitr = list(NULL), From faa6ef803d6a669ecc0f6cca06dee279688270e8 Mon Sep 17 00:00:00 2001 From: atusy <30277794+atusy@users.noreply.github.com> Date: Fri, 26 May 2023 10:38:12 +0900 Subject: [PATCH 4/7] chore: devtools::document() --- man/output_format.Rd | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/man/output_format.Rd b/man/output_format.Rd index f545b4a4f0..c895d9491c 100644 --- a/man/output_format.Rd +++ b/man/output_format.Rd @@ -82,9 +82,11 @@ execution (as registered with a \code{\link{on.exit}} handler).} \item{file_scope}{A function that will split markdown input to pandoc into multiple named files. This is useful when the caller has concatenated a set of Rmd files together (as \pkg{bookdown} does), and those files may need to -processed by pandoc using the \code{--file-scope} option. The function -should return a named list of files w/ \code{name} and \code{content} for -each file.} +processed by pandoc using the \code{--file-scope} option. The first +argument is input file paths and the second is \code{NULL} or current file +scope which is a named list of files w/ \code{name} and \code{content} for +each file. The return is the new file scope. Also, the arguments should +include \code{...} for the future extensions.} \item{base_format}{An optional format to extend.} } From 4da5b3091850db094b83255fd1bfa2136b9adb49 Mon Sep 17 00:00:00 2001 From: atusy <30277794+atusy@users.noreply.github.com> Date: Fri, 26 May 2023 10:41:57 +0900 Subject: [PATCH 5/7] chore: add news --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index ed5aea9f88..d85c603b06 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,8 @@ rmarkdown 2.22 - Mentions that **webshot** or **webshot2** is required to take screenshot of HTML widget. When not installed, an error message mentionning `always_allow_html: true` solution will be shown, but setting this is not the solution (quarto-dev/quarto-cli#4225). +- Fixed `file_scope` being lost when extending output formats that considers the `file_scope`. Also, `file_scope` gains second argument which receives the returned values of the base `file_scope` (#2488). + rmarkdown 2.21 ================================================================================ From 428054d00d0128fb32d3d6695bede105ac77a4db Mon Sep 17 00:00:00 2001 From: atusy <30277794+atusy@users.noreply.github.com> Date: Fri, 26 May 2023 20:52:23 +0900 Subject: [PATCH 6/7] fix: ensure base file scope is evaluated prior to evaluating overlay --- R/output_format.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/output_format.R b/R/output_format.R index b3e12a6e4a..b7fa3043c7 100644 --- a/R/output_format.R +++ b/R/output_format.R @@ -174,7 +174,8 @@ merge_file_scope <- function(base, } if (has_ellipsis) { return(function(x, current_scope = NULL, ...) { - overlay(x, base(x, current_scope, ...), ...) + scope <- base(x, current_scope, ...) + overlay(x, scope, ...) }) } } From 2f67f41ff708b9844cc7f98e7c312c652dc154fa Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 11 Jul 2023 12:24:23 +0200 Subject: [PATCH 7/7] Bump version [skip ci] --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 375ce3a7e9..21e9390b6b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: rmarkdown Title: Dynamic Documents for R -Version: 2.23.1 +Version: 2.23.2 Authors@R: c( person("JJ", "Allaire", , "jj@posit.co", role = "aut"), person("Yihui", "Xie", , "xie@yihui.name", role = c("aut", "cre"), comment = c(ORCID = "0000-0003-0645-5666")),