mbeckerle commented on a change in pull request #253: Support terminators and
initiators containing %ES;
URL: https://github.com/apache/incubator-daffodil/pull/253#discussion_r298293890
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/DelimiterParsers.scala
##########
@@ -74,57 +74,20 @@ class DelimiterTextParser(
return false
}
- private def findES(delimIter: DelimiterIterator): Maybe[DFADelimiter] = {
- delimIter.reset()
- while (delimIter.hasNext()) {
- val d = delimIter.next()
- if (d.lookingFor == "%ES;") {
- return One(d)
- }
- }
- Nope
- }
-
- private def findLocalES(state: PState): Maybe[DFADelimiter] = {
- val delimIter = new LocalTypedDelimiterIterator(delimiterType,
state.mpstate.delimiters, state.mpstate.delimitersLocalIndexStack)
- findES(delimIter)
- }
+ override def parse(start: PState): Unit = {
- private def findRemoteES(state: PState): Maybe[DFADelimiter] = {
- val delimIter = new RemoteTypedDelimiterIterator(delimiterType,
state.mpstate.delimiters, state.mpstate.delimitersLocalIndexStack)
- findES(delimIter)
- }
+ val maybeDelimIter =
+ if (delimiterType == DelimiterTextType.Terminator && !isDelimited) {
+ Maybe(new LocalTypedDelimiterIterator(delimiterType,
start.mpstate.delimiters, start.mpstate.delimitersLocalIndexStack.top))
+ } else if (delimiterType == DelimiterTextType.Initiator ||
!start.delimitedParseResult.isDefined) {
+ Maybe(new
RemoteTerminatingMarkupAndLocalTypedDelimiterIterator(delimiterType,
start.mpstate.delimiters, start.mpstate.delimitersLocalIndexStack.top))
+ } else {
+ Nope
+ }
- override def parse(start: PState): Unit = {
val foundDelimiter =
- if (delimiterType == DelimiterTextType.Initiator ||
!start.delimitedParseResult.isDefined) {
- //
- // We are going to scan for delimiter text.
- //
- // FIXME: This is incorrect. It grabs all local delimiters even if
we're last in a group so the
Review comment:
I don't think you should remove this comment.
```
<sequence separator=";">
<element foo ...>
<sequence separator =",">
<element bar ...>
<element baz ...>
</sequence>
<element quux ...>
</sequence>
```
The above is effectively a 4-tuple foo;bar,baz;quux where the middle comma
is the separator of the inner sequence.
When scanning for the end of the baz element, we shoudlnt' be scanning for
either "," or ";". Rather we should scan only for ";". That's the bug this
fixme is about.
We should create a test for this I guess, and assuming my analysis is
correct and this bug exists, create a JIRA ticket and comment out the test,
marked by that ticket.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
