Closed grimbough closed 1 year ago
Thanks a lot @grimbough for addressing the issue with citations in PDF output from RMD vignettes, and all the rest of small fixes included in this PR. I'm happy to review it whenever you're ready, just let me know. Cheers!
Thanks @aoles I'd happy recieve a review now.
The second set of commits address an issue I've noticed recently where newer versions of pandoc add some additional classes to the footnote div
and the existing code doesn't find them and move them to the margins. I've added a unit tests and a github workflow that uses a variety of Pandoc versions to try and tests for this. You can see and R CMD check that fails with Pandoc 2.17 at https://github.com/grimbough/BiocStyle/actions/runs/2436503421 and then after my patch at https://github.com/grimbough/BiocStyle/actions/runs/2436503421
Thanks for the review @aoles, sorry I haven't replied until now. I'm currently on parerntal leave for a few months and had too many things to tidy up before I stopped work. I'll happily take a look at making the suggested changes when I get some time, but it might be a while.
2. Also, the way how the bibliography in the "Authoring R Markdown vignettes" vignette shows up appended directly after the output of
sessionInfo()
looks somehow weird to me. Do you think you could improve on this? What I could imagine is a similar structure as in the "Bioconductor LATEX Style 2.0" vignette: have "References" as an unnumbered section, and move "Session info" to an Appendix, cf. here.
Hi @aoles For the point above, do we achieve the formatting programmatically, or is just that the default template for the BiocStyle sweave format has a better example layout and so achieve a nicer end product, but someone could still make an ugly document if they tried?
Hi @aoles ,
I finally got round to addressing your review.
1) Turns out setting link-citations: yes
is sufficient for links to work in PDF output too, so that was an easy fix. I've also put that in the header of the Rmd vignette and the two document skeletons, so it will be the default going forward.
2) I've restructured the vignette and document skeletons to include a # References
section before the sessionInfo()
. I also modified the pre_processor()
function to look for that exact section name, and automatically insert the weird <div class='refs>
and the # (APPENDIX) Appendix {-}
so that people don't have to go hunting for that syntax. It's not exactly easy to remember. The PDF and HTML versions of the vignette now render like this:
Not sure why there isn't a heading for the Appendix in the PDF version, but generally I think it looks much more structured than before.
Thanks for taking the time to take a look. It's probably be good to get these merged into the main if you like them. My fork is diverging quite far now!
Hi Mike @grimbough,
I hope you are doing fine. I was wondering whether it would be feasible to wrap-up the review soonish in order to merge before the upcoming BioC release? Please let me know if there is anything I can help you with.
Looking forward to hearing from you.
Cheers! Andrzej
Thanks @grimbough for all your nice work! 👍 I appreciate you efforts in maintaining BiocStyle ❤️
This primarily addresses #94 where citations can't be used inside an R Markdown vignette using
BiocStyle::pdf_document()
I've added a new pandoc template that includes the missing latex environments
CSLReferences
andcslreferences
required by citeproc. I've passed this on similar features found in the tufte package. Interestingly rmarkdown now seems to use the templates shipped with pandoc, and maybe that would also be a way to deal with this, but the current approach seems to work.I've also added a GitHub workflow to build the R Markdown vignette with various versions of Pandoc and TinyTex. I've also added a citation to the Rmd vignette so that the referencing infrastructure can be tested. This workflow would fail before my patch here is applied.
I've also made three small changes to
Bioconductor.sty
to suppress some warnings that are always produced, but which I don't think are informative to an end user as they can't do anything to change them. These were:I can restore the if they may be useful, but I've always been slightly annoyed to see them.
~I don't see an easy way to fix the first error, so I just suppress it with the silence package.~ I've reverted this, it was breaking the Sweave vignette. The second might influence the layout of a two-sided PDF, but I'm not sure it's actually possible to produce one of those with BiocStyle. The third states it is ignored, so I guess it doesn't do anything and I've removed the line that leads to it.