Skip to content
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

Register tufte engines in .onLoad()? #117

Closed
MichaelChirico opened this issue Oct 1, 2022 · 4 comments
Closed

Register tufte engines in .onLoad()? #117

MichaelChirico opened this issue Oct 1, 2022 · 4 comments

Comments

@MichaelChirico
Copy link

Hi all,

We are working on an issue in {lintr} whereby tufte's specific engines are not recognized and thus cause spurious lints / syntax issues: r-lib/lintr#796

We'd like to avoid a solution where we hard-code some information specific to {tufte}, which opens a pandora's box for us to maintain this information not only for {tufte}, but any other engine-generating {knitr} downstreams as well.

The solution we've landed on for {bookdown} is pretty simple -- we need only run loadNamespace("bookdown") and voila! we can use knitr::knit_engines$get() to tell us what engines are registered. See https://github.com/r-lib/lintr/pull/1552/files/94c50f355a53c2d50e0fd8f1be1f99d51531e297

That's because {bookdown} is registering its engines in its .onLoad() hook:

https://github.com/rstudio/bookdown/blob/7b53f2dd7ddb57b7e78ee1575c06b892f549c4e8/R/zzz.R#L4-L6
https://github.com/rstudio/bookdown/blob/7b53f2dd7ddb57b7e78ee1575c06b892f549c4e8/R/utils.R#L589-L594

Is there any disadvantage to doing something similar for the engines {tufte} provides?

@yihui
Copy link
Member

yihui commented Oct 3, 2022

We registered the marginfigure engine in different output format functions because it behaves differently for different output formats. We could definitely register it in .onLoad() and overwrite it inside output format functions. That should be harmless to this package.

@yihui yihui moved this to Backlog in R Markdown Team Projects Oct 3, 2022
@yihui yihui moved this from Backlog to Todo In Progress in R Markdown Team Projects Oct 3, 2022
@yihui yihui closed this as completed in a097026 Oct 3, 2022
Repository owner moved this from Todo In Progress to Done in R Markdown Team Projects Oct 3, 2022
@MichaelChirico
Copy link
Author

Oh that makes sense, thanks for clarifying.

So adding a registration in .onLoad in this case is kind of like a virtual class declaration? the "class" of engine is declared but the implementation is left to "subclasses", is that a good way to describe it?

@yihui
Copy link
Member

yihui commented Oct 3, 2022

Yes.

@github-actions
Copy link

github-actions bot commented Apr 4, 2023

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants