Skip to content

fix-up R parser bug in get_source_expressions#1056

Merged
MichaelChirico merged 2 commits intomasterfrom
fix-octal
Apr 10, 2022
Merged

fix-up R parser bug in get_source_expressions#1056
MichaelChirico merged 2 commits intomasterfrom
fix-octal

Conversation

@MichaelChirico
Copy link
Copy Markdown
Collaborator

Split off from #1032 because in principle it could affect the logic whenever STR_CONST is involved (especially, it's probably needed as part of #1034)

AshesITR
AshesITR previously approved these changes Apr 9, 2022
@MichaelChirico MichaelChirico changed the title fix-up R parser bug in get_source_expressions WIP: fix-up R parser bug in get_source_expressions Apr 9, 2022
@MichaelChirico
Copy link
Copy Markdown
Collaborator Author

Adding a NEWS bullet and tests...

@MichaelChirico MichaelChirico changed the title WIP: fix-up R parser bug in get_source_expressions fix-up R parser bug in get_source_expressions Apr 9, 2022
@MichaelChirico MichaelChirico requested a review from AshesITR April 9, 2022 20:53
@AshesITR
Copy link
Copy Markdown
Collaborator

Will this fix the xml as well? I suppose no, just wanted to confirm.

@MichaelChirico
Copy link
Copy Markdown
Collaborator Author

MichaelChirico commented Apr 10, 2022

Yes it will, because xmlparsedata builds from the parsed data.frame if we pass on.

Here we pass the data.frame:

tryCatch(xml2::read_xml(xmlparsedata::xml_parse_data(parsed_content)), error = function(e) NULL)

Here xmlparsedata builds off the data.frame it receives instead of running utils::getParseData() again:

https://github.com/r-lib/xmlparsedata/blob/ee6a15f0b9162e2d215635d9771dc5c09d3eacbf/R/package.R#L65

(this is why the other touch-ups, i.e. for tabs and equals signs, are also reflected on the XML tree we get back from xml_parse_data())

@MichaelChirico MichaelChirico merged commit f1643b7 into master Apr 10, 2022
@MichaelChirico MichaelChirico deleted the fix-octal branch April 10, 2022 08:02
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