From 59d2ab5b29133147e963b5f560715a19be9bf6d7 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 31 Jan 2025 17:57:00 +0100 Subject: [PATCH 01/10] Use only `tools:quarto` env in search path for storing our own functions --- src/resources/rmd/execute.R | 39 +++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/resources/rmd/execute.R b/src/resources/rmd/execute.R index 42c2f479469..ebb33fc8b21 100644 --- a/src/resources/rmd/execute.R +++ b/src/resources/rmd/execute.R @@ -142,6 +142,10 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r df_print = df_print ) + # create a hidden environment to store specific objects + # Beware to use non conflicted name as this will be in second position right after globalenv. + .quarto_tools_env <- attach(NULL, name = "tools:quarto") + # we need ojs only if markdown has ojs code cells # inspect code cells for spaces after line breaks @@ -153,26 +157,31 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r needs_ojs ) { local({ - # create a hidden environment to store specific objects - .quarto_tools_env <- attach(NULL, name = "tools:quarto") # source ojs_define() function and save it in the tools environment source(file.path(resourceDir, "rmd", "ojs_static.R"), local = TRUE) assign("ojs_define", ojs_define, envir = .quarto_tools_env) }) } - - env <- globalenv() - env$.QuartoInlineRender <- function(v) { - if (is.null(v)) { - "NULL" - } else if (inherits(v, "AsIs")) { - v - } else if (is.character(v)) { - gsub(pattern="(\\[|\\]|[`*_{}()>#+-.!])", x=v, replacement="\\\\\\1") - } else { - v + + # special internal function for rendering inline code using Quarto syntax + local({ + quarto_inline_render <- function(v) { + if (is.null(v)) { + "NULL" + } else if (inherits(v, "AsIs")) { + v + } else if (is.character(v)) { + gsub(pattern="(\\[|\\]|[`*_{}()>#+-.!])", x=v, replacement="\\\\\\1") + } else { + v + } } - } + assign( + ".QuartoInlineRender", + quarto_inline_render, + envir = .quarto_tools_env + ) + }) render_output <- rmarkdown::render( input = input, @@ -180,7 +189,7 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r knit_root_dir = knit_root_dir, params = params, run_pandoc = FALSE, - envir = env + envir = globalenv() ) knit_meta <- attr(render_output, "knit_meta") files_dir <- attr(render_output, "files_dir") From f22c871e7cdc5b746f332d68dfb05cdae229e50c Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 31 Jan 2025 18:08:00 +0100 Subject: [PATCH 02/10] Add snapshot for inline code test --- .../inline-jupyter.md.snapshot | 38 +++++++++++++++++++ .../inline-execution/inline-jupyter.qmd | 5 +++ .../inline-execution/inline-knitr.md.snapshot | 38 +++++++++++++++++++ .../inline-execution/inline-knitr.qmd | 36 +++++++++--------- 4 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 tests/docs/smoke-all/inline-execution/inline-jupyter.md.snapshot create mode 100644 tests/docs/smoke-all/inline-execution/inline-knitr.md.snapshot diff --git a/tests/docs/smoke-all/inline-execution/inline-jupyter.md.snapshot b/tests/docs/smoke-all/inline-execution/inline-jupyter.md.snapshot new file mode 100644 index 00000000000..e123435acd2 --- /dev/null +++ b/tests/docs/smoke-all/inline-execution/inline-jupyter.md.snapshot @@ -0,0 +1,38 @@ +--- +_quarto: {} +title: inline jupyter expressions +toc-title: Table of contents +--- + +::: {.cell execution_count="1"} +``` {.python .cell-code} +from IPython.display import Markdown +x = 1 +y = "foo" +z = '"foo"' +a = '"foo' +b = '*foo*' +c = "*foo*" +d = "'foo" +e = "1" +f = False +g = True +h = None +i = Markdown("*foo*") +``` +::: + +Here's inline output: + +- 1 +- foo +- "foo" +- "foo +- \*foo\* +- \*foo\* +- 'foo +- 1 +- False +- True +- None +- *foo* diff --git a/tests/docs/smoke-all/inline-execution/inline-jupyter.qmd b/tests/docs/smoke-all/inline-execution/inline-jupyter.qmd index 3872ba5d0cc..2df26e8f07b 100644 --- a/tests/docs/smoke-all/inline-execution/inline-jupyter.qmd +++ b/tests/docs/smoke-all/inline-execution/inline-jupyter.qmd @@ -1,6 +1,11 @@ --- title: "inline jupyter expressions" +format: markdown engine: jupyter +_quarto: + tests: + markdown: + ensureSnapshotMatches: true --- ```{python} diff --git a/tests/docs/smoke-all/inline-execution/inline-knitr.md.snapshot b/tests/docs/smoke-all/inline-execution/inline-knitr.md.snapshot new file mode 100644 index 00000000000..06b4ce9751f --- /dev/null +++ b/tests/docs/smoke-all/inline-execution/inline-knitr.md.snapshot @@ -0,0 +1,38 @@ +--- +_quarto: {} +title: inline knitr expressions +toc-title: Table of contents +--- + +::: cell +``` {.r .cell-code} +x = 1 +y = "foo" +z = '"foo"' +a = '"foo' +b = '*foo*' +c = "*foo*" +d = "'foo" +e = "1" +f = FALSE +g = TRUE +h = NULL +i = "*foo*" +class(i) <- c("character", "asis") +``` +::: + +Here's inline output: + +- 1 +- foo +- "foo" +- "foo +- \*foo\* +- \*foo\* +- 'foo +- 1 +- FALSE +- TRUE +- NULL +- \*foo\* diff --git a/tests/docs/smoke-all/inline-execution/inline-knitr.qmd b/tests/docs/smoke-all/inline-execution/inline-knitr.qmd index 62804b069ce..06b4ce9751f 100644 --- a/tests/docs/smoke-all/inline-execution/inline-knitr.qmd +++ b/tests/docs/smoke-all/inline-execution/inline-knitr.qmd @@ -1,9 +1,11 @@ --- -title: "inline knitr expressions" -engine: knitr +_quarto: {} +title: inline knitr expressions +toc-title: Table of contents --- -```{r} +::: cell +``` {.r .cell-code} x = 1 y = "foo" z = '"foo"' @@ -18,19 +20,19 @@ h = NULL i = "*foo*" class(i) <- c("character", "asis") ``` +::: -Here's inline output: - -- `{r} x` -- `{r} y` -- `{r} z` -- `{r} a` -- `{r} b` -- `{r} c` -- `{r} d` -- `{r} e` -- `{r} f` -- `{r} g` -- `{r} h` -- `{r} i` +Here's inline output: +- 1 +- foo +- "foo" +- "foo +- \*foo\* +- \*foo\* +- 'foo +- 1 +- FALSE +- TRUE +- NULL +- \*foo\* From 799160fafe78464c637dd81d64921d95e9bb5c43 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 31 Jan 2025 18:08:23 +0100 Subject: [PATCH 03/10] fixup previous --- src/resources/rmd/execute.R | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/resources/rmd/execute.R b/src/resources/rmd/execute.R index ebb33fc8b21..860863b29c8 100644 --- a/src/resources/rmd/execute.R +++ b/src/resources/rmd/execute.R @@ -164,8 +164,7 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r } # special internal function for rendering inline code using Quarto syntax - local({ - quarto_inline_render <- function(v) { + .quarto_tools_env$.QuartoInlineRender <- function(v) { if (is.null(v)) { "NULL" } else if (inherits(v, "AsIs")) { @@ -176,12 +175,6 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r v } } - assign( - ".QuartoInlineRender", - quarto_inline_render, - envir = .quarto_tools_env - ) - }) render_output <- rmarkdown::render( input = input, From d618c2d70962accfb44ecd062029c3195fce2769 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 4 Feb 2025 15:58:24 -0600 Subject: [PATCH 04/10] Manage internal env through setter / getter / cleaner --- src/resources/rmd/execute.R | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/resources/rmd/execute.R b/src/resources/rmd/execute.R index 860863b29c8..62dcbdee0e6 100644 --- a/src/resources/rmd/execute.R +++ b/src/resources/rmd/execute.R @@ -145,6 +145,17 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r # create a hidden environment to store specific objects # Beware to use non conflicted name as this will be in second position right after globalenv. .quarto_tools_env <- attach(NULL, name = "tools:quarto") + .quarto_tools_env$.assignToQuartoToolsEnv <- function(name, value) { + assign(name, value, envir = .quarto_tools_env) + } + .quarto_tools_env$.getFromQuartoToolsEnv <- function(name) { + get0(name, envir = .quarto_tools_env) + } + .quarto_tools_env$.rmFromQuartoToolsEnv <- function(name) { + if (exists(name, envir = .quarto_tools_env)) { + rm(name, envir = .quarto_tools_env) + } + } # we need ojs only if markdown has ojs code cells # inspect code cells for spaces after line breaks From a2fc713dc94a7d36e1b84e1e02bd973d5255b776 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 4 Feb 2025 16:06:01 -0600 Subject: [PATCH 05/10] Hack around knitr 1.49 behavior With knitr 1.49, using sew.knit_asis correctly pass modified options$results = "asis" to output hooks for consideration. However, quarto checks for options$results == "asis" in more places and knitr does not modified the cell option outside of output hook context. So we need quarto to save the step from output hook context to be reused through the rest of chunk hook processing. This change correctly make asis_output() results don't have unnecessary cell div --- src/resources/rmd/hooks.R | 20 ++++++++++++++++++++ src/resources/rmd/patch.R | 7 +++++++ 2 files changed, 27 insertions(+) diff --git a/src/resources/rmd/hooks.R b/src/resources/rmd/hooks.R index 530cbd11b35..5e57e3bc053 100644 --- a/src/resources/rmd/hooks.R +++ b/src/resources/rmd/hooks.R @@ -165,6 +165,16 @@ knitr_hooks <- function(format, resourceDir, handledLanguages) { } delegating_output_hook = function(type, classes) { delegating_hook(type, function(x, options) { + ### START Knitr hack: + # since knitr 1.49, we can detect if output: asis + # was set by an R function itself (not cell option) + # We save the information for our other processing + # after output hook (i.e after sew method is called) + if (identical(options[["results"]], "asis")) { + .assignToQuartoToolsEnv("cell_options", list(asis_output = TRUE)) # nolint: object_usage_linter, line_length_linter. + } + ### END + if (identical(options[["results"]], "asis") || isTRUE(options[["collapse"]])) { x @@ -182,6 +192,16 @@ knitr_hooks <- function(format, resourceDir, handledLanguages) { # entire chunk knit_hooks$chunk <- delegating_hook("chunk", function(x, options) { + ## START knitr hack: + ## catch knit_asis output from save output hook state + asis_output <- .getFromQuartoToolsEnv("cell_options")$asis_output # nolint: object_usage_linter, line_length_linter. + if (isTRUE(asis_output)) { + options[["results"]] <- "asis" + } + # chunk hook is called last and we can clean the cell storage + on.exit(.rmFromQuartoToolsEnv("cell_options"), add = TRUE) # nolint: object_usage_linter, line_length_linter. + ## END + # Do nothing more for some specific chunk content ----- # Quarto language handler diff --git a/src/resources/rmd/patch.R b/src/resources/rmd/patch.R index addf5c8af06..ea97d799d83 100644 --- a/src/resources/rmd/patch.R +++ b/src/resources/rmd/patch.R @@ -152,6 +152,13 @@ if (utils::packageVersion("knitr") >= "1.32.8") { } else { knitr:::sew.knit_asis(x, options, ...) } + ## START knitr hack: + ## catch knit_asis output from save output hook state + asis_output <- .getFromQuartoToolsEnv("cell_options")$asis_output # nolint: object_usage_linter, line_length_linter + if (isTRUE(asis_output)) { + options[["results"]] <- "asis" + } + ## END # if it's an html widget then it was already wrapped # by add_html_caption From 864dbd726157e370f660663cc2b57aa7f572e661 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 4 Feb 2025 16:10:10 -0600 Subject: [PATCH 06/10] source ojs_define directly in the tool environment --- src/resources/rmd/execute.R | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/resources/rmd/execute.R b/src/resources/rmd/execute.R index 62dcbdee0e6..5ee19296b7c 100644 --- a/src/resources/rmd/execute.R +++ b/src/resources/rmd/execute.R @@ -167,11 +167,11 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r !is_shiny_prerendered(knitr::opts_knit$get("rmarkdown.runtime")) && needs_ojs ) { - local({ - # source ojs_define() function and save it in the tools environment - source(file.path(resourceDir, "rmd", "ojs_static.R"), local = TRUE) - assign("ojs_define", ojs_define, envir = .quarto_tools_env) - }) + # source ojs_define() function into the tools environment + source( + file = file.path(resourceDir, "rmd", "ojs_static.R"), + local = .quarto_tools_env + ) } # special internal function for rendering inline code using Quarto syntax From 40ae02c2490395084dfd7b6c275a93629b5219f6 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 4 Feb 2025 16:16:43 -0600 Subject: [PATCH 07/10] assign .QuartoInlineRender into new tools env --- src/resources/rmd/execute.R | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/resources/rmd/execute.R b/src/resources/rmd/execute.R index 5ee19296b7c..9fe66b178e8 100644 --- a/src/resources/rmd/execute.R +++ b/src/resources/rmd/execute.R @@ -157,9 +157,24 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r } } + # special internal function for rendering inline code using Quarto syntax + .assignToQuartoToolsEnv(".QuartoInlineRender", function(v) { # nolint: object_usage_linter, line_length_linter. + if (is.null(v)) { + "NULL" + } else if (inherits(v, "AsIs")) { + v + } else if (is.character(v)) { + gsub( + pattern = "(\\[|\\]|[`*_{}()>#+-.!])", + x = v, replacement = "\\\\\\1" + ) + } else { + v + } + }) + # we need ojs only if markdown has ojs code cells # inspect code cells for spaces after line breaks - needs_ojs <- grepl("(\n|^)[[:space:]]*```+\\{ojs[^}]*\\}", markdown) # FIXME this test isn't failing in shiny mode, but it doesn't look to be # breaking quarto-shiny-ojs. We should make sure this is right. @@ -173,19 +188,6 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r local = .quarto_tools_env ) } - - # special internal function for rendering inline code using Quarto syntax - .quarto_tools_env$.QuartoInlineRender <- function(v) { - if (is.null(v)) { - "NULL" - } else if (inherits(v, "AsIs")) { - v - } else if (is.character(v)) { - gsub(pattern="(\\[|\\]|[`*_{}()>#+-.!])", x=v, replacement="\\\\\\1") - } else { - v - } - } render_output <- rmarkdown::render( input = input, From 91dd70690c441a6e77c834bace38dcfab84742f2 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 5 Feb 2025 11:11:30 -0600 Subject: [PATCH 08/10] correctly clean tool env --- src/resources/rmd/execute.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resources/rmd/execute.R b/src/resources/rmd/execute.R index 9fe66b178e8..98ccc440cd0 100644 --- a/src/resources/rmd/execute.R +++ b/src/resources/rmd/execute.R @@ -153,7 +153,7 @@ execute <- function(input, format, tempDir, libDir, dependencies, cwd, params, r } .quarto_tools_env$.rmFromQuartoToolsEnv <- function(name) { if (exists(name, envir = .quarto_tools_env)) { - rm(name, envir = .quarto_tools_env) + rm(list = c(name), envir = .quarto_tools_env) } } From e4844e84f5cce11aabb9ed382fc56eb2ef089023 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 5 Feb 2025 11:12:02 -0600 Subject: [PATCH 09/10] format some code --- src/resources/rmd/hooks.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/resources/rmd/hooks.R b/src/resources/rmd/hooks.R index 5e57e3bc053..a1f5afd53c1 100644 --- a/src/resources/rmd/hooks.R +++ b/src/resources/rmd/hooks.R @@ -420,7 +420,7 @@ knitr_hooks <- function(format, resourceDir, handledLanguages) { # if there is a label, additional classes, a forwardAttr, or a cell.cap # then the user is deemed to have implicitly overridden results = "asis" # (as those features don't work w/o an enclosing div) - needCell <- isTRUE(nzchar(label)) || + needCell <- isTRUE(nzchar(label)) || length(classes) > 1 || isTRUE(nzchar(forwardAttr)) || isTRUE(nzchar(cell.cap)) @@ -429,7 +429,8 @@ knitr_hooks <- function(format, resourceDir, handledLanguages) { } else { paste0( options[["indent"]], "::: {", - labelId(label), paste(classes, collapse = " ") ,forwardAttr, "}\n", x, "\n", cell.cap , + labelId(label), paste(classes, collapse = " "), + forwardAttr, "}\n", x, "\n", cell.cap, options[["indent"]], ":::" ) } From a9d096b0a68ad95ae25a198a720842493d813915 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 5 Feb 2025 11:13:50 -0600 Subject: [PATCH 10/10] don't apply asis output if some option implicitly required wrapping cell div like if html-table-processing none has been set --- src/resources/rmd/patch.R | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/resources/rmd/patch.R b/src/resources/rmd/patch.R index ea97d799d83..fdcd714e848 100644 --- a/src/resources/rmd/patch.R +++ b/src/resources/rmd/patch.R @@ -103,7 +103,7 @@ wrap_asis_output <- function(options, x) { if (identical(options[["html-table-processing"]], "none")) { attrs <- paste(attrs, "html-table-processing=none") } - + # if this is an html table then wrap it further in ```{=html} # (necessary b/c we no longer do this by overriding kable_html, # which is in turn necessary to allow kableExtra to parse @@ -112,9 +112,17 @@ wrap_asis_output <- function(options, x) { !grepl('^
', x)) { x <- paste0("`````{=html}\n", x, "\n`````") } - - # If asis output, don't include the output div - if (identical(options[["results"]], "asis")) return(x) + + # If asis output, don't include the output div, + # unless a feature requiring it is used + # if there additional classes, some added attr, + # then the user is deemed to have implicitly overridden results = "asis" + # (as those features don't work w/o an enclosing div) + needCell <- length(classes) > 1 || isTRUE(nzchar(attrs)) # nolint: object_name_linter, line_length_linter. + + if (identical(options[["results"]], "asis") && !needCell) { + return(x) + } output_div(x, output_label_placeholder(options), classes, attrs) }