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

Fix citation(auto=meta) with non-native encoding #689

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@bastistician
Contributor

bastistician commented May 28, 2018

The encoding of the citation test package is changed to latin1.
On Linux, this lets test-build-citation-authors.R fail from read_citation() with

Error in parse(text = x) : 
  invalid multibyte character in parser at line 1

To fix this, the modified create_meta() function re-encodes the character strings obtained from read.dcf() to UTF-8.

@jayhesselberth

This comment has been minimized.

Collaborator

jayhesselberth commented May 28, 2018

Can you confirm whether this fixes #672?

Cc @strengejacke

meta <- as.list(dcf[1, ])
dcf <- read.dcf(path)[1, ]
if ("Encoding" %in% names(dcf)) {
Encoding(dcf) <- dcf[["Encoding"]]

This comment has been minimized.

@hadley

hadley May 28, 2018

Member

I think my approach in https://github.com/r-lib/pkgdown/pull/679/files#diff-4603a117ddf509c48012889b8c3ad44bR12 is more correct. But why doesn't it work?

This comment has been minimized.

@bastistician

bastistician May 28, 2018

Contributor

You are probably right. Your PR to re-encode the dcf content is closer to what, e.g., packageDescription() does, so might be more correct. ;) It does fix the citation(auto=meta) issue. My PR avoids lapply and makes use of the efficient enc2utf8 primitive. But maybe there are drawbacks.

@jayhesselberth

This comment has been minimized.

Collaborator

jayhesselberth commented May 29, 2018

@WastlM can you confirm whether your test passes with Hadley's approach? Also, can you please create a new test directory and test for the latin1 encoding (instead of overwriting the existing CITATION file).

jayhesselberth added a commit that referenced this pull request May 30, 2018

read CITATION with latin1 encoding and citation(auto = meta)
ports test from #689 and code from #679

need to confirm whether this addresses #672, which seems more likely to be a windows bug.

closes #689, closes #679

jayhesselberth added a commit that referenced this pull request May 30, 2018

read CITATION with latin1 encoding and citation(auto = meta) (#692)
ports test from #689 and code from #679

need to confirm whether this addresses #672, which seems more likely to be a bug in R on windows.

closes #689, closes #679
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment