Escape Rd comments in markdown (#904)

Providing detailed message when you upgrade

Fixes #879

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: roxygen2
 Title: In-Line Documentation for R
-Version: 6.1.1.9000
+Version: 6.1.99.9000
 Authors@R: 
     c(person(given = "Hadley",
              family = "Wickham",
@@ -55,4 +55,4 @@ VignetteBuilder:
     knitr
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 6.1.1.9000
+RoxygenNote: 6.1.99.9000

NEWS.md

@@ -1,5 +1,9 @@
 # roxygen2 (development version)
 
+* Rd comments (`%`) are automatically escaped in markdown text. This is 
+  a backward incompatible check that will require that you replace
+  existing uses of `\%` with `%` (#879).
+
 * `@describeIn` can now be used with any combination of function types 
   (#666, #848).
 

R/markdown-escaping.R

@@ -220,15 +220,29 @@ make_random_string <- function(length = 32) {
   )
 }
 
-#' Escape \\% and \\$ and \\_ once more, because commonmark
-#' removes the escaping. We do this everywhere currently.
+#' Check markdown escaping
+#'
+#' This is a regression test for Markdown escaping.
+#'
+#' @details
+#' Each of the following bullets should look the same when rendered:
+#'
+#' * Double escapes: \\, \\%, \\$, \\_
+#' * Backticks: `\`, `\%`, `\$`, `\_`
+#' * `\verb{}`: \verb{\\}, \verb{\\%}, \verb{\$}, \verb{\_}
+#'
+#' \[ this isn't a link \]
+#' \\[ neither is this \\]
 #'
 #' @param text Input text.
 #' @return Double-escaped text.
+#' @keywords internal
 
 double_escape_md <- function(text) {
-  text <- gsub("\\%", "\\\\%", text, fixed = TRUE)
-  text <- gsub("\\_", "\\\\_", text, fixed = TRUE)
-  text <- gsub("\\$", "\\\\$", text, fixed = TRUE)
+  text <- gsub("\\", "\\\\", text, fixed = TRUE)
+
+  # De-dup escaping used to avoid [] creating a link
+  text <- gsub("\\\\[", "\\[", text, fixed = TRUE)
+  text <- gsub("\\\\]", "\\]", text, fixed = TRUE)
   text
 }

R/markdown-link.R

@@ -137,7 +137,7 @@ parse_link <- function(destination, contents) {
     )
 
   } else {
-    contents <- gsub("%", "\\\\%", mdxml_link_text(contents))
+    contents <- mdxml_link_text(contents)
 
     list(
       paste0(

R/markdown.R

@@ -39,7 +39,7 @@ mdxml_node_to_rd <- function(xml, tag) {
     unknown = mdxml_children_to_rd(xml, tag),
 
     paragraph = paste0("\n\n", mdxml_children_to_rd(xml, tag)),
-    text = xml_text(xml),
+    text = escape_comment(xml_text(xml)),
     emph = paste0("\\emph{", mdxml_children_to_rd(xml, tag), "}"),
     strong = paste0("\\strong{", mdxml_children_to_rd(xml, tag), "}"),
     softbreak = "\n",
@@ -64,11 +64,11 @@ mdxml_node_to_rd <- function(xml, tag) {
 
 mdxml_unknown <- function(xml, tag) {
   roxy_tag_warning(tag, "Unknown xml node: ", xml_name(xml))
-  xml_text(xml)
+  escape_comment(xml_text(xml))
 }
 mdxml_unsupported <- function(xml, tag, feature) {
   roxy_tag_warning(tag, "Use of ", feature, " is not currently supported")
-  xml_text(xml)
+  escape_comment(xml_text(xml))
 }
 
 mdxml_code <- function(xml, tag) {
@@ -91,7 +91,7 @@ can_parse <- function(x) {
 }
 
 escape_verb <- function(x) {
-  x <- gsub("\\", "\\\\", x, fixed = TRUE)
+  # Don't need to escape \\ because that's already handled in double_escape_md()
   x <- gsub("%", "\\%", x, fixed = TRUE)
   x <- gsub("{", "\\{", x, fixed = TRUE)
   x <- gsub("}", "\\}", x, fixed = TRUE)
@@ -135,7 +135,7 @@ mdxml_link <- function(xml) {
   if (!is.null(link)) {
     paste0(link, collapse = "")
   } else if (dest == "" || dest == xml_text(xml)) {
-    paste0("\\url{", xml_text(xml), "}")
+    paste0("\\url{", escape_comment(xml_text(xml)), "}")
   } else {
     paste0("\\href{", dest, "}{", mdxml_link_text(contents), "}")
   }
@@ -147,11 +147,15 @@ mdxml_link <- function(xml) {
 mdxml_link_text <- function(xml_contents) {
   text <- xml_text(xml_contents)
   text[xml_name(xml_contents) %in% c("linebreak", "softbreak")] <- " "
-  paste0(text, collapse = "")
+  escape_comment(paste0(text, collapse = ""))
 }
 
 mdxml_image = function(xml) {
   dest <- xml_attr(xml, "destination")
   title <- xml_attr(xml, "title")
   paste0("\\figure{", dest, "}{", title, "}")
 }
+
+escape_comment <- function(x) {
+  gsub("%", "\\%", x, fixed = TRUE)
+}

R/safety.R

@@ -51,6 +51,21 @@ update_roxygen_version <- function(base_path) {
       " You only have version ", cur, call. = FALSE, immediate. = TRUE)
   } else if (!identical(cur, prev)) {
     message("Updating roxygen version in ", desc_path)
+    upgrade_7_0_0(cur)
     desc::desc_set(RoxygenNote = cur, file = desc_path)
   }
 }
+
+upgrade_7_0_0 <- function(cur_version) {
+  if (numeric_version(cur_version) > "6.1.99") {
+    return()
+  }
+  rule <- paste0(paste(rep("-", options('width')), collapse = ""))
+  inform(c(
+    rule,
+    "Changes in roxygen2 7.0.0:",
+    "* `%` is now escaped automatically in Markdown mode.",
+    "Please carefully check .Rd files for changes",
+    rule
+  ))
+}

tests/testthat/test-markdown.R

@@ -347,23 +347,8 @@ So far so good. \\preformatted{ *these are not
   expect_equal(get_tag(out1, "description")[[2]], desc1)
 })
 
-test_that("% and $ and _ are not unescaped", {
-  out1 <- roc_proc_text(rd_roclet(), "
-    #' Title
-    #'
-    #' Description. It has some \\% and \\$ and also \\_.
-    #'
-    #' @param foo Item with \\% characters: \\%. And also \\$ and \\_.
-    foo <- function(foo) {}")[[1]]
-  out2 <- roc_proc_text(rd_roclet(), "
-    #' Title
-    #'
-    #' Description. It has some \\% and \\$ and also \\_.
-    #'
-    #' @param foo Item with \\% characters: \\%. And also \\$ and \\_.
-    #' @md
-    foo <- function(foo) {}")[[1]]
-  expect_equivalent_rd(out1, out2)
+test_that("% is automatically escaped", {
+  expect_equal(markdown("5%"), "5\\%")
 })
 
 test_that("Escaping is kept", {