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

vectorise textVersion of bib entries #1507

Merged
merged 6 commits into from
Feb 23, 2021

Conversation

bastistician
Copy link
Contributor

tweaking a949228 and #1267 to account for CITATION files with multiple bib entries, where pkgdown currently always uses the textVersion even if that is missing

tweaking a949228 and r-lib#1267 to account for CITATION files with multiple bib entries, where pkgdown currently always uses the textVersion even if that is missing
@bastistician
Copy link
Contributor Author

See the current colorspace site at http://colorspace.r-forge.r-project.org/authors.html (generated with pkgdown 1.6.1) for an example of missing HTML versions of the citations:

screenshot

I expect many other packages with multiple citations to be affected by this bug as the textVersion element is often not needed and then "recommended to leave unspecified", see help(bibentry).

@hadley
Copy link
Member

hadley commented Feb 19, 2021

Could you provide a test or two too please?

@bastistician
Copy link
Contributor Author

Could you provide a test or two too please?

Yes, see my additional commit.
Note that all CITATION tests are excluded from R CMD check.

BTW, I think the "textVersion" format should be HTML-escaped in case it contains & < or > (e.g., "Proof of b < a"):

diff --git a/R/build-home-citation.R b/R/build-home-citation.R
index fe19872..a69b757 100644
--- a/R/build-home-citation.R
+++ b/R/build-home-citation.R
@@ -48,7 +48,7 @@ data_citations <- function(pkg = ".") {
   cit <- list(
     html = ifelse(text_version == "",
                   format(cit, style = "html"),
-                  paste0("<p>", text_version, "</p>")),
+                  paste0("<p>", escape_html(text_version), "</p>")),
     bibtex = format(cit, style = "bibtex")
   )

Let me know if I should add this change as a commit to this PR.

Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, would be great to fix the HTML escaping while you're in here (with another test please). Thanks for the help!

tests/testthat/test-build-citation-authors.R Outdated Show resolved Hide resolved
tests/testthat/test-build-citation-authors.R Outdated Show resolved Hide resolved
@hadley
Copy link
Member

hadley commented Feb 19, 2021

Does R already take care of escaping { in bibtex? i.e. what if the title is Proof of b < a { c?

@hadley hadley merged commit 1b34b4a into r-lib:master Feb 23, 2021
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

Successfully merging this pull request may close these issues.

2 participants