-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Use XmlWriter.Create instead of XmlTextWriter #54949
Use XmlWriter.Create instead of XmlTextWriter #54949
Conversation
src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializer.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializer.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializer.cs
Outdated
Show resolved
Hide resolved
…XmlSerializer.cs Co-authored-by: Stephen Toub <stoub@microsoft.com>
…XmlSerializer.cs Co-authored-by: Stephen Toub <stoub@microsoft.com>
…XmlSerializer.cs Co-authored-by: Stephen Toub <stoub@microsoft.com>
XmlTextWriter xmlWriter = new XmlTextWriter(textWriter); | ||
xmlWriter.Formatting = Formatting.Indented; | ||
xmlWriter.Indentation = 2; | ||
XmlWriter xmlWriter = XmlWriter.Create(textWriter, new XmlWriterSettings() { CheckCharacters = true, IndentChars = " ", Indent = true }); | ||
Serialize(xmlWriter, o, namespaces); |
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.
Why explicitly set CheckCharacters and IndentChars? You are seeting them to their default values. Indent
is the only property which you are changing from the default.
XmlTextWriter xmlWriter = new XmlTextWriter(stream, null); | ||
xmlWriter.Formatting = Formatting.Indented; | ||
xmlWriter.Indentation = 2; | ||
XmlWriter xmlWriter = XmlWriter.Create(stream, new XmlWriterSettings() { CheckCharacters = true, IndentChars = " ", Indent = true }); | ||
Serialize(xmlWriter, o, namespaces); |
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.
Same for this, skip setting values to the defaults.
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.
Remove the extra XmlWriterSettings property setting where you are setting the default
Use XmlReader/Writer.Create instead of XmlTextReader/Writer.
XmlTextWriter is not strict about some of the things it writes. Like
the null character, which is technically illegal in XML 1.0 and 1.1.
(The way we used the XmlTextReader counterpart, we would end
up successfully serializing a null character, and then hit an exception
when trying to deserialize what we just serialized. This inability to
round trip breaks a core tenet of serialization.)
Since the long ago days of .Net 2.0, Microsoft has been
recommending people to use XmlReader/Writer.Create()
instead of directly instantiating XmlTextReader/Writer. This PR
will bring us in line with those recommendations from long ago.
Fixes #1411