Fix issue with xml file line ending normalization #1378
Conversation
| var leadingWhitespaceNode = XmlDocument.CreateWhitespace("\n" + parentIndentation + DEFAULT_INDENT); | ||
| var trailingWhiteSpaceNode = XmlDocument.CreateWhitespace("\n" + parentIndentation); | ||
| var leadingWhitespaceNode = XmlDocument.CreateWhitespace(Environment.NewLine + parentIndentation + DEFAULT_INDENT); | ||
| var trailingWhiteSpaceNode = XmlDocument.CreateWhitespace(Environment.NewLine + parentIndentation); |
There was a problem hiding this comment.
This seems fine, but it's probably not exactly right. We should have a file-format newline that's distinct from the current OS newline, or folks will be confused when projects created/modified on Windows and Linux are different.
| // This issue does not apply to XmlTextReader (above) which is not shipped with .NET Core yet. | ||
|
|
||
| // NOTE: This doesn't work in .NET Core. | ||
| //var xmlReaderType = typeof(XmlReader).GetTypeInfo().Assembly.GetType("System.Xml.XmlTextReaderImpl"); |
There was a problem hiding this comment.
How does it fail? Just can't find XmlTextReaderImpl "next to" XmlReader?
There was a problem hiding this comment.
On Full Framework settings the Normalize property to false fixes the issue and all the tests pass (using XmlReader). On .NET Core, the Normalize property is not there so it can't set it. I tried doing what the Normalize setter does in FF (set _normalize = value and _ps.eolNormalized = !value) but it doesn't have any effect on the line normalization. I suspect it's happening too late but I'm not 100% sure.
Background: * Previous implementation on Full Framework used XmlTextReader(path). This contained an issue with certain characters (dotnet#985) and was fixed by using streams (dotnet#1004). dotnet#1004 also changed from XmlTextReader to XmlReader. * XmlReader contains logic to normalize line endings. Internally, it sets the Normalize property to true and replaces (some? all?) \r\n with \n. This change switches implementation to use XmlTextReader. This class sets the internal Normalize to false and does not replace \r\n with \n. This fixes dotnet#1340. However, .NET Core does not ship with XmlTextReader, only XmlReader. dotnet#1340 still exists for .NET Core.
AndyGerlicher
force-pushed
the
xmltextreaderfix
branch
from
9be8258
to
7a7b61a
Compare
I've spent way too much time on this already. Pushing to get feedback or ideas. You can see I tried to set _normalize = false. This worked on Full Framework, but seems to do nothing on .NET Core. Whatever implementation I use seems to have some sort of bug and is not usable...
Background:
This change switches implementation to use XmlTextReader with a stream on Full Framework. This class sets the internal Normalize to false and does not replace \r\n with \n. This fixes #1340 and keeps the fix @maddin2016 made for #985.
However, .NET Core does not ship with XmlTextReader, only XmlReader. #1340 still exists for .NET Core.