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

Fix a bug where TextType content got newlines it shouldn't have #37

Merged
merged 2 commits into from Jul 21, 2019

Conversation

@amoeba
Copy link
Contributor

@amoeba amoeba commented Jul 20, 2019

@jeanetteclark and others found this while adding subscripts to
an EML document. What happens is that as_jsonlist is reading in
the contents of TextType para and section elements, and
converting them to literal XML strings with this code.

<para>H<subscript>2</subscript>O</para> gets turned into

para
  H
  <subscript>2</subscript>
  O

and calling as.character on the above followed by a paste with
collapse = "\n" introduces newlines between each child of the para.
This isn't a huge issue for most uses cases but, when rendering to
HTML via XSLT, you end up with spaces between the H and the 2
because browsers are getting the the newline and converting it to
whitespace.

Fixes ropensci/EML#282

@jeanetteclark and others found this while adding subscripts to
an EML document. What happens is that as_jsonlist is reading in
the contents of TextType `para` and `section` elements, and
converting them to literal XML strings with this code.

<para>H<subscript>2</subscript>O</para> gets turned into

para
  H
  <subscript>2</subscript>
  O

and calling as.character on the above followed by a paste with
collapse = "\n" introduces newlines between each child of the para.
This isn't a huge issue for most uses cases but, when rendering to
HTML via XSLT, you end up with spaces between the H and the 2
because browsers are getting the the newline and converting it to
whitespace.

Fixes ropensci/EML#282
@amoeba
Copy link
Contributor Author

@amoeba amoeba commented Jul 20, 2019

(Failed Travis build looks to be related to the recent switch from the opencpu/jq PPA to jeroen/jq so: unrelated to my PR I think). All tests pass and devtools::check() comes back clean.

@cboettig
Copy link
Member

@cboettig cboettig commented Jul 20, 2019

@amoeba
Copy link
Contributor Author

@amoeba amoeba commented Jul 20, 2019

Okay, done!

@cboettig cboettig merged commit 8ea4420 into ropensci:master Jul 21, 2019
4 checks passed
4 checks passed
codecov/patch 100% of diff hit (target 88.11%)
Details
codecov/project 88.11% (+0%) compared to c67786b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.