Skip to content

Commit

Permalink
Fix interactions of dynamic code and fragile tags
Browse files Browse the repository at this point in the history
Before this, we evaluated dynamic after the fragile tags
were removed (set aside temporarily). This is problematic,
because these tags were also removed from within inline
code and code blocks, and more importantly, you could
not reliably return fragile tags from the dynamic code.

After this patch the order is this:
1. Parse the tag as markdown, find the inline code
   and the code blocks, execute them, substitute
   back the result.
2. Set aside the fragile tags, temporarily.
3. Parse as markdown again, generate Rd.
4. Put back the fragile tags.

This is a breaking change in theory, however dynamic
code is relatively recent, and the affected documentation
is somewhat obscure, so we can probably get away with it.
The potentially affected cases are:
* People having dynamic code in fragile tags. E.g.

    #' blah \out{`r expression`} blah

  This will now evaluate the expression.
* People generating fragile tags dynamically. E.g. as in
  the original post of #1115
  We actually improve this use case with this patch.

Closes #1115.
  • Loading branch information
gaborcsardi committed Jun 15, 2020
1 parent d148c84 commit c72632c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 6 deletions.
11 changes: 6 additions & 5 deletions R/markdown.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@

markdown <- function(text, tag = NULL, sections = FALSE) {
tag <- tag %||% list(file = NA, line = NA)
tryCatch(
expanded_text <- markdown_pass1(text),
error = function(e) {
Expand All @@ -10,7 +11,8 @@ markdown <- function(text, tag = NULL, sections = FALSE) {
stop(message, call. = FALSE)
}
)
markdown_pass2(expanded_text, tag = tag, sections = sections)
escaped_text <- escape_rd_for_md(expanded_text)
markdown_pass2(escaped_text, tag = tag, sections = sections)
}

#' Expand the embedded inline code
Expand Down Expand Up @@ -59,14 +61,13 @@ markdown <- function(text, tag = NULL, sections = FALSE) {

markdown_pass1 <- function(text) {
text <- paste(text, collapse = "\n")
esc_text <- escape_rd_for_md(text)
mdxml <- xml_ns_strip(md_to_mdxml(esc_text, sourcepos = TRUE))
mdxml <- xml_ns_strip(md_to_mdxml(text, sourcepos = TRUE))
code_nodes <- xml_find_all(mdxml, ".//code | .//code_block")
rcode_nodes <- keep(code_nodes, is_markdown_code_node)
if (length(rcode_nodes) == 0) return(esc_text)
if (length(rcode_nodes) == 0) return(text)
rcode_pos <- parse_md_pos(map_chr(rcode_nodes, xml_attr, "sourcepos"))
out <- eval_code_nodes(rcode_nodes)
str_set_all_pos(esc_text, rcode_pos, out, rcode_nodes)
str_set_all_pos(text, rcode_pos, out, rcode_nodes)
}

is_markdown_code_node <- function(x) {
Expand Down
18 changes: 18 additions & 0 deletions tests/testthat/test-markdown-code.R
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,21 @@ test_that("fence options are used", {
details <- out1$get_value("details")
expect_false(grepl("Error", details))
})

test_that("dynamic code in fragile tags still runs", {
out <- markdown("foo \\out{`r 1+1`} bar")
expect_equal(out, "foo \\out{2} bar")
})

test_that("fragile tags in dynamic code are left alone", {
out <- markdown("foo `r substr('\\\\out{xxx}', 2, 4)` bar")
expect_equal(out, "foo out bar")
})

test_that("fragile tags in generated code", {
out <- markdown("foo `r '\\\\out{*1*}'` bar")
expect_equal(out, "foo \\out{*1*} bar")

expect_silent(out2 <- markdown("foo `r '\\\\out{<span></span>}'` bar"))
expect_equal(out2, "foo \\out{<span></span>} bar")
})
2 changes: 1 addition & 1 deletion tests/testthat/testNamespace/DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ Author: Hadley <hadley@rstudio.com>
Maintainer: Hadley <hadley@rstudio.com>
Encoding: UTF-8
Version: 0.1
RoxygenNote: 7.0.2.9000
RoxygenNote: 7.1.0.9000

0 comments on commit c72632c

Please sign in to comment.