stevedlawrence commented on code in PR #821:
URL: https://github.com/apache/daffodil/pull/821#discussion_r1038443766
##########
daffodil-japi/src/main/scala/org/apache/daffodil/japi/infoset/Infoset.scala:
##########
@@ -244,7 +246,7 @@ class XMLTextInfosetOutputter private (outputter:
SXMLTextInfosetOutputter)
* insert indentation and newlines where it will not affect the
* content of the XML.
*/
- def this(os: java.io.OutputStream, pretty: Boolean) = this(new
SXMLTextInfosetOutputter(os, pretty))
+ def this(os: java.io.OutputStream, pretty: Boolean, xmlOutputStyle:
XMLOutputStyle) = this(new SXMLTextInfosetOutputter(os, pretty, xmlOutputStyle))
Review Comment:
I think this should be a new constructor, so we have both a 2-arg and 3-arg
constructor, and each one calls the appropriate SXMLTextInfosetOutputter
constructor. This would allow for backwards compatibility. Ideally users of the
current XMLTextInfosetOutputter (Including existing API tests) shouldn't be
required to add the 3 parameter.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala:
##########
@@ -435,9 +437,9 @@ class InteractiveDebugger(runner:
InteractiveDebuggerRunner, eCompilers: Express
}
}
- private def infosetToString(ie: InfosetElement): String = {
+ private def infosetToString(ie: InfosetElement, xmlOutputStyle:
XMLOutputStyle): String = {
val bos = new java.io.ByteArrayOutputStream()
- val xml = new XMLTextInfosetOutputter(bos, true)
+ val xml = new XMLTextInfosetOutputter(bos, true, xmlOutputStyle)
Review Comment:
I'd suggest the debugger be opinionated about what kind of Infoset it
outputs and just use the 2-arg constructor to get the default standard output
style. I don't think it's important that this be configurable in he debugger.
Passing aroudn this xmlOutputSyle in this and other functions can be removed.
And if it was, it should be controlled by a debugger setting, similar to
other settings like breakOnFailure, removeHidden, infosetLines, etc. Tunables
aren't really used to control the debugger.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/XMLTextInfosetOutputter.scala:
##########
@@ -156,8 +158,11 @@ class XMLTextInfosetOutputter private (writer:
java.io.Writer, pretty: Boolean)
if (!isNilled(simple) && simple.hasValue) {
if (simple.erd.optPrimType.get == NodeInfo.String) {
- val simpleVal = simple.dataValueAsString
- if (simple.erd.runtimeProperties.get(XMLTextInfoset.stringAsXml) ==
"true") {
+ var simpleVal = simple.dataValueAsString
+ if (xmlOutputStyle == XMLOutputStyle.PrettyPrintSafe){
Review Comment:
I think this check and simpleVal change wants to be in the other else block.
If `stringAsXML` is true, then we don't care at all about this pretty print
stuff since the string will be output as raw XML. I think the logic wants to be
something like:
```scala
val simpleVal = simple.dataValueAsString
if (....stringAsXml == "true") {
writeSringAsXml(simpleVal)
} else {
val simpleValEscaped = xmlOutputStyle match {
case XMLOutputStyle.Standard => // Utility.escape stuff
case XMLOutputStyle.PrettyPrintSafe => // CDATA format stuff
}
writer.write(simpleValEscaped)
}
```
This also easily extends to other XMLOutputStyles if we ever add more.
##########
daffodil-japi/src/main/scala/org/apache/daffodil/japi/infoset/Infoset.scala:
##########
@@ -244,7 +246,7 @@ class XMLTextInfosetOutputter private (outputter:
SXMLTextInfosetOutputter)
* insert indentation and newlines where it will not affect the
* content of the XML.
Review Comment:
Needs documentation describing the xmlOutputStyle param.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
