-
Notifications
You must be signed in to change notification settings - Fork 547
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
Openxmlreader ignorews option #857
Openxmlreader ignorews option #857
Conversation
a25e9e7
to
51296aa
Compare
51296aa
to
c0ba6c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start. Thanks for the fix and the test. I left a few comments.
test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlReaderTest.cs
Outdated
Show resolved
Hide resolved
test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlReaderTest.cs
Outdated
Show resolved
Hide resolved
test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlReaderTest.cs
Outdated
Show resolved
Hide resolved
Introduce new constructor Restore old constructors
d0b9eda
to
a59969a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there. A couple of doc issues and a question about old behavior.
Adjust comment Restore previous whitespace handling
Thanks for the review. Sorry messed that up. Wasn't careful enough doing the changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the fix!
Hi, sadly today I discovered an issue with the changes. In the case where an xml file has whitespace after the actual content for example: <document>
<body>
...
</body>
</document> (space after the >) the OpenXmlPartReader will crash with an InvalidOperationException as it is trying to get information for a child element with an empty stack |
Changes discussed in #848 @twsouthwick