Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
apache-rat-core/src/main/java/org/apache/rat/report/xml/writer/XmlWriter.java
Fixed
Show fixed
Hide fixed
apache-rat-core/src/main/java/org/apache/rat/report/xml/writer/XmlWriter.java
Fixed
Show fixed
Hide fixed
apache-rat-core/src/main/java/org/apache/rat/report/xml/writer/XmlWriter.java
Fixed
Show fixed
Hide fixed
|
ottlinger
left a comment
ottlinger
left a comment
There was a problem hiding this comment.
Thanks, LGTM - all sonarcloud warnings seem reasonable and should be applied before merging.
Besides I was just wonderung if more tests are needed to cover more of the corner cases that XmlWriter handles.
Thanks for all your work!
| writer.write("<!-- "); | ||
| content(text); | ||
| writer.write(" -->"); | ||
| maybeCloseElement(); |
There was a problem hiding this comment.
Can you reformat this section or is it a UI problem in the web UI? (lines seem to have different indentation)
| @Override | ||
| public void close() throws IOException { | ||
| closeDocument(); | ||
| if (appendable instanceof Closeable) { |
There was a problem hiding this comment.
if (appendable instanceof Closeable) {
Sonarcloud's suggestion seems reasonable to me .... would you mind changing?
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
|
|
||
| public class XmlWriterTest { |
There was a problem hiding this comment.
While scanning the changes in the writer package I wondered if there's enough code coverage for the special cases handled in XmlWriter or if more tests could be added (I only look at the code from the GitHub UI and not in an IDE).
| <name>Apache Creadur RAT::Core</name> | ||
| <description>The core functionality of RAT that is used by all clients.</description> | ||
| <properties> | ||
| <sonar.exclusions>src/main/java/org/apache/rat/report/xml/writer/XMLChar.java</sonar.exclusions> |
There was a problem hiding this comment.
Would it make sense to add a comment here such as
"Code ist copied over from Xerces directly plus a reference to a RAT-xyz?"
Thanks, just for future people to give more context.
| transformer.setOutputProperty(OutputKeys.INDENT, "yes"); | ||
| transformer.setOutputProperty(OutputKeys.ENCODING, "UTF-8"); | ||
| transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "4"); | ||
| Transformer transformer = StandardXmlFactory.create(styleIn); |
There was a problem hiding this comment.
Thanks for addressing this duplicate code warning by putting it in a new class.
2 participants
Update the XmlWriter to write to any appendable.
Move the XMLChar class to its own file and use the implementation for Xerces-j to pick up fixes.