[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #262: Unordered sequences

[GitBox <gi...@apache.org>]

stevedlawrence commented on a change in pull request #262: Unordered sequences
URL: https://github.com/apache/incubator-daffodil/pull/262#discussion_r320314259
 
 

 ##########
 File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
 ##########
 @@ -44,6 +49,23 @@ abstract class SequenceParserBase(
   override lazy val childProcessors = childParsers
 }
 
+abstract class UnorderedSequenceParserBase(
+  srd: SequenceRuntimeData,
+  choiceParser: ChoiceParser,
+  childParsersArg: Vector[Parser])
+  extends SequenceParserBase(srd, childParsersArg) {
+
+  override protected def parse(pstate: PState) = {
+
+    do {
+      choiceParser.parse(pstate)
+    } while (pstate.processorStatus eq Success)
 
 Review comment:
   I don't think the separator parsers have any logic for infix/postifx/prefix. The parser prepended to the child parsers is created from ``sq.sequenceSeparator``. This val is a ``SequenceSeparator`` which extends ``DelimiterText``, which creates a ``TextParser``. The ``TextParser`` just scans for delimiters. It doesn't care about infix/postfix or other things like separator suppression policy.
   
   I'm not saying this is wrong, but I don't understand how this works and it feels like it shouldn't work. The logic for dealing with infix/postfix/prefix with ordered sequences is in SeparatedSequenceParsers, which this doesnt' seem to use at all. If this is correct, I think it needs some documentation about why it works.
   
   Is looks like me like it either tries to parse a single separator or one of any child element, and keeps going as long as that succeeds. For example, if we have this schema:
   ```xsd
   <xs:sequence dfdl:sequenceKind="unordered" dfdl:separator="$" dfdl:separatorPosition="postfix">
     <xs:element name="foo" type="float" />
     <xs:element name="bar" type="int" />
   </xs:sequence>
   ```
   Then this data should work:
   ```
   1.234$5$
   ```
   But this data should not since there's no data before the first postfix separator:
   ```
   $1.234$5$
   ```
   I feel like with the way you have it the latter would sucessfully parse since it doesn't care about separator position as far as I can tell.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services