Skip to content

Reworked vignette processing via new 'asis' driver #1394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 19, 2025

Conversation

eddelbuettel
Copy link
Member

Fixes #1393

This PR adds a new vignette driver 'asis' (taken with thanks and appreciation from R.rsp by @HenrikBengtsson but shortened / simplified a little) which preserves hyperlinks in pdf vignettes by keeping the pre-made pdf vignettes 'as is' but merely adding the vignette metainfo via new 'asis' files (which get installed too).

This PR mostly shows the minimal processing steps. It ought to be viable to keep the asis files in a subdirectory, extract the underlying .tex files from the respective .Rmd files in a processing step, prepend the asis info and process as Sweave files (processing once, running bibtex, processing twice). This would not be narrower, but reinvents the wheel a little.

Checklist

  • Code compiles correctly
  • R CMD check still passes all tests
  • Preferably, new tests were added which fail without the change
  • Document the changes by file in ChangeLog

@Enchufa2
Copy link
Member

Enchufa2 commented Jul 18, 2025

Is it necessary to export these functions? Can't they simply be inlined? If we prefer to avoid changing the NAMESPACE, etc. Otherwise, it's fine.

EDIT: Because, as you see in the mention above, others may start depending upon this driver, instead of just using the appropriate package designed and maintained for this. :)

@eddelbuettel
Copy link
Member Author

eddelbuettel commented Jul 18, 2025

Is it necessary to export these functions?

I have three or four (or five) other package where I am doing the pdfpages thing, and this approach is better. But to use it from another package I need to export them. I could put them elsewhere but these other packages also already use Rcpp so it makes some sense...

And yes I could use R.rsp but

> p <- tools::package_dependencies("R.rsp", recursive=TRUE, db=db)[[1]]
> p
[1] "methods"     "stats"       "tools"       "utils"       "R.methodsS3" "R.oo"        "R.utils"    
[8] "R.cache"     "digest"     
> 

it pulls half a dozen packages with it.

@kylebaron
Copy link

kylebaron commented Jul 18, 2025

FWIW ... I'm in similar situation as @eddelbuettel - already using Rcpp and this seems like a very lightweight way to get the vignette.

The help entry is very helpful; you could make it less so and maybe indicate that it is intended for RcppCore use only if you really don't want the public to pick it up. I would definitely not use this if you said please don't use.

Anyway, thanks for showing us how to do this!

KTB

@eddelbuettel
Copy link
Member Author

Hey @kylebaron thanks for chiming in! As you read along my musings, there really are three possible methods and this one (despite the added tiny export of two new functions -- but hey I found another one to retire so it's not all bad ...) is my current favourite. Because keeping rmarkdown (or otherwise) made pdfs 'as is' is good, and having a non-intrusive way to pass these one seems good too. And my previous approach (using latex pdfpages via Rnw) swallos pdf links so that is not as good as I thought it was...

@Enchufa2
Copy link
Member

Mmh, but it seems it is possible to register the vignette engine without exporting the weave and tangle function, as knitr does if I'm not missing anything.

@eddelbuettel
Copy link
Member Author

eddelbuettel commented Jul 18, 2025

The registration call would be (in another client package)

    tools::vignetteEngine("asis", package = pkgname, pattern = "[.](pdf|html)[.]asis$",
                          weave = Rcpp::asisWeave, tangle = Rcpp::asisTangle)	# nocov end

How can we do that without exporting the two tagged functions? There could well be a way I am not seeing right now, what we have in this PR follows the model of R.rsp and it exports the two and has them used by other packages. I was aiming for the same model. Can you do better than I have here?

@Enchufa2
Copy link
Member

Enchufa2 commented Jul 18, 2025

But the client package just needs to set VignetteBuilder to Rcpp::asis, right?, because it's already registered by Rcpp.

I mean I certainly don't call tools::vignetteEngine in my packages to use knitr's engine, or R.rsp's for that matter. I just set the VignetteBuilder tag/variable and suggest the package with the engine.

@eddelbuettel
Copy link
Member Author

Good point, I think I had overlooked that part. In which case asisWeave and asisTange may remain documented but not exported.

And confirmed by adopting RcppAnnoy to use this. Works without exporting, will commit small improvement to PR. Smaller exported surface is better, thanks for catching this.

Also rewords commented example slightly
@eddelbuettel eddelbuettel requested a review from Copilot July 19, 2025 11:42
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a new "asis" vignette driver that preserves hyperlinks in PDF vignettes by processing pre-made PDFs directly rather than wrapping them in LaTeX documents. The implementation replaces the existing .Rnw wrapper files with simplified .asis files containing only vignette metadata.

  • Adds a new "asis" vignette engine borrowed and simplified from the R.rsp package
  • Replaces LaTeX wrapper files (.Rnw) with metadata-only files (.pdf.asis) for all vignettes
  • Updates the build process to copy PDFs directly to the vignettes directory instead of a pdf subdirectory

Reviewed Changes

Copilot reviewed 29 out of 49 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
R/asis.R Implements the new asisWeave and asisTangle functions for processing asis vignettes
R/zzz.R Registers the new asis vignette engine during package load
vignettes/*.pdf.asis New metadata files replacing the old .Rnw wrappers for each vignette
vignettes/*.Rnw Removed LaTeX wrapper files that included PDFs via pdfpages
vignettes/rmd/Makefile Updates PDF copy destination from pdf/ subdirectory to parent directory
man/asisWeave.Rd Documentation for the new asis vignette functions
DESCRIPTION Adds VignetteBuilder field and updates version

@eddelbuettel
Copy link
Member Author

That was actually useful by Copilot as it spotted four typos / cases of sloppy editing by me. None in code though.

@eddelbuettel eddelbuettel merged commit 2f69f10 into master Jul 19, 2025
22 checks passed
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.

Clickable hyperlinks inside vignettes
3 participants