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

Enhancement: support for non-standard RMarkdown directory structures. #1859

Open
3 tasks done
jonathan-g opened this issue Jul 8, 2020 · 7 comments · May be fixed by #2199
Open
3 tasks done

Enhancement: support for non-standard RMarkdown directory structures. #1859

jonathan-g opened this issue Jul 8, 2020 · 7 comments · May be fixed by #2199
Labels
feature a feature request or enhancement theme: paths path related improvment / issue

Comments

@jonathan-g
Copy link
Contributor

I have the same issue as was raised in #146 (and I originally posted this issue as a comment there).

I organize my reveal.js slides for a semester in a directory structure:

semester:
    lecture_lib (common material: css, js, etc.)
        assets (custom css themes, custom js, images, etc. that are shared across the whole project)
        revealjs
        font-awesome
    slides
        class_01
            assets (per-class assets)
                images (images to include)
                videos (video files)
                fig (figures generated by RMarkdown)
        class_02
            assets (per-class assets)
                images (images to include)
                videos (video files)
                fig (figures generated by RMarkdown)
        c;lass_03
            assets (per-class assets)
                images (images to include)
                videos (video files)
                fig (figures generated by RMarkdown)
        ...

This lets me keep a consistent structure for each lecture.

If I use pandoc 2.7.x, RMarkdown works fine with my directory structures, but if I use any later version of Pandoc rendering the .Rmd file fails with an error message about relativeTo() because lib_dir is not a descendant of output_dir.

This is not a bug per-se since by design, RMarkdown does not support this kind of directory structure. But it is something that a number of people have expressed interest in (#146, #472, https://github.com/jayhesselberth/widgetdown/issues/4, workflowr/workflowr#95).

My non-standard (and unsupported) directory structure worked for many years, using a custom reveal.js HTML template for Pandoc to handle a more flexible directory structure but it broke several months ago when Pandoc moved to version 2.9 and rmarkdown introduced html_dependency_header_attrs to fix the duplication of HTML class attributes from <section> to <h[1-9]> tags that Pandoc introduced with version 2.8 or 2.9. I was able to work around this by continuing to work with an older Pandoc version (2.7).

I raise this in part because I think I could address this with a relatively minor PR (I have this working on a fork of rmarkdown at https://github.com/jonathan-g/rmarkdown/tree/minimal-tree-fix, and it changes about 14 lines of code, all of which are in the html_dependencies_as_string() function). I need to test this patch more before submitting it as a PR, but it should allow this non-standard directory structure through the YAML lib_dir parameter without disturbing any use cases except where lib_dir begins with ...


By filing an issue to this repo, I promise that

  • I have fully read the issue guide at https://yihui.org/issue/.
  • I have provided the necessary information about my issue.
    • If I'm asking a question, I have already asked it on Stack Overflow or RStudio Community, waited for at least 24 hours, and included a link to my question there.
    • If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included xfun::session_info('rmarkdown'). I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: remotes::install_github('rstudio/rmarkdown').
    • If I have posted the same issue elsewhere, I have also mentioned it in this issue.
  • I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.

@yihui
Copy link
Member

yihui commented Sep 29, 2020

If the change required is relatively simple, please feel free to submit a PR. I'd love to support the directory structure that you mentioned. Thank you!

@jonathan-g
Copy link
Contributor Author

What I have now is 96 new lines of code in html_dependencies.R comprising

  • one contiguous block of about 86 LOC providing two new internal (i.e., not exported) functions
  • and 10 new lines of code in the function html_dependencies_as_string().

@cderv
Copy link
Collaborator

cderv commented Sep 30, 2020

I think you can submit the PR @jonathan-g !

@jdfoote
Copy link

jdfoote commented May 27, 2021

Any update on this?

I am making revealjs slides for a class, and it seems like it makes sense to just refer to the revealjs library rather than to include it in every subdirectory. I thought that was a common use case for the lib_dir command?

Is there a better approach?

@jonathan-g
Copy link
Contributor Author

Sorry for the delay. I have been holding off because I wanted to test my patch to make sure it doesn't break anything, but I should be able to submit a PR soon. In the meantime, if you want to look at my work, my fork is at https://github.com/jonathan-g/rmarkdown and the patch for the new directory tree structure can be found in the branches https://github.com/jonathan-g/rmarkdown/tree/jg-devel and https://github.com/jonathan-g/rmarkdown/tree/minimal-tree-fix (this last one is prunding my changes down to a minimal patch to keep the PR small).

I've been using the modified version of rmarkdown in the jg-devel branch for the past year with no problems but I'm super cautious about making a PR to such a widely used package and want to take the time to be sure that there aren't unintended breaking changes for other people's use cases.

@jonathan-g
Copy link
Contributor Author

I have submitted PR #2199 to implement this enhancement.

@cderv cderv added feature a feature request or enhancement theme: paths path related improvment / issue labels Jan 7, 2022
@sciatro
Copy link

sciatro commented Feb 26, 2022

Thanks for doing this @jonathan-g. Having google turn up an 8 year old issue (#146) as the top result is disconcerting but great to see your work moving toward acceptance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement theme: paths path related improvment / issue
Projects
Status: Backlog
5 participants