From f7879374f33d06559e7df5f9505c0d28eacbbaa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Sun, 13 Sep 2026 18:45:02 +0000 Subject: [PATCH 1/2] ci: Document with a patched roxygen2 that keeps the sentence gap commonmark discards the whitespace a line break stands for, so roxygen prose written one sentence per line loses the gap between sentences in the rendered help. Only the text renderer is affected, which is what `?topic` shows. A new composite action installs roxygen2 from upstream with the R/ part of krlmlr/roxygen2@f-sentence-spacing applied on top, and runs just before the Roxygenize step. It shallow-clones upstream, fetches the branch, applies the diff restricted to R/ so conflicts in the test files cannot fail it, and aborts rather than silently installing an unpatched build. It then asserts that what it installed really carries the patch. Config/roxygen2/version becomes 8.1.0.9100. The .9100 suffix distinguishes a patched build from upstream's own .9000 development builds; if upstream moves, the x.y.z part follows it and the suffix stays. DESCRIPTION is DCF and cannot carry a comment, so the explanation lives in a Config/cynkra/roxygen2 field. This is a separate decision from the line-break reformatting, and is kept in its own pull request so it can be taken or left on its own. Without it, the reformatting simply renders as it does today, with one space between sentences. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WWhverMTZZKgEpUuTK117m --- .github/workflows/R-CMD-check.yaml | 3 + .github/workflows/roxygen2-fork/action.yml | 105 +++++++++++++++++++++ DESCRIPTION | 9 +- 3 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/roxygen2-fork/action.yml diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index b45308a8b..d1b889de7 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -173,6 +173,9 @@ jobs: with: base: ${{ inputs.ref || github.head_ref }} + - name: Install roxygen2 from the fork branch + uses: ./.github/workflows/roxygen2-fork + - name: Roxygenize the documentation id: roxygenize continue-on-error: true diff --git a/.github/workflows/roxygen2-fork/action.yml b/.github/workflows/roxygen2-fork/action.yml new file mode 100644 index 000000000..e020c6e76 --- /dev/null +++ b/.github/workflows/roxygen2-fork/action.yml @@ -0,0 +1,105 @@ +name: "Action to install roxygen2 from a fork branch" +description: > + This action installs roxygen2 with only the `R/` part of a fork branch applied + on top of upstream, and stamps the result as a `.9100` build so that a + package's `Config/roxygen2/version` says which roxygen2 documented it. + +inputs: + upstream: + description: "Repository to install, in owner/repo form" + required: false + default: "r-lib/roxygen2" + fork: + description: "Repository holding the branch to apply, in owner/repo form" + required: false + default: "krlmlr/roxygen2" + branch: + description: "Branch whose `R/` changes are applied on top of upstream" + required: false + default: "f-sentence-spacing" + +runs: + using: "composite" + steps: + - name: Install roxygen2 with the fork's R changes + run: | + ## -- Install roxygen2 from a fork branch -- + set -euo pipefail + + upstream="${{ inputs.upstream }}" + fork="${{ inputs.fork }}" + branch="${{ inputs.branch }}" + + workdir="$(mktemp -d)" + trap 'rm -rf "$workdir"' EXIT + + # Upstream at its tip: this is the code that gets installed, so the + # build tracks upstream rather than a fork that may be stale. Depth 50 + # is enough to reach the branch point without fetching years of history. + git clone --depth 50 "https://github.com/${upstream}.git" "$workdir/pkg" + cd "$workdir/pkg" + echo "upstream ${upstream}@$(git rev-parse --short HEAD)" + + git fetch --depth 50 "https://github.com/${fork}.git" "$branch" + echo "fork ${fork}@${branch} $(git rev-parse --short FETCH_HEAD)" + + # Diff from the branch point, not from the tip's parent: the branch + # carries several commits and a tip-only diff would apply just the last. + if ! base="$(git merge-base HEAD FETCH_HEAD)"; then + echo "::error title=roxygen2 fork::No common ancestor within 50 commits of ${upstream} and ${fork}@${branch}." + echo "Rebase the branch on upstream, or raise the fetch depth here." + exit 1 + fi + + # Only `R/`. The branch also carries tests, NEWS and a regenerated + # `man/`, none of which this build runs, and all of which are far more + # likely to conflict as upstream moves. The R change is deliberately + # shaped to keep this patch small: one line in `R/markdown.R`, and + # everything else in a file of its own that upstream will never create, + # because a patch that adds a whole file cannot conflict. + git diff "$base" FETCH_HEAD -- R/ > "$workdir/R.patch" + + if [ ! -s "$workdir/R.patch" ]; then + echo "::error title=roxygen2 fork::${fork}@${branch} changes nothing under R/." + echo "Either the branch has landed upstream and this action should be removed," + echo "or the branch name is wrong." + exit 1 + fi + + # --3way so the patch still applies when upstream has moved around it. + # A conflict is a hard stop: installing an unpatched roxygen2 would + # regenerate every man/ file without the change, and the diff would + # look like unrelated documentation churn rather than a failed install. + if ! git apply --3way --verbose "$workdir/R.patch"; then + echo "::error title=roxygen2 fork::Could not apply ${fork}@${branch} onto ${upstream}." + echo "This is usually an ordinary merge conflict: upstream has changed the same lines." + echo "Rebase the branch on upstream and push it again." + exit 1 + fi + + # Stamp the build. roxygen2 writes its own version into a package's + # Config/roxygen2/version, so this is what makes it visible that the + # documentation was generated with the patch: upstream numbers its + # development builds x.y.z.9000, and this takes the same x.y.z with + # .9100. Derived from upstream's own version so it follows automatically + # when upstream moves. + Rscript -e ' + d <- read.dcf("DESCRIPTION") + v <- d[1, "Version"] + d[1, "Version"] <- sub("^([0-9]+[.][0-9]+[.][0-9]+).*$", "\1.9100", v) + write.dcf(d, "DESCRIPTION", keep.white = colnames(d)) + cat("stamped", v, "->", read.dcf("DESCRIPTION")[1, "Version"], "\n") + ' + + R CMD INSTALL --no-docs . + + # Fail here rather than three steps later with a puzzling man/ diff. + Rscript -e ' + v <- as.character(packageVersion("roxygen2")) + patched <- exists("mdxml_keep_sentence_spacing", envir = asNamespace("roxygen2")) + cat("roxygen2", v, "patched:", patched, "\n") + if (!grepl("[.]9100$", v) || !patched) { + stop("roxygen2 was not installed from the fork branch.", call. = FALSE) + } + ' + shell: bash diff --git a/DESCRIPTION b/DESCRIPTION index 4c798c6f8..407ec9f0e 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -63,4 +63,11 @@ Config/testthat/start-first: vignette-formats, as_tibble, add, invariants Config/usethis/last-upkeep: 2025-06-07 Encoding: UTF-8 Roxygen: list(markdown = TRUE) -Config/roxygen2/version: 8.1.0.9000 +Config/roxygen2/version: 8.1.0.9100 +Config/cynkra/roxygen2: The .9100 suffix on Config/roxygen2/version marks a patched + roxygen2, not an upstream development build. Upstream numbers its own + development builds x.y.z.9000; the build that documents this package takes the + same x.y.z and uses .9100. It is upstream plus the sentence-spacing fix from + krlmlr/roxygen2@f-sentence-spacing, installed by + .github/workflows/roxygen2-fork. Regenerating man/ with a stock roxygen2 drops + the gap after every sentence that ends a line. From 063ff8e2167751caeaa87ec0094143baaa761c0f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 22:08:19 +0000 Subject: [PATCH 2/2] fix(ci): Stamp the roxygen2 version with a backreference, not an octal escape The version stamp wrote the replacement as "\1". R parses that as the octal escape for \001, not as a regex backreference, so DESCRIPTION ended up with a malformed version and R CMD INSTALL aborted with "Malformed package version". Every job that installs roxygen2 through this action failed there. The replacement is now "\\1", verified to stamp 8.1.0.9000 to 8.1.0.9100. The post-install guard asserted only the .9100 suffix, and the corrupt "\001.9100" satisfies that too, which is why the bug survived the check meant to catch it. The guard now asserts the whole x.y.z.9100 shape, and passes inherits = FALSE to exists(). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WWhverMTZZKgEpUuTK117m --- .github/workflows/roxygen2-fork/action.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/roxygen2-fork/action.yml b/.github/workflows/roxygen2-fork/action.yml index e020c6e76..eb5ea1ff0 100644 --- a/.github/workflows/roxygen2-fork/action.yml +++ b/.github/workflows/roxygen2-fork/action.yml @@ -86,7 +86,7 @@ runs: Rscript -e ' d <- read.dcf("DESCRIPTION") v <- d[1, "Version"] - d[1, "Version"] <- sub("^([0-9]+[.][0-9]+[.][0-9]+).*$", "\1.9100", v) + d[1, "Version"] <- sub("^([0-9]+[.][0-9]+[.][0-9]+).*$", "\\1.9100", v) write.dcf(d, "DESCRIPTION", keep.white = colnames(d)) cat("stamped", v, "->", read.dcf("DESCRIPTION")[1, "Version"], "\n") ' @@ -96,9 +96,12 @@ runs: # Fail here rather than three steps later with a puzzling man/ diff. Rscript -e ' v <- as.character(packageVersion("roxygen2")) - patched <- exists("mdxml_keep_sentence_spacing", envir = asNamespace("roxygen2")) + patched <- exists("mdxml_keep_sentence_spacing", envir = asNamespace("roxygen2"), inherits = FALSE) cat("roxygen2", v, "patched:", patched, "\n") - if (!grepl("[.]9100$", v) || !patched) { + # Assert the whole shape, not just the suffix: a malformed stamp such as + # "\001.9100" also ends in .9100, which is how the escaping bug in this + # very expression went unnoticed until CI refused to install the result. + if (!grepl("^[0-9]+[.][0-9]+[.][0-9]+[.]9100$", v) || !patched) { stop("roxygen2 was not installed from the fork branch.", call. = FALSE) } '