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

Parse significant whitespace #745

Merged
merged 4 commits into from Jul 24, 2020
Merged

Parse significant whitespace #745

merged 4 commits into from Jul 24, 2020

Conversation

Pim-Mostert
Copy link
Contributor

Reference Issue

Fixes #744

What does this implement/fix? Explain your changes.

It configures the XmlTextReader to parse significant whitespace. For reference: XmlTextReader.WhitespaceHandling Property
Note:

Because the XmlTextReader does not have DTD information available to it, SignificantWhitepsace nodes are only returned within an xml:space='preserve' scope.

Any other comments?

I really don't know too much about either SVG or XML, so I hope this fix is appropriate.

Copy link
Member

@mrbean-bremen mrbean-bremen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Can you please add a line in the release notes (doc/ReleaseNotes.md)?

@@ -1,4 +1,4 @@
using System;
using System;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the file encoding has changed.
Please apply setting of EditorConfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to VS code, the file is encoded in UTF-8 with BOM and CRLF, as is specified in .editorconfig. Also, I compared the previous and new version of this file with a HEX editor and the first couple of rows are identical. To be honest, I have no idea why GitHub highlights the first line as a change.

Copy link
Contributor

@H1Gdev H1Gdev Jul 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BOM does not exist in this source file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should now be present

@H1Gdev
Copy link
Contributor

H1Gdev commented Jul 19, 2020

Please add tests like this PR.

@Pim-Mostert
Copy link
Contributor Author

Looks good! Can you please add a line in the release notes (doc/ReleaseNotes.md)?

Sure

@Pim-Mostert
Copy link
Contributor Author

Please add tests like this PR.

Sure, done!

@@ -8,6 +8,7 @@ The release versions are NuGet releases.
* check if BaseUri is absolute (see [PR #738](https://github.com/vvvv/SVG/pull/738))

### Fixes
* fixed the parsing of significant whitespace (see [#744](https://github.com/vvvv/SVG/issues/744) and [PR #744])(https://github.com/vvvv/SVG/pull/745))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #745

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And remove close bracket.

[PR #745](

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it - apologies for my sloppiness

@@ -8,6 +8,7 @@ The release versions are NuGet releases.
* check if BaseUri is absolute (see [PR #738](https://github.com/vvvv/SVG/pull/738))

### Fixes
* fixed the parsing of significant whitespace (see [#744](https://github.com/vvvv/SVG/issues/744) and [PR #745](https://github.com/vvvv/SVG/pull/745))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By convention, the list seems to be in ascending order.
Please add it below the list.

@mrbean-bremen mrbean-bremen merged commit b0f8981 into svg-net:master Jul 24, 2020
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.

Whitespace only text nodes are ignored
3 participants