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

try to avoid passing long xml strings to file.exists() #543

Merged
merged 8 commits into from
Feb 18, 2023

Conversation

JanMarvin
Copy link
Owner

Windows R-devels file.exists() does not like our approach of handing down potential XML or file path. I've added is_xml() in the if-clause. This should check if the xml character string is XML content and file.exists(xml) should be called only if xml is not XML.

Still, in this case we will have to parse actual XML strings twice, due to the nature of the is_xml() function.

Since this error is currently only in Windows R-Devel it might simply be something triggered by current R development. If this ERROR remains over the next couple of days or if CRAN reaches out, we still can merge this or come up with a better idea.

══ Failed tests ════════════════════════════════════════════════════════════════
  ── Error ('test-class-workbook.R:460'): add_drawing works ──────────────────────
  Error in `file.exists(xml)`: file name conversion problem -- name too long?
  Backtrace:1. └─wb_workbook()$add_worksheet()$add_drawing(xml = tmp) at test-class-workbook.R:460:2
   2. └─openxlsx2::xml_node_name(xml)
   3. └─openxlsx2::read_xml(xml, ...)
   4. └─base::file.exists(xml)
  
  [ FAIL 1 | WARN 0 | SKIP 18 | PASS 1167 ]
  Error: Test failures
  Execution halted

JanMarvin and others added 8 commits February 15, 2023 12:13
Avoid reading xml, avoid custom file.exists(), check if nchar(xml) > PATH_MAX
Avoid reading xml, avoid custom file.exists(), check if nchar(xml) > PATH_MAX
…sx2 into lighter_file_exist_call

# Conflicts:
#	R/RcppExports.R
#	src/helper_functions.cpp
remove r-devel for windows
@JanMarvin JanMarvin merged commit ebd639a into main Feb 18, 2023
@JanMarvin JanMarvin deleted the lighter_file_exist_call branch February 18, 2023 12:44
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.

None yet

1 participant