diff --git a/NEWS.md b/NEWS.md index 388ad6fc..0a10473d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # roxygen2 (development version) +* Markdown processing has been rewritten around a single tokenizer for the combined markdown/Rd grammar, replacing the old escape/unescape passes. Processing large documentation files is now substantially faster, and Rd tags behave consistently in every markdown context. +* Markdown text containing `\%` now renders as `%`; previously it produced `\\%` in the Rd file, which truncated the displayed line at the `%`. +* Markdown links with an escaped closing bracket (e.g. `[bar\]`) no longer leak a link reference definition into the generated Rd. +* An Rd tag with an unterminated argument (e.g. `\code{a % b}`, where the unescaped `%` comments out the closing brace) now generates a warning instead of being silently reinterpreted as markdown. * Markdown processing now handles multibyte characters inside Rd tags correctly; previously a tag like `\code{café}` would corrupt the markdown interpretation of the text that followed it. * Markdown warnings triggered by a `rd_family_title` prefix (e.g. for an unsupported level 1 heading) no longer error. * Markdown link targets are now resolved against a per-run index of each package's help topics, instead of one `help()` call per topic and package. This substantially speeds up documenting packages with many cross-reference links, e.g. `roxygenize()` on testthat is about a third faster. diff --git a/R/cpp11.R b/R/cpp11.R index 634b6f2a..6adf9bf6 100644 --- a/R/cpp11.R +++ b/R/cpp11.R @@ -24,6 +24,10 @@ find_includes <- function(path) { .Call(`_roxygen2_find_includes`, path) } +tokenizeMd <- function(text, verbatim) { + .Call(`_roxygen2_tokenizeMd`, text, verbatim) +} + wrapUsage <- function(string, width, indent) { .Call(`_roxygen2_wrapUsage`, string, width, indent) } diff --git a/R/markdown-escaping.R b/R/markdown-escaping.R deleted file mode 100644 index a8c5b6e8..00000000 --- a/R/markdown-escaping.R +++ /dev/null @@ -1,293 +0,0 @@ -#' Escape fragile Rd tags -#' -#' @description -#' `escape_rd_for_md()` replaces fragile Rd tags with placeholders, to avoid -#' interpreting them as markdown. `unescape_rd_for_md()` puts the original -#' text back in place of the placeholders after the markdown parsing is done. -#' The fragile tags are listed in `escaped_for_md`. A fragile tag is -#' protected together with all of its brace arguments, so markdown is never -#' interpreted inside them. -#' -#' @param text Input text. Potentially contains Rd and/or -#' markdown markup. -#' @returns -#' * `escape_rd_for_md`: a "safe" version of the input text, where -#' each fragile Rd tag is replaced by a placeholder. The -#' original text is added as an attribute for each placeholder. -#' * `unescape_rd_for_md`: the original Rd text. -#' @rdname markdown-internals -#' @keywords internal -escape_rd_for_md <- function(text) { - rd_tags <- find_fragile_rd_tags(text, escaped_for_md) - protected <- protect_rd_tags(text, rd_tags) - double_escape_md(protected) -} - -escaped_for_md <- paste0( - "\\", - c( - "acronym", - "code", - "command", - "CRANpkg", - "deqn", - "doi", - "dontrun", - "dontshow", - "donttest", - "email", - "env", - "eqn", - "figure", - "file", - "if", - "ifelse", - "kbd", - "link", - "linkS4class", - "method", - "mjeqn", - "mjdeqn", - "mjseqn", - "mjsdeqn", - "mjteqn", - "mjtdeqn", - "newcommand", - "option", - "out", - "packageAuthor", - "packageDescription", - "packageDESCRIPTION", - "packageIndices", - "packageMaintainer", - "packageTitle", - "pkg", - "PR", - "preformatted", - "renewcommand", - "S3method", - "S4method", - "samp", - "special", - "testonly", - "url", - "var", - "verb" - ) -) - -#' @param rd_text The markdown parsed and interpreted text. -#' @param esc_text The original escaped text from -#' `escape_rd_for_md()`. -#' @rdname markdown-internals -unescape_rd_for_md <- function(rd_text, esc_text) { - id <- attr(esc_text, "roxygen-markdown-subst")$id - tags <- attr(esc_text, "roxygen-markdown-subst")$tags - - for (i in seq_len(nrow(tags))) { - ph <- paste0(id, "-", i, "-") - rd_text <- sub(ph, tags$text[i], rd_text, fixed = TRUE) - } - - rd_text -} - -#' Find all fragile tags (int the supplied list) in the text -#' -#' Ignore the tags that are embedded into a fragile tag. -#' -#' @param text Input text, character scalar. -#' @param fragile Character vector of fragile tags to find. -#' @return Data frame of fragile tags, with columns: -#' `tag`, `start`, `end`, `argend`, -#' `text`. -#' -#' @noRd - -find_fragile_rd_tags <- function(text, fragile) { - tags <- find_all_rd_tags(text) - ftags <- tags[tags$tag %in% fragile, ] - - ## Remove embedded ones - keep <- map_lgl(seq_len(nrow(ftags)), function(i) { - sum(ftags$start <= ftags$start[i] & ftags$argend >= ftags$argend[i]) == 1 - }) - - ftags <- ftags[keep, ] - - if (nrow(ftags)) { - ftags$text <- substring(text, ftags$start, ftags$argend) - } - - ftags -} - -#' Find all (complete) Rd tags in a string -#' -#' Complete means that we include the argument(s) as well. -#' -#' @param text Input text, character scalar. -#' -#' @noRd - -find_all_rd_tags <- function(text) { - text_len <- nchar(text) - - ## Find the tag names - tags <- find_all_tag_names(text) - - ## Find the end of the argument list for each tag. Note that - ## tags might be embedded into the arguments of other tags. - tags$argend <- map_int(seq_len(nrow(tags)), function(i) { - tag_plus <- substr(text, tags$end[i], text_len) - findEndOfTag(tag_plus, is_code = FALSE, start = 0L) + tags$end[i] - }) - - tags -} - -#' Find all tag names in a string -#' -#' Note that we also protect these tags within code, strings -#' and comments, for now. We'll see if this causes any -#' problems. -#' -#' @param text Input text, scalar. -#' @return Data frame, with columns: `tag`, `start`, -#' `end`. -#' -#' @noRd - -find_all_tag_names <- function(text) { - ## Find the tags without arguments first - m <- gregexpr(r"(\\[a-zA-Z][a-zA-Z0-9]*)", text)[[1]] - if (m[[1]] == -1L) { - tag_pos <- matrix( - integer(), - ncol = 2, - dimnames = list(NULL, c("start", "end")) - ) - } else { - tag_pos <- cbind( - start = as.integer(m), - end = as.integer(m) + attr(m, "match.length") - 1L - ) - } - - if (nrow(tag_pos) == 0) { - data.frame(tag = character(), start = integer(), end = integer()) - } else { - data.frame( - tag = substring(text, tag_pos[, "start"], tag_pos[, "end"]), - as.data.frame(tag_pos) - ) - } -} - -#' Replace fragile Rd tags with placeholders -#' -#' @param text The text, character scalar. -#' @param rd_tags Fragile Rd tags, in a data frame, -#' as returned by `find_fragile_rd_tags`. -#' @return Text, after the substitution. The original -#' text is added as an attribute. -#' -#' @noRd - -protect_rd_tags <- function(text, rd_tags) { - id <- make_random_string() - - text <- re_sub_same(text, rd_tags, id) - - attr(text, "roxygen-markdown-subst") <- - list(tags = rd_tags, id = id) - - text -} - -#' Replace parts of the same string -#' -#' It assumes that the intervals to be replaced do not -#' overlap. Gives an error otherwise. -#' -#' @param str String scalar. -#' @param repl Data frame with columns: `start`, `end`, -#' `argend`, `text`. -#' @param id Placeholder string. -#' @return Input string with the replacements performed. -#' Note that all replacements are performed in parallel, -#' at least conceptually. -#' -#' @noRd - -re_sub_same <- function(str, repl, id) { - repl <- repl[order(repl$start), ] - - if (is.unsorted(repl$end) || is.unsorted(repl$argend)) { - cli::cli_abort("Replacement intervals must not overlap.", .internal = TRUE) - } - - for (i in seq_len(nrow(repl))) { - ## The trailing - is needed, to distinguish between -1 and -10 - new_text <- paste0(id, "-", i, "-") - str <- paste0( - substr(str, 1, repl$start[i] - 1), - new_text, - substr(str, repl$argend[i] + 1, nchar(str)) - ) - - ## Need to shift other coordinates (we shift everything, - ## it is just simpler). - inc <- nchar(new_text) - (repl$argend[i] - repl$start[i] + 1) - repl$start <- repl$start + inc - repl$end <- repl$end + inc - repl$argend <- repl$argend + inc - } - - str -} - -#' Make a random string -#' -#' We use this as the placeholder, to make sure that the -#' placeholder does not appear in the text. -#' -#' @return String scalar -#' -#' @noRd - -make_random_string <- function(length = 32) { - paste( - sample(c(LETTERS, letters, 0:9), length, replace = TRUE), - collapse = "" - ) -} - -#' Check markdown escaping -#' -#' This is a regression test for Markdown escaping. -#' -#' @details -#' Each of the following bullets should look the same when rendered: -#' -#' * Backticks: `\`, `\%`, `\$`, `\_` -#' * `\verb{}`: \verb{\\}, \verb{\\%}, \verb{\$}, \verb{\_} -#' -#' \[ this isn't a link \] -#' \\[ neither is this \\] -#' -#' @param text Input text. -#' @return Double-escaped text. -#' @keywords internal -#' @examples -#' "%" # percent -#' "\"" # double quote -#' '\'' # single quote -double_escape_md <- function(text) { - text <- gsub(r"(\)", r"(\\)", text, fixed = TRUE) - - # De-dup escaping used to avoid [] creating a link - text <- gsub(r"(\\[)", r"(\[)", text, fixed = TRUE) - text <- gsub(r"(\\])", r"(\])", text, fixed = TRUE) - text -} diff --git a/R/markdown-link.R b/R/markdown-link.R index f916c42a..7478e73f 100644 --- a/R/markdown-link.R +++ b/R/markdown-link.R @@ -70,8 +70,9 @@ get_md_linkrefs <- function(text) { paste0( "(?x)", "(?<=[^\\]\\\\]|^)", # must not be preceded by ] or \ - "\\[([^\\]\\[]+)\\]", # match anything inside of [] - "(?:\\[([^\\]\\[]+)\\])?", # match optional second pair of [] + # match anything inside of [], not ending with \ (an escaped bracket) + "\\[([^\\]\\[]*[^\\]\\[\\\\])\\]", + "(?:\\[([^\\]\\[]*[^\\]\\[\\\\])\\])?", # optional second pair of [] "(?=[^\\[{]|$)" # must not be followed by [ or { ), text, @@ -101,6 +102,7 @@ parse_link <- function(destination, contents, state) { return(NULL) } destination <- sub("^R:", "", URLdecode(destination)) + Encoding(destination) <- "UTF-8" # restore encoding dropped URLdecodse ## if contents is a `code tag`, then we need to move this outside is_code <- FALSE diff --git a/R/markdown-tokenize.R b/R/markdown-tokenize.R new file mode 100644 index 00000000..83ba1abb --- /dev/null +++ b/R/markdown-tokenize.R @@ -0,0 +1,120 @@ +# Tokenizing the combined markdown/Rd grammar +# +# `md_tokenize()` replaces every backslash-initiated construct with an +# inert placeholder, "\uE000\uE001", so that commonmark can parse +# the remaining text as pure markdown, with no escaping. See +# src/tokenizeMd.cpp for the grammar. `restore_tokens()` substitutes the +# original constructs back while the parsed markdown is translated to Rd; +# the `mode` argument selects how a token is rendered in the Rd context +# it lands in: +# +# * `text`: regular Rd text. Verbatim tags come back as live Rd; the +# escaped bracket escapes `\\[` and `\\]` drop one backslash (matching +# what the markdown escape `\[` does to a bare bracket); everything +# else comes back as typed. +# * `verb`: inside `\verb{}`, `\code{}` or `\preformatted{}`. Everything +# renders literally, so token text is Rd-escaped -- except verbatim Rd +# tags, which are inserted as typed: the Rd parser keeps unknown macros +# in verbatim contexts as literal text, and this is how the escaping +# code always behaved. +# * `raw`: unprocessed output, e.g. the body of a generated `\Sexpr{}`. + +verbatim_rd_tags <- c( + "acronym", + "code", + "command", + "CRANpkg", + "deqn", + "doi", + "dontrun", + "dontshow", + "donttest", + "email", + "env", + "eqn", + "figure", + "file", + "if", + "ifelse", + "kbd", + "link", + "linkS4class", + "method", + "mjeqn", + "mjdeqn", + "mjseqn", + "mjsdeqn", + "mjteqn", + "mjtdeqn", + "newcommand", + "option", + "out", + "packageAuthor", + "packageDescription", + "packageDESCRIPTION", + "packageIndices", + "packageMaintainer", + "packageTitle", + "pkg", + "PR", + "preformatted", + "renewcommand", + "S3method", + "S4method", + "samp", + "special", + "testonly", + "url", + "var", + "verb" +) + +md_tokenize <- function(text, tag = NULL) { + out <- tokenizeMd(text, verbatim_rd_tags) + if (out$stripped > 0) { + cli::cli_warn( + "Removed {out$stripped} private-use unicode character{?s} (U+E000/U+E001), which roxygen2 uses internally." + ) + } + for (name in unique(out$incomplete)) { + warn_roxy_tag( + tag, + c( + "markdown translation failed", + x = paste0("\\", name, " has an unterminated argument"), + i = "Rd tag arguments must have balanced braces, and an unescaped % comments out the rest of the line: write \\% for a literal %" + ) + ) + } + out +} + +restore_tokens <- function(x, state, mode = c("text", "verb", "raw")) { + if (length(state$tokens) == 0 || !grepl("\uE000", x, fixed = TRUE)) { + return(x) + } + mode <- match.arg(mode) + + m <- gregexpr("\uE000[0-9]+\uE001", x, perl = TRUE) + regmatches(x, m) <- lapply(regmatches(x, m), function(placeholder) { + idx <- as.integer(gsub("[\uE000\uE001]", "", placeholder)) + src <- state$tokens[idx] + type <- state$types[idx] + + switch( + mode, + raw = src, + text = ifelse(src %in% c("\\\\[", "\\\\]"), substring(src, 2), src), + verb = ifelse(type == "verbatim", src, escape_rd_verb(src)) + ) + }) + x +} + +escape_rd_verb <- function(x) { + x <- gsub("\\", "\\\\", x, fixed = TRUE) + x <- gsub("%", "\\%", x, fixed = TRUE) + x <- gsub("{", "\\{", x, fixed = TRUE) + x <- gsub("}", "\\}", x, fixed = TRUE) + x +} diff --git a/R/markdown.R b/R/markdown.R index 239823f1..16767442 100644 --- a/R/markdown.R +++ b/R/markdown.R @@ -6,9 +6,9 @@ markdown <- function(text, tag = NULL, sections = FALSE) { text } ) - escaped_text <- escape_rd_for_md(expanded_text) + tokens <- md_tokenize(expanded_text, tag) tryCatch( - markdown_pass2(escaped_text, tag = tag, sections = sections), + markdown_pass2(tokens, tag = tag, sections = sections), error = function(e) { warn_roxy_tag(tag, "markdown failed to process", parent = e) text @@ -16,16 +16,29 @@ markdown <- function(text, tag = NULL, sections = FALSE) { ) } -markdown_pass2 <- function(text, tag = NULL, sections = FALSE) { - esc_text_linkrefs <- add_linkrefs_to_md(text) +markdown_pass2 <- function(tokens, tag = NULL, sections = FALSE) { + text_linkrefs <- add_linkrefs_to_md(tokens$text) - mdxml <- md_to_mdxml(esc_text_linkrefs) + mdxml <- md_to_mdxml(text_linkrefs) state <- new.env(parent = emptyenv()) state$tag <- tag state$has_sections <- sections + state$tokens <- tokens$tokens + state$types <- tokens$types rd <- mdxml_children_to_rd_top(mdxml, state) - map_chr(rd, unescape_rd_for_md, text) + # Restore the tokens that landed in regular text; verbatim and raw + # contexts have already restored theirs during the tree walk + rd[] <- map_chr(rd, restore_tokens, state = state, mode = "text") + if (!is.null(names(rd))) { + names(rd) <- map_chr( + names(rd), + restore_tokens, + state = state, + mode = "text" + ) + } + rd } md_to_mdxml <- function(x, ...) { @@ -82,7 +95,7 @@ mdxml_node_to_rd <- function(xml, state) { paragraph = paste0("\n\n", mdxml_children_to_rd(xml, state)), text = if (is_true(state$in_link_code)) { - escape_verb(xml_text(xml)) + restore_tokens(escape_verb(xml_text(xml)), state, "verb") } else { escape_comment(xml_text(xml)) }, @@ -138,21 +151,27 @@ mdxml_break <- function(state) { if (isTRUE(state$inlink)) " " else "\n" } -mdxml_code <- function(xml, tag) { +mdxml_code <- function(xml, state) { code <- xml_text(xml) + # Decide what the code is based on its original source + raw_code <- restore_tokens(code, state, "raw") - if (grepl("^Rd ", code)) { + if (grepl("^Rd ", raw_code)) { paste0( "\\Sexpr[stage=render,results=rd]{", - substr(code, 4, nchar(code)), + substr(raw_code, 4, nchar(raw_code)), "}" ) - } else if (can_parse(code) || code %in% special) { + } else if (can_parse(raw_code) || raw_code %in% special) { # See escaping details at # https://cran.rstudio.com/doc/manuals/r-devel/R-exts.html#Insertions - paste0("\\code{", gsub("%", "\\\\%", code), "}") + paste0( + "\\code{", + restore_tokens(gsub("%", "\\\\%", code), state, "verb"), + "}" + ) } else { - paste0("\\verb{", escape_verb(code), "}") + paste0("\\verb{", restore_tokens(escape_verb(code), state, "verb"), "}") } } @@ -207,7 +226,7 @@ mdxml_code_block <- function(xml, state) { if (!is.na(info)) paste0(" ", info), "\">}}", "\\preformatted{", - escape_verb(xml_text(xml)), + restore_tokens(escape_verb(xml_text(xml)), state, "verb"), "}", "\\if{html}{\\out{}}" ) @@ -224,7 +243,8 @@ can_parse <- function(x) { } escape_verb <- function(x) { - # Don't need to escape \\ because that's already handled in double_escape_md() + # No need to escape backslashes: they are all tokenized, and + # restore_tokens() escapes them as it puts them back x <- gsub("%", "\\%", x, fixed = TRUE) x <- gsub("{", "\\{", x, fixed = TRUE) x <- gsub("}", "\\}", x, fixed = TRUE) diff --git a/man/double_escape_md.Rd b/man/double_escape_md.Rd deleted file mode 100644 index 22627fba..00000000 --- a/man/double_escape_md.Rd +++ /dev/null @@ -1,33 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/markdown-escaping.R -\name{double_escape_md} -\alias{double_escape_md} -\title{Check markdown escaping} -\usage{ -double_escape_md(text) -} -\arguments{ -\item{text}{Input text.} -} -\value{ -Double-escaped text. -} -\description{ -This is a regression test for Markdown escaping. -} -\details{ -Each of the following bullets should look the same when rendered: -\itemize{ -\item Backticks: \verb{\\}, \verb{\\\%}, \verb{\\$}, \verb{\\_} -\item \verb{\verb{}}: \verb{\\}, \verb{\\\%}, \verb{\$}, \verb{\_} -} - -[ this isn't a link ] -\[ neither is this \] -} -\examples{ -"\%" # percent -"\"" # double quote -'\'' # single quote -} -\keyword{internal} diff --git a/man/markdown-internals.Rd b/man/markdown-internals.Rd deleted file mode 100644 index 8c6431eb..00000000 --- a/man/markdown-internals.Rd +++ /dev/null @@ -1,37 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/markdown-escaping.R -\name{escape_rd_for_md} -\alias{escape_rd_for_md} -\alias{unescape_rd_for_md} -\title{Escape fragile Rd tags} -\usage{ -escape_rd_for_md(text) - -unescape_rd_for_md(rd_text, esc_text) -} -\arguments{ -\item{text}{Input text. Potentially contains Rd and/or -markdown markup.} - -\item{rd_text}{The markdown parsed and interpreted text.} - -\item{esc_text}{The original escaped text from -\code{escape_rd_for_md()}.} -} -\value{ -\itemize{ -\item \code{escape_rd_for_md}: a "safe" version of the input text, where -each fragile Rd tag is replaced by a placeholder. The -original text is added as an attribute for each placeholder. -\item \code{unescape_rd_for_md}: the original Rd text. -} -} -\description{ -\code{escape_rd_for_md()} replaces fragile Rd tags with placeholders, to avoid -interpreting them as markdown. \code{unescape_rd_for_md()} puts the original -text back in place of the placeholders after the markdown parsing is done. -The fragile tags are listed in \code{escaped_for_md}. A fragile tag is -protected together with all of its brace arguments, so markdown is never -interpreted inside them. -} -\keyword{internal} diff --git a/src/cpp11.cpp b/src/cpp11.cpp index e44bc0f7..17701453 100644 --- a/src/cpp11.cpp +++ b/src/cpp11.cpp @@ -47,6 +47,13 @@ extern "C" SEXP _roxygen2_find_includes(SEXP path) { return cpp11::as_sexp(find_includes(cpp11::as_cpp>(path))); END_CPP11 } +// tokenizeMd.cpp +cpp11::writable::list tokenizeMd(std::string text, cpp11::strings verbatim); +extern "C" SEXP _roxygen2_tokenizeMd(SEXP text, SEXP verbatim) { + BEGIN_CPP11 + return cpp11::as_sexp(tokenizeMd(cpp11::as_cpp>(text), cpp11::as_cpp>(verbatim))); + END_CPP11 +} // wrapUsage.cpp std::string wrapUsage(std::string string, int width, int indent); extern "C" SEXP _roxygen2_wrapUsage(SEXP string, SEXP width, SEXP indent) { @@ -63,6 +70,7 @@ static const R_CallMethodDef CallEntries[] = { {"_roxygen2_leadingSpaces", (DL_FUNC) &_roxygen2_leadingSpaces, 1}, {"_roxygen2_rdComplete", (DL_FUNC) &_roxygen2_rdComplete, 2}, {"_roxygen2_tokenise_block", (DL_FUNC) &_roxygen2_tokenise_block, 3}, + {"_roxygen2_tokenizeMd", (DL_FUNC) &_roxygen2_tokenizeMd, 2}, {"_roxygen2_wrapUsage", (DL_FUNC) &_roxygen2_wrapUsage, 3}, {NULL, NULL, 0} }; diff --git a/src/tokenizeMd.cpp b/src/tokenizeMd.cpp new file mode 100644 index 00000000..9d6aa4dd --- /dev/null +++ b/src/tokenizeMd.cpp @@ -0,0 +1,196 @@ +#include +#include +#include +#include +#include +#include + +using namespace cpp11::literals; + +// Tokenizer for the combined markdown/Rd grammar used in roxygen comments. +// +// The only overlap between the two languages is the backslash: everything +// backslash-initiated belongs to Rd, everything else belongs to markdown. +// So we scan once, left to right, and replace every backslash-initiated +// construct with an inert placeholder "" (private-use-area +// sentinels around a 1-based index into the returned token vector). +// Placeholders are plain text to commonmark in every context (text, code +// spans, fenced blocks, link labels, tables), so the sanitized text can be +// parsed as markdown without any escaping. The grammar: +// +// bs_token := "\" NAME braces* NAME in `verbatim` -> tag + all brace args +// | "\" NAME other NAME (args remain markdown) +// | "\" PUNCT two-character escape, e.g. \% \\ \{ +// | "\" lone backslash (also before a backtick, +// so code-span delimiters are never eaten) +// +// Two exceptions, because \[ and \] are markdown bracket escapes: "\[" and +// "\]" are passed through for commonmark to handle, and "\\[" / "\\]" are +// single three-character tokens (the trailing bracket must not become an +// active markdown bracket). +// +// All scanning is over UTF-8 bytes; multibyte characters can never match +// ASCII specials, so they pass through untouched. + +// Scan a brace group starting at s[start] == '{'. Returns true and sets +// *end to the byte index of the matching '}', or returns false if the +// group is incomplete. Follows Rd rules: \ escapes the next character, +// % starts a comment that runs to the end of the line. +static bool scan_brace_group(const std::string& s, int start, int* end) { + int n = s.size(); + int braces = 0; + bool escape = false; + bool comment = false; + + for (int i = start; i < n; i++) { + char c = s[i]; + if (escape) { + escape = false; + } else if (comment) { + if (c == '\n') + comment = false; + } else { + switch (c) { + case '\\': + escape = true; + break; + case '%': + comment = true; + break; + case '{': + braces++; + break; + case '}': + braces--; + if (braces == 0) { + *end = i; + return true; + } + break; + } + } + } + + return false; +} + +static bool is_ascii_alpha(char c) { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); +} + +static bool is_ascii_alnum(char c) { + return is_ascii_alpha(c) || (c >= '0' && c <= '9'); +} + +static bool is_ascii_punct(char c) { + return (c >= '!' && c <= '/') || (c >= ':' && c <= '@') || + (c >= '[' && c <= '`') || (c >= '{' && c <= '~'); +} + +[[cpp11::register]] +cpp11::writable::list tokenizeMd(std::string text, + cpp11::strings verbatim) { + std::set vtags; + for (R_xlen_t v = 0; v < verbatim.size(); v++) { + vtags.insert(std::string(verbatim[v])); + } + + static const std::string OPEN = "\xEE\x80\x80"; // U+E000 + static const std::string CLOSE = "\xEE\x80\x81"; // U+E001 + + int n = text.size(); + std::string out; + out.reserve(n); + std::vector tokens; + std::vector types; + std::vector incomplete; + int n_stripped = 0; + + for (int i = 0; i < n; i++) { + char c = text[i]; + + // Strip pre-existing sentinel characters so placeholders can't be forged + if ((unsigned char)c == 0xEE && i + 2 < n && + (unsigned char)text[i + 1] == 0x80 && + ((unsigned char)text[i + 2] == 0x80 || + (unsigned char)text[i + 2] == 0x81)) { + n_stripped++; + i += 2; + continue; + } + + if (c != '\\') { + out += c; + continue; + } + + int end = i; // inclusive last byte of this token + std::string type = "backslash"; + if (i + 1 < n && is_ascii_alpha(text[i + 1])) { + int j = i + 2; + while (j < n && is_ascii_alnum(text[j])) + j++; + end = j - 1; + + std::string name = text.substr(i + 1, j - i - 1); + if (vtags.count(name)) { + type = "verbatim"; + // Consume all complete brace groups; an incomplete group is left + // to markdown, like the brace groups of non-verbatim tags, and + // reported so that the caller can warn + while (end + 1 < n && text[end + 1] == '{') { + int gend; + if (!scan_brace_group(text, end + 1, &gend)) { + incomplete.push_back(name); + break; + } + end = gend; + } + } else { + type = "tag"; + } + } else if (i + 1 < n && (text[i + 1] == '[' || text[i + 1] == ']')) { + // Markdown bracket escape: the only backslash construct that + // commonmark must see, so that \[ suppresses link parsing + out += c; + out += text[i + 1]; + i++; + continue; + } else if (i + 1 < n && is_ascii_punct(text[i + 1]) && + text[i + 1] != '`') { + type = "escape"; + end = i + 1; + // \\[ renders as a backslash + literal bracket, so the bracket + // must be hidden from the markdown parser along with the backslashes + if (text[i + 1] == '\\' && i + 2 < n && + (text[i + 2] == '[' || text[i + 2] == ']')) { + end = i + 2; + } + } + + tokens.push_back(text.substr(i, end - i + 1)); + types.push_back(type); + out += OPEN; + out += std::to_string(tokens.size()); + out += CLOSE; + i = end; + } + + cpp11::writable::strings rtokens(tokens.size()); + cpp11::writable::strings rtypes(types.size()); + for (size_t t = 0; t < tokens.size(); t++) { + rtokens[t] = tokens[t]; + rtypes[t] = types[t]; + } + cpp11::writable::strings rincomplete(incomplete.size()); + for (size_t t = 0; t < incomplete.size(); t++) { + rincomplete[t] = incomplete[t]; + } + + return cpp11::writable::list( + {"text"_nm = cpp11::writable::strings({out}), + "tokens"_nm = rtokens, + "types"_nm = rtypes, + "incomplete"_nm = rincomplete, + "stripped"_nm = cpp11::writable::integers({n_stripped})}); +} diff --git a/tests/testthat/_snaps/markdown.md b/tests/testthat/_snaps/markdown.md index f69ee1be..4a5d11d5 100644 --- a/tests/testthat/_snaps/markdown.md +++ b/tests/testthat/_snaps/markdown.md @@ -99,6 +99,15 @@ x Level 1 headings are not supported in @seealso i Do you want to put the heading in @description or @details? +# % inside a fragile tag argument breaks protection, with a warning + + Code + out <- markdown("\\code{a % b} *x*") + Message + x markdown translation failed + x \code has an unterminated argument + i Rd tag arguments must have balanced braces, and an unescaped % comments out the rest of the line: write \% for a literal % + # markdown() warnings work without a tag Code diff --git a/tests/testthat/test-markdown-link.R b/tests/testthat/test-markdown-link.R index 9a317eb1..a5d1f500 100644 --- a/tests/testthat/test-markdown-link.R +++ b/tests/testthat/test-markdown-link.R @@ -302,7 +302,7 @@ test_that("another markdown link bug is fixed", { " #' Title #' - #' Description, see [escape_rd_for_md()]. + #' Description, see [md_tokenize()]. #' #' And also [object]. #' @md @@ -313,7 +313,7 @@ test_that("another markdown link bug is fixed", { " #' Title #' - #' Description, see \\code{\\link[=escape_rd_for_md]{escape_rd_for_md()}}. + #' Description, see \\code{\\link[=md_tokenize]{md_tokenize()}}. #' #' And also \\link{object}. foo <- function() {}" diff --git a/tests/testthat/test-markdown-tokenize.R b/tests/testthat/test-markdown-tokenize.R new file mode 100644 index 00000000..fd5369f6 --- /dev/null +++ b/tests/testthat/test-markdown-tokenize.R @@ -0,0 +1,85 @@ +ph <- function(i) paste0("\uE000", i, "\uE001") + +test_that("md_tokenize splits text into markdown and Rd tokens", { + tk <- md_tokenize("a \\code{x *y*} *b* \\emph{*c*}") + expect_equal(tk$text, paste0("a ", ph(1), " *b* ", ph(2), "{*c*}")) + expect_equal(tk$tokens, c("\\code{x *y*}", "\\emph")) + expect_equal(tk$types, c("verbatim", "tag")) +}) + +test_that("verbatim tags consume all brace groups", { + expect_equal(md_tokenize("\\ifelse{a}{b}{c} x")$tokens, "\\ifelse{a}{b}{c}") +}) + +test_that("brace matching follows Rd rules", { + # escaped braces don't count + expect_equal(md_tokenize("\\code{a \\} b}")$tokens, "\\code{a \\} b}") + # braces nest + expect_equal(md_tokenize("\\code{a {b} c}")$tokens, "\\code{a {b} c}") + # % comments out the rest of the line, so this group never completes and + # the tag falls back to a bare name, with the braces left to markdown + expect_message( + tk <- md_tokenize("\\code{a % b}"), + "unterminated argument" + ) + expect_equal(tk$tokens, "\\code") + expect_equal(tk$incomplete, "code") +}) + +test_that("escapes and lone backslashes are tokenized", { + tk <- md_tokenize("\\% \\\\ \\") + expect_equal(tk$tokens, c("\\%", "\\\\", "\\")) + expect_equal(tk$types, c("escape", "escape", "backslash")) +}) + +test_that("bracket escapes are left for the markdown parser", { + tk <- md_tokenize("\\[x\\]") + expect_equal(tk$text, "\\[x\\]") + expect_equal(tk$tokens, character()) + + # but \\[ hides its bracket, because it renders as \[ + tk <- md_tokenize("\\\\[x\\\\]") + expect_equal(tk$text, paste0(ph(1), "x", ph(2))) + expect_equal(tk$tokens, c("\\\\[", "\\\\]")) +}) + +test_that("a backslash never consumes a backtick", { + tk <- md_tokenize("`\\`") + expect_equal(tk$text, paste0("`", ph(1), "`")) + expect_equal(tk$types, "backslash") +}) + +test_that("multibyte characters pass through", { + tk <- md_tokenize("é \\code{café} ü") + expect_equal(tk$text, paste0("é ", ph(1), " ü")) + expect_equal(tk$tokens, "\\code{café}") +}) + +test_that("pre-existing sentinel characters are stripped with a warning", { + expect_warning(tk <- md_tokenize("a \uE000 b \uE001 c"), "private-use") + expect_equal(tk$text, "a b c") + expect_equal(tk$tokens, character()) +}) + +test_that("restore_tokens restores according to context", { + tk <- md_tokenize("\\code{x} \\emph \\% \\\\[ \\") + state <- as.environment(tk) + + expect_equal( + restore_tokens(tk$text, state, "text"), + "\\code{x} \\emph \\% \\[ \\" + ) + expect_equal( + restore_tokens(tk$text, state, "verb"), + "\\code{x} \\\\emph \\\\\\% \\\\\\\\[ \\\\" + ) + expect_equal( + restore_tokens(tk$text, state, "raw"), + "\\code{x} \\emph \\% \\\\[ \\" + ) +}) + +test_that("restore_tokens leaves token-free text alone", { + state <- as.environment(list(tokens = character(), types = character())) + expect_equal(restore_tokens("plain *text*", state, "text"), "plain *text*") +}) diff --git a/tests/testthat/test-markdown.R b/tests/testthat/test-markdown.R index a4c15913..462d85c3 100644 --- a/tests/testthat/test-markdown.R +++ b/tests/testthat/test-markdown.R @@ -735,6 +735,84 @@ test_that("headings and empty sections", { expect_false("details" %in% names(out1$fields)) }) +# Characterization tests: pin down the interaction between Rd markup and +# markdown before replacing the escaping machinery --------------------------- + +test_that("backslash escapes in text", { + expect_equal(markdown("\\[ not a link \\]"), "[ not a link ]") + expect_equal(markdown("\\\\[ nor this \\\\]"), "\\[ nor this \\]") + # \% used to become \\%, which truncated the rendered line at the % + expect_equal(markdown("50\\% \\{x\\} a\\_b"), "50\\% \\{x\\} a\\_b") + expect_equal(markdown("A \\\\ B"), "A \\\\ B") + expect_equal(markdown("a \\` b"), "a \\` b") + # an escaped closing bracket used to leak a link reference definition + expect_equal(markdown("\\[foo] and [bar\\]"), "[foo] and [bar]") +}) + +test_that("Rd tags in code spans and code blocks are inserted as typed", { + expect_equal(markdown("`\\code{x}`"), "\\verb{\\code{x}}") + expect_equal( + markdown("```\n\\code{x} \\%\n```"), + paste0( + "\\if{html}{\\out{
}}", + "\\preformatted{\\code{x} \\\\\\%\n}", + "\\if{html}{\\out{
}}" + ) + ) + expect_equal( + markdown("text\n\n \\code{x} indented\n more\n\nafter"), + paste0( + "text\n\n", + "\\if{html}{\\out{
}}", + "\\preformatted{\\code{x} indented\nmore\n}", + "\\if{html}{\\out{
}}", + "\n\nafter" + ) + ) +}) + +test_that("Rd tags work inside links, tables, and HTML", { + expect_equal( + markdown("[\\code{x}] and [`y`][print]"), + "\\link{\\code{x}} and \\code{\\link[=print]{y}}" + ) + expect_equal( + markdown("a \\code{x} b"), + "a \\if{html}{\\out{}}\\code{x}\\if{html}{\\out{}} b" + ) + expect_equal( + markdown("
\n\\code{x} *y*\n
\n"), + "\\if{html}{\\out{\n
\n\\code{x} *y*\n
\n}}" + ) + expect_equal( + markdown("| \\code{x} | *y* |\n|---|---|\n| a | b |"), + "\\tabular{ll}{\n \\code{x} \\tab \\emph{y} \\cr\n a \\tab b \\cr\n}" + ) + expect_equal( + markdown("`Rd \\out{z}`"), + "\\Sexpr[stage=render,results=rd]{\\out{z}}" + ) +}) + +test_that("non-fragile tags allow markdown in their arguments", { + expect_equal( + markdown("\\emph{*x*} \\strong{`y`}"), + "\\emph{\\emph{x}} \\strong{\\code{y}}" + ) +}) + +test_that("multi-line fragile tags are protected across paragraphs", { + expect_equal( + markdown("\\preformatted{\n a *b*\n\n c\n} *after*"), + "\\preformatted{\n a *b*\n\n c\n} \\emph{after}" + ) +}) + +test_that("% inside a fragile tag argument breaks protection, with a warning", { + expect_snapshot(out <- markdown("\\code{a % b} *x*")) + expect_equal(out, "\\code{a \\% b} \\emph{x}") +}) + test_that("markdown() warnings work without a tag", { expect_snapshot(out <- markdown("# Heading")) }) diff --git a/tests/testthat/test-rd-markdown-escaping.R b/tests/testthat/test-rd-markdown-escaping.R deleted file mode 100644 index df813dec..00000000 --- a/tests/testthat/test-rd-markdown-escaping.R +++ /dev/null @@ -1,133 +0,0 @@ -tag_df <- function(tag, start, end, argend = NULL) { - df <- data.frame( - tag = tag, - start = start, - end = end - ) - if (!is.null(argend)) { - df$argend <- argend - } - df -} - -test_that("find_all_tag_names", { - text <- r"(blah blah \mytag blah blah)" - expect_equal( - find_all_tag_names(text), - tag_df(r"(\mytag)", 11, 16) - ) -}) - -test_that("find_all_rd_tags", { - cases <- list( - ## No tags - list("", character(), numeric(), numeric(), numeric()), - list("nothing to see here", character(), numeric(), numeric(), numeric()), - list("\nstill\nnothing\n", character(), numeric(), numeric(), numeric()), - - ## One tag - list(r"(blah blah \mytag blah blah)", r"(\mytag)", 11, 16, 16), - list(r"(blah blah \mytag{arg1} blah blah)", r"(\mytag)", 11, 16, 22), - list(r"(blah blah \mytag{arg1}{arg2} blah blah)", r"(\mytag)", 11, 16, 28), - list(r"(blah\mytag)", r"(\mytag)", 5, 10, 10), - list(r"(blah \mytag)", r"(\mytag)", 6, 11, 11), - list(r"(blah\mytag{arg})", r"(\mytag)", 5, 10, 15), - list(r"(\mytag hoohoo)", r"(\mytag)", 1, 6, 6), - list(r"(\mytag)", r"(\mytag)", 1, 6, 6), - list(r"(\mytag{arg})", r"(\mytag)", 1, 6, 11), - list("blah \\mytag\nblah blah", r"(\mytag)", 6, 11, 11), - - ## Multiple tags - list( - r"(blah \tag1 \tag2{arg} blah)", - c(r"(\tag1)", r"(\tag2)"), - c(6, 12), - c(10, 16), - c(10, 21) - ), - list( - r"(blah \tag1{ \tag2{arg} } blah)", - c(r"(\tag1)", r"(\tag2)"), - c(6, 13), - c(10, 17), - c(24, 22) - ), - list( - "blah \\tag1{\n\\tag2{arg}\n} blah", - c(r"(\tag1)", r"(\tag2)"), - c(6, 13), - c(10, 17), - c(24, 22) - ) - ) - - for (case in cases) { - expect_equal( - find_all_rd_tags(case[[1]]), - do.call(tag_df, case[-1]), - info = case[[1]] - ) - } -}) - -test_that("find_all_rd_tags uses character offsets for multibyte text", { - expect_equal( - find_all_rd_tags("x \\code{café} y"), - tag_df("\\code", 3, 7, 13) - ) - expect_equal( - find_all_rd_tags("é \\mytag{ééé}{ü} é"), - tag_df("\\mytag", 3, 8, 16) - ) -}) - -test_that("find_fragile_rd_tags", { - fragile <- c(r"(\frag)", r"(\frag1)", r"(\frag2)") - - cases <- list( - list(r"(This is \frag{here}, \this{arg} not)", r"(\frag)"), - list(r"(Embedded \frag{ into \frag1{arg} plus })", r"(\frag)"), - list( - r"(blah \cmd{ \frag{arg} \frag{arg} } \frag2 blah)", - c(r"(\frag)", r"(\frag)", r"(\frag2)") - ) - ) - - for (case in cases) { - expect_equal( - find_fragile_rd_tags(case[[1]], fragile)$tag, - case[[2]], - info = case[[1]] - ) - } -}) - - -test_that("re_sub_same", { - expect_equal( - re_sub_same( - "123456789ab", - data.frame(start = c(1, 6), end = c(2, 10), argend = c(2, 10)), - "xxx" - ), - "xxx-1-345xxx-2-b" - ) - - expect_equal( - re_sub_same( - "123456789ab", - data.frame(start = c(1, 8), end = c(7, 10), argend = c(7, 10)), - "xxx" - ), - "xxx-1-xxx-2-b" - ) - - expect_equal( - re_sub_same( - "123456789ab", - data.frame(start = numeric(), end = numeric(), argend = numeric()), - "xxx" - ), - "123456789ab" - ) -}) diff --git a/vignettes/rd-formatting.Rmd b/vignettes/rd-formatting.Rmd index 60b533e1..61ba3f04 100644 --- a/vignettes/rd-formatting.Rmd +++ b/vignettes/rd-formatting.Rmd @@ -318,7 +318,7 @@ If you want to avoid this restriction, set the `restrict_image_formats` roxygen2 ### Some Rd tags can't contain markdown -When mixing `Rd` and Markdown notation, most `Rd` tags may contain Markdown markup, the ones that can *not* are: `r paste0("\x60", roxygen2:::escaped_for_md, "\x60", collapse = ", ")`. +When mixing `Rd` and Markdown notation, most `Rd` tags may contain Markdown markup, the ones that can *not* are: `r paste0("\x60\\", roxygen2:::verbatim_rd_tags, "\x60", collapse = ", ")`. ### Mixing Markdown and `Rd` markup