Skip to content

Replace markdown escaping with a combined md/Rd tokenizer#1901

Open
hadley wants to merge 9 commits into
mainfrom
md-grammar
Open

Replace markdown escaping with a combined md/Rd tokenizer#1901
hadley wants to merge 9 commits into
mainfrom
md-grammar

Conversation

@hadley

@hadley hadley commented Jul 5, 2026

Copy link
Copy Markdown
Member

Stacked on #1900.

This replaces escape_rd_for_md()/unescape_rd_for_md() and the double-escaping pass with an explicit grammar for how Rd and markdown mix in roxygen comments. The two languages only overlap at the backslash, so a single byte-wise C++ scan (src/tokenizeMd.cpp) classifies every backslash-initiated construct:

bs_token := "\" NAME braces*   NAME in verbatim list -> tag + all brace args
          | "\" NAME           other tags: args remain markdown
          | "\" PUNCT          two-character escape, e.g. \% \\ \{
          | "\"                lone backslash (never consumes a backtick)

Each token becomes an inert private-use-area placeholder, so commonmark parses text containing no backslashes at all — no escaping, no un-escaping, no placeholder-collision handling. Tokens are restored during the XML→Rd walk with the escaping appropriate to where they landed: as-typed in regular text, Rd-escaped inside \verb{}/\code{}/\preformatted{}, raw inside generated \Sexpr{} bodies. \[ and \] pass through untokenized, because they're commonmark bracket escapes — that single rule is what used to require the de-duplication special case in double_escape_md().

Highlights:

  • The first commit adds characterization tests pinning current behaviour for the tricky interactions (Rd tags inside code spans/blocks, links, tables, HTML, multi-line arguments, backslash escapes) before any machinery changed.
  • Rd-tags-inside-indented-code-blocks need no block-level parsing at all: the placeholder lands in a verbatim context and is restored as typed, matching the old behaviour by construction.
  • escape_verb() loses its hidden ordering dependency on double_escape_md(); all escaping is now local and explicit.
  • Re-documenting roxygen2 itself with the new pipeline leaves man/ byte-identical.
  • ~2.3x faster on tag-heavy documents (25KB, 400 Rd tags), ~1.3x on typical small texts; profiling shows remaining time is the commonmark parse and XML walk, i.e. the escaping overhead is gone entirely.
  • One deliberate behaviour change (NEWS): \% in markdown text now produces \% in the Rd file instead of \\%, which rendered as a backslash followed by a comment that truncated the line.

🤖 Generated with Claude Code

hadley added 7 commits July 5, 2026 15:12
* Make roxygen_parse_tag() UTF-8 aware, so findEndOfTag() accepts and
  returns character offsets. Previously its byte offsets were mixed with
  character offsets from gregexpr()/substr(), so a multibyte character
  inside a fragile Rd tag (e.g. \code{café}) corrupted markdown
  processing of the following text, and strip_rd_tag() mis-sliced
  examples containing multibyte characters.

* Fix warning paths for tag-less markdown(): the fake default tag
  crashed in basename(), and rd-family.R passed a bare string that
  crashed on tag$tag. Both now pass NULL, which warn_roxy_tag()
  handles.

* Delete dead markdown_pass1() and seven helpers duplicated between
  markdown.R and markdown-code.R (the markdown.R copies silently shadow
  the markdown-code.R ones due to collation order).

* Remove the stale claim that markdown is interpreted inside \if/
  \ifelse arguments; fragile tags are protected with all their
  arguments.

* Skip the commonmark parse in markdown_evaluate() when the text
  contains no backtick or tilde, so tags without code don't pay for a
  full parse.
These pin down exactly how Rd markup and markdown currently interact
(backslash escapes, Rd tags inside code spans/blocks, links, tables,
HTML, multi-line arguments) before the escaping machinery is replaced
with a tokenizer.
tokenizeMd() implements the combined markdown/Rd grammar in a single
byte-wise scan: every backslash-initiated construct becomes an inert
private-use-area placeholder, so the remaining text can be parsed as
pure markdown with no escaping. restore_tokens() substitutes the
original constructs back according to the Rd context they land in
(text, verb, or raw).
markdown() now tokenizes Rd constructs instead of escaping them:
commonmark parses text containing no backslashes at all, and tokens are
restored during the XML-to-Rd walk with context-appropriate escaping
(raw for \Sexpr bodies, Rd-escaped in verbatim contexts, as-typed in
regular text). This removes the double-escaping pass and the hidden
coupling between double_escape_md() and escape_verb().

One intentional behaviour change: \% in text now stays \%, instead of
becoming \\%, which truncated the rendered line at the %.
escape_rd_for_md(), unescape_rd_for_md(), double_escape_md() and their
helpers are fully replaced by the tokenizer.
Two follow-ups the explicit grammar makes easy:

* A markdown link with an escaped closing bracket ([bar\]) no longer
  leaks a link reference definition into the generated Rd: reference
  content must not end with a backslash.

* A verbatim Rd tag whose brace argument never closes (e.g.
  \code{a % b}, where the unescaped % comments out the closing brace)
  now warns instead of being silently reinterpreted as markdown. The
  output is unchanged.
@hadley

hadley commented Jul 5, 2026

Copy link
Copy Markdown
Member Author

Follow-up commit fixes the two pre-existing warts the characterization tests documented:

  • [bar\] (escaped closing bracket) no longer leaks a link reference definition into the Rd output — reference content must not end with a backslash.
  • An Rd tag whose brace argument never closes (e.g. \code{a % b}, where the unescaped % comments out the closing brace) now warns with a hint to write \%, instead of being silently reinterpreted as markdown. Output is unchanged.

Both have NEWS bullets; man/ remains byte-identical when self-documenting.

Base automatically changed from markdown-fixes to main July 5, 2026 23:49
#Conflicts:
#	R/markdown-escaping.R
#	R/markdown.R
#	man/markdown-internals.Rd
#	tests/testthat/_snaps/markdown.md
#	tests/testthat/test-markdown.R
#	tests/testthat/test-rd-markdown-escaping.R
@hadley hadley requested a review from gaborcsardi July 5, 2026 23:56
@hadley

hadley commented Jul 5, 2026

Copy link
Copy Markdown
Member Author

@gaborcsardi very low priority but this is take two at re-doing the escaping/unescaping. I think it's a lot cleaner now.
It doesn't make much difference performance wise, except if you have large sections with tons of Rd tags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant