-
Notifications
You must be signed in to change notification settings - Fork 3
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
Remove special chars from xml output #28
Conversation
related to: apache/maven-invoker-plugin#242 |
src/test/java/org/codehaus/plexus/util/xml/Xpp3DomWriterTest.java
Outdated
Show resolved
Hide resolved
Looking at the code as a whole I absolutely do not understand why the framework allows to produce invalid (unescaped) XML. It simply does not make sense to me. |
@slawekjaranowski I have pushed a naive solution to the problem. @elharo WDYT? |
Next commit with removing special chars - fix build in invoker |
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.
Looks reasonable, but I'd like @elharo to check as well.
Can 4.x be used with Maven 3.x? |
Generally no - I will cherry pick to 3.x |
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.
I'm not very familiar with this API or code base, but at first glance it seems OK. I would add documentation in the appropriate spots indicating that C0 controls are omitted from output.
src/test/java/org/codehaus/plexus/util/xml/pull/MXSerializerTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/codehaus/plexus/util/xml/PrettyPrintXMLWriter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/codehaus/plexus/util/xml/PrettyPrintXMLWriter.java
Outdated
Show resolved
Hide resolved
text = escapeXml(text); | ||
|
||
Matcher m = lowersText.matcher(text); | ||
StringBuffer b = new StringBuffer(); |
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.
StringBuilder builder
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.
appendReplacement
require StringBuffer
, StringBuilder
is supported from JDK 9
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.
It occurs to me that there are also some issues in this class with unpaired surrogates, but one piece of technical arcana at a time. :-)
No description provided.