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

Update read.jl to pass through Custom XML internal files #261

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

TimG1964
Copy link
Contributor

@TimG1964 TimG1964 commented Jun 20, 2024

Addresses issue #210

Simple change to read.jl to pass through Custom XML internal files in the same way as binary files are already passed through instead of simply ignoring them. As Issue #210 points out, when they are ignored during read, their absence causes an assertion error during write.

Discussion on Discourse here.

This is a test for issue felipenoris#210. It checks that templates that are read can be written again. It needs an example .xlsx file containing internal customXml files to test.
@TimG1964
Copy link
Contributor Author

TimG1964 commented Jun 21, 2024

I've now added an example .xlsx file that contains customXml internal files. I don't think any of the existing examples contain these internal files. In my example, these elements were not added on purpose by the user but were created by Excel itself in the normal use of the application. I don't know their purpose or function but just acknowledge their presence.

I've also added a test to the test suite to read/write this customXml.xlsx file as a template. In the current master release, this test will fail, replicating issue #210. With my proposed code change to read.jl, the test will pass.

@felipenoris felipenoris merged commit 39400f4 into felipenoris:master Aug 26, 2024
@felipenoris
Copy link
Owner

Awesome! Thanks!

@TimG1964
Copy link
Contributor Author

@felipenoris Could you make a new release for this, please?

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

2 participants