stevedlawrence commented on a change in pull request #478:
URL: https://github.com/apache/daffodil/pull/478#discussion_r617512405
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -84,21 +180,42 @@ class DaffodilParseOutputStreamContentHandler(out:
OutputStream, pretty: Boolean
writer.write(System.lineSeparator())
outputIndentation(writer)
}
+
+ // extracts attributes (or new mappings in the case where namespacePrefix
== true) from atts
+ val pmAttrRes = processAttributes(atts)
+ /**
+ * represents the attributes for the current element. We use this to
generate the attributes list
+ * within the start tag
+ */
+ val currentElementAttributes = pmAttrRes._1
+ currentElementPrefixMapping =
+ if (pmAttrRes._2 != null) {
+ currentElementPrefixMapping
+ } else {
+ NamespaceBinding(pmAttrRes._2.prefix, pmAttrRes._2.uri,
currentElementPrefixMapping)
+ }
Review comment:
Thinking more about this, does this just wanted to be this?
```scala
if (pmAttrRes._2 != null) {
currentElementPrefixMapping = pmAttrRes._2
}
```
The idea being that if pmAttrsRes._2 was set, that means we are not getting
prefix mappings via startPrefixMapping, but instead get them from processing
the attributes. In that case, we shouldn't have gotten any attributes from
startPrefixMapping and just override it with whatever we got from
processingAttributes?
Which makes me wonder if this logic of changing currentElementPrefixMapping
wants to be in processAttributes? That function already mutates
activePrefixMappings, so maybe it should also mutate
currentElementPrefixMappings? That way processAttributes only needs to retuen
the attributes and not the new prefix mappings. And then startElement can
assume that one of startPrefixMapping or processAttributes mutated these state
variables correctly and it doesn't have to worry about it?
Another thought, is it possible to get prefix mappings from both
startPrefixMaping and mappings parsed in proessAttributes? If so should
processAttributes merge mappings it finds with currentElementPrefixMapping? Or
should currentElementPrefixMapping always be null if processAttributes finds
prefix mappings?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
