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

Use with pkgdown #26

Open
eddelbuettel opened this issue Mar 28, 2022 · 7 comments
Open

Use with pkgdown #26

eddelbuettel opened this issue Mar 28, 2022 · 7 comments

Comments

@eddelbuettel
Copy link

eddelbuettel commented Mar 28, 2022

One package I look after uses pkgdown; the decision to do so predates my involvement with the package. After switching the vignettes in that package to simplermarkdown, I noticed that pkgdown plainly ignored them. That appears to be a bug in pkgdown and I reported it as issue #2065 there.

Making pkgdown use simplermarkdown vignettes is still possibly but needs two small changes to accomodate pkgdown:

  • as it only looks at .Rmd files, we are forced to create the files with this ending (at least for the processing, I much prefer to keep .md files in the repo)
  • as it ignores VignetteIndexEntry we have to add a YAML header with a title field.

Again, I consider this to be clear deficiencies in pkgdown: if and when R recognises vignette files (as evidenced by browseVignettes()), so should other tools. But we will see if this view is shared, and a change is going to be made.

Until then, or in general, both of these issues could be addressed with a contributed helper function. We could tuck it away in inst/scripts or something to not have another dependency on a YAML parser just for this. That said, do you have code to parse the header (and html comment) of current simplermarkdown files?

@djvanderlaan
Copy link
Owner

Not sure I completely understand.

Can't you just add the header with the title to the file in combination with the VignetteIndexEntry lines? simplermarkdown is fine with that; it will also parse the title.

simplermarkdown does not have to parse anything, which, I think, is one of the nice things of the way the package works. All parsing is done by pandoc (thereby avoiding issues like in pkgdown: a long as pandoc can parse the file it is valid). pandoc returns the parse tree in json format, simplermarkdown modifies the parse tree and passes it back to pandoc. The contents of the yaml header is in the parse tree. Currently I am not using that. If you want to something similar you could look at the first 4 lines of mdweave where the markdown file is converted to json. I think it would also be fine to add a helper function to simplermarkdown that returns the yaml header parsed. Let me know if that would help; that shouldn't be too difficult to implement.

As for the html-comments. The parsing of this is done by the tools in tools. I think that part is really simple. It just looks for lines containing the vignette commands (would guess a simple grep). The html-comments are needed so that pandoc ignores these lines.

@eddelbuettel
Copy link
Author

Can't you just add the header with the title to the file in combination with the VignetteIndexEntry lines?

Yes i will probably write something locally that

 loop over all files vignettes/*.md
 adds a YAML header with title vignette: section
 once done, calls pkgdown::build_site
 restores the files

and I was just wondering if you had any deeper thoughts. The mdweave comment is good, and I agree with the basic premise. It's all a compromise giving is the elegance and simplicity of simplermarkdown (plus md files in the repo, treated better by GitHub I find; smaller tarball; smaller installed package; but one dependency) along with the intgegration by pkgdown.

@djvanderlaan
Copy link
Owner

What package is it? That I can have a look how it is structured now?

@eddelbuettel
Copy link
Author

eddelbuettel commented Mar 29, 2022

The rendered pages are here, and the sources with md files (for R, and GitHub) and Rmd files (for pkgdown) are here.

I don't want to make this about this package, and it is a little large so it is not a good test case. Happy to create one though if it were used to advance the cause. What we have now it fine -- note for example how (even though we use simplermarkdown there is now a 'table of contents' constructed by pkgdown on the right -- but clumsy because of the required file changes pre and post rendering via pkgdown.

PS And I may have to give in and call these .Rmd files as the 'source' link is of course stale. And as you say, I can probably add the YAML header anyway. In which case we'd be done here and would just have to document 'how to co-exist with pkgdown' if one so choses...

@djvanderlaan
Copy link
Owner

Looking at package_vignettes (in https://github.com/r-lib/pkgdown/blob/main/R/package.R).

 base <- path(path, "vignettes")

  if (!dir_exists(base)) {
    vig_path <- character()
  } else {
    vig_path <- dir_ls(base, regexp = "\\.[rR]md$", type = "file", recurse = TRUE)
  }

  vig_path <- path_rel(vig_path, base)
  vig_path <- vig_path[!grepl("^_", path_file(vig_path))]
  vig_path <- vig_path[!grepl("^tutorials", path_dir(vig_path))]

  yaml <- purrr::map(path(base, vig_path), rmarkdown::yaml_front_matter)

It seems that rmarkdown::yaml_front_matter doesn't detect the yaml block properly which seems to be caused by the html-comment at the beginning. When moving the html-comment after the yaml-header, it does detect the yaml-block. So that might be part of the fix. The problem with the extensions remains.

I believe the 'fix' to pkgdown would be to change the following line from line above

vig_path <- dir_ls(base, regexp = "\\.[rR]md$", type = "file", recurse = TRUE)

to

vig_path <- dir_ls(base, regexp = "\\.[rR]?md$", type = "file", recurse = TRUE)

@djvanderlaan
Copy link
Owner

In your example you don't seem to execute any code. In that case it is easy to switch between rmarkdown and simplermarkdown the files can be substituted. But, I believe the fix you now use won't be as easy when there is code that needs to be executed. In that case rmarkdown and simplermarkdown are not compatible.

The 'cleaner' way would then be to run simplermarkdown::md_weave on each file writing the output to a Rmd file. The output of md_weave is plain markdown, so this should also be a valid rmarkdown file.

@eddelbuettel
Copy link
Author

In your example you don't seem to execute any code.

Earlier versions did. Sometimes I punt and just rely on pandoc for typsetting for which your package is great.

The 'cleaner' way would then be to run simplermarkdown::md_weave on each file writing the output to a Rmd file.

Brilliant. I can try that. Then the only cost is a littered (and enlarged) vignettes/ dir. We can live with that. In a sane world they'd take that one-char change as a PR.

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

No branches or pull requests

2 participants