Support including (R)md files into a manual page #902

gaborcsardi · 2019-09-10T10:54:38Z

This facilitates sharing text between README/vignettes and manual pages.

TODO:

hadley

This is a surprisingly small amount of code!

Can you please add a bullet to news, and some documentation to the rd vignette? I think the most important things to mention are what the path is relative to (the package root?), that the Rmd is copied to a temporary directory (so you can't rely on local files), and that it's not cached (so you'll need to be careful to make it fast).

I think it closes #669, #696, and #799.

.Rbuildignore

R/rd-include-rmd.R

gaborcsardi · 2019-09-10T12:27:33Z

that the Rmd is copied to a temporary directory (so you can't rely on local files), and that it's not cached (so you'll need to be careful to make it fast).

Yeah, let me think about this, it would be great to have caching by default. Unfortunately we need to make a copy to make the links to package objects work.

gaborcsardi · 2019-09-10T12:31:13Z

As for the sectioning, I think it would be reasonable to do this:

if there are no headings in the Rmd, then put the whole generated Rd into \details{}.
if there are subsections in the Rmd (i.e. ## and below), then create subsections inside \details{}.
level 1 headings, i.e. #, go in their own \section{}s.
if there is a level 1 heading, but not at the beginning of the Rmd file, then the beginning of the file goes to \details{}, up to the first #, and then we create new sections.

I think this is pretty intuitive, and most of often this is what you want.

gaborcsardi · 2019-09-11T11:24:10Z

@hadley What should be the working directory? The package dir? The directory of the R file being processed?

Right now document() and roxygenize() do not change the working directory AFAICT. rmarkdown/knitr change it to the directory of the Rmd file.

We could change the working directory or try calculate the file name relative to the package directory. The former is simpler.

hadley · 2019-09-11T11:46:19Z

I think we should consistently use the package directory as the working directory

gaborcsardi · 2019-09-11T12:19:13Z

I think we should consistently use the package directory as the working directory

OK, for now I'll switch for @includeRmd and then we can think about switching for document() and/or roxygenize().

gaborcsardi · 2019-09-11T12:26:50Z

Btw. @includeRmd has proper support for sections now. We could have the same for "regular" roxygen2 markdown, we just need to turn it now, basically. Then we could use # etc. for (sub)sectioning.

Re #799, at this point it is super easy to add an @rmd tag as well, basically the same as @includeRmd() but takes the text from the block, instead of the external file. It would need some conventions where the actual Rmd files are created to support caching, whether they would share an environment, etc. So I will not do this now, but I think we could keep #799 open and have another PR for it later.

gaborcsardi · 2019-09-11T12:38:02Z

Actually, re paths, since rmarkdown/knitr defaults to set the wd to the directory of the Rmd, I think we should do the same, for the chunks of the Rmd. We would still switch to the package directory for running rmarkdown::render(), so the paths you specify in @includeRmd would be relative to that. Is this OK?

hadley

I'm happy for this to close #799 — I'd like to live with this for a while before we consider more changes.

I was thinking mostly about the path to the .Rmd, not paths from within the .Rmd. I know there is a bunch of subtle behaviour in knitr when you change the working directory, so it might break with complex includes (or maybe outputs?), but I think that's fine for a first pass.

Should be documented with a new section in https://roxygen2.r-lib.org/articles/rd.html#do-repeat-yourself

R/markdown-link.R

R/markdown.R

R/rd-include-rmd.R

R/rd.R

R/utils.R

gaborcsardi · 2019-09-11T16:31:09Z

I was thinking mostly about the path to the .Rmd

Yes, that's now relative to the package.

I'll update and add the missing stuff in a minute....

gaborcsardi · 2019-09-11T17:24:17Z

@hadley Fixed all I think. Pls take a look at the text in the vignette, and whether it is at the best place.

hadley · 2019-09-11T18:18:28Z

Can you please merge/rebase fix the failure?

gaborcsardi · 2019-09-11T18:21:03Z

Rebased, but I think it'll still fail, because there is no pandoc.

hadley · 2019-09-11T18:30:41Z

Filed an issue: r-lib/r-azure-pipelines#5

hadley · 2019-09-12T14:45:59Z

Can you please merge/rebase once more and add skip_if_not(rmarkdown::pandoc_available()) so the azure builds pass?

gaborcsardi · 2019-09-12T15:15:58Z

Yeah, a minute. knitr/pandoc also does escaping, so this PR needs an update or at least additional tests, now that the escaping in different in roxygen.

gaborcsardi · 2019-09-12T19:02:38Z

Rebased, and tests are now skipped.

But wait, you added some commits again, let me rebase again....

All Rmd is added to the \details{} section now. Sections/headers are not (yet) handled.

So that the markdown parser will be able to create sections for these.

gaborcsardi · 2019-09-12T19:04:13Z

OK, rebased again.

Rules are: - text before the first (level 1) heading goes into \details{}. - level 1 headings get their own \section{}. - other levels creates subsections, and go into \details{} or a \section{}, depending on whether they appear before the first level 1 heading or not.

- use a child document instead of a full copy - set cache.path and fig.path to the default of the original Rmd, so caching works out of the box - still need to turn caching on in the Rmd

- `@includeRmd` path is relative for the package root - knitr's `root.dir` is set to the directory of the original Rmd. (I.e. _not_ the wrapper Rmd that we create.)

Easier to test and troubleshoot.

gaborcsardi · 2019-09-12T19:12:49Z

And again to fix merge glitches...

hadley reviewed Sep 10, 2019

View reviewed changes

.Rbuildignore Show resolved Hide resolved

R/rd-include-rmd.R Outdated Show resolved Hide resolved

gaborcsardi force-pushed the feature/rmd-import branch 2 times, most recently from 0ab3a00 to 3e0bcc2 Compare September 11, 2019 09:29

hadley reviewed Sep 11, 2019

View reviewed changes

gaborcsardi force-pushed the feature/rmd-import branch from c3b55de to 0bdd879 Compare September 11, 2019 18:20

gaborcsardi force-pushed the feature/rmd-import branch from b36812b to f424b08 Compare September 12, 2019 15:20

gaborcsardi added 3 commits September 12, 2019 20:03

Ignore private package lib

046f837

PoC implementation

d3b8f78

All Rmd is added to the \details{} section now. Sections/headers are not (yet) handled.

Pass proper roxy tags to include Rmd

5a6ad59

So that the markdown parser will be able to create sections for these.

gaborcsardi force-pushed the feature/rmd-import branch from 2a0f6eb to cef8d88 Compare September 12, 2019 19:04

gaborcsardi added 4 commits September 12, 2019 20:11

More includeRmd tests

afaa3af

includeRmd: make caching work better

dac4653

- use a child document instead of a full copy - set cache.path and fig.path to the default of the original Rmd, so caching works out of the box - still need to turn caching on in the Rmd

Better relative path for @includeRmd

9bbc0e7

- `@includeRmd` path is relative for the package root - knitr's `root.dir` is set to the directory of the original Rmd. (I.e. _not_ the wrapper Rmd that we create.)

gaborcsardi added 7 commits September 12, 2019 20:11

Document @includeRmd in the Rd vignette

21becdf

Make get_md_linkrefs() return a vector

a2ec299

Easier to test and troubleshoot.

Use rlang::is_installed()

787f90b

Fix code style

960af48

Add NEWS for @includeRmd

dfc2b3e

Adding Gabor as author

a8721c9

Skip includeRmd tests if no pandoc is available

1b355ff

gaborcsardi force-pushed the feature/rmd-import branch from cef8d88 to 1b355ff Compare September 12, 2019 19:11

hadley added 5 commits September 12, 2019 14:25

WS

9a374ae

News + vignette tweaks

d6e1ec4

Clarify return type of get_md_linkrefs()

1d2a862

Re-document

a4cefe0

More advice on caching

44421c1

hadley merged commit 7a8da92 into master Sep 12, 2019

hadley deleted the feature/rmd-import branch September 12, 2019 20:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support including (R)md files into a manual page #902

Support including (R)md files into a manual page #902

gaborcsardi commented Sep 10, 2019 •

edited

Loading

hadley left a comment •

edited

Loading

gaborcsardi commented Sep 10, 2019

gaborcsardi commented Sep 10, 2019

gaborcsardi commented Sep 11, 2019

hadley commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

hadley left a comment

gaborcsardi commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

hadley commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

hadley commented Sep 11, 2019

hadley commented Sep 12, 2019

gaborcsardi commented Sep 12, 2019

gaborcsardi commented Sep 12, 2019

gaborcsardi commented Sep 12, 2019

gaborcsardi commented Sep 12, 2019

Support including (R)md files into a manual page #902

Support including (R)md files into a manual page #902

Conversation

gaborcsardi commented Sep 10, 2019 • edited Loading

hadley left a comment • edited Loading

Choose a reason for hiding this comment

gaborcsardi commented Sep 10, 2019

gaborcsardi commented Sep 10, 2019

gaborcsardi commented Sep 11, 2019

hadley commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

hadley left a comment

Choose a reason for hiding this comment

gaborcsardi commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

hadley commented Sep 11, 2019

gaborcsardi commented Sep 11, 2019

hadley commented Sep 11, 2019

hadley commented Sep 12, 2019

gaborcsardi commented Sep 12, 2019

gaborcsardi commented Sep 12, 2019

gaborcsardi commented Sep 12, 2019

gaborcsardi commented Sep 12, 2019

gaborcsardi commented Sep 10, 2019 •

edited

Loading

hadley left a comment •

edited

Loading