stevedlawrence commented on a change in pull request #74: Daffodil trailing sep
URL: https://github.com/apache/incubator-daffodil/pull/74#discussion_r195099378
 
 

 ##########
 File path: 
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/NilEmptyCombinatorParsers.scala
 ##########
 @@ -19,14 +19,8 @@ package org.apache.daffodil.processors.parsers
 
 import org.apache.daffodil.processors.TermRuntimeData
 
-case class SimpleNilOrEmptyOrValueParser(ctxt: TermRuntimeData, nilParser: 
Parser, emptyParser: Parser, valueParser: Parser)
-  extends AltCompParser(ctxt, Seq(nilParser, emptyParser, valueParser))
-
 case class SimpleNilOrValueParser(ctxt: TermRuntimeData, nilParser: Parser, 
valueParser: Parser)
-  extends AltCompParser(ctxt, Seq(nilParser, valueParser))
-
-case class SimpleEmptyOrValueParser(ctxt: TermRuntimeData, emptyParser: 
Parser, valueParser: Parser)
-  extends AltCompParser(ctxt, Seq(emptyParser, valueParser))
+  extends ChoiceParser(ctxt, Seq(nilParser, valueParser))
 
 case class ComplexNilOrContentParser(ctxt: TermRuntimeData, emptyParser: 
Parser, contentParser: Parser)
-  extends AltCompParser(ctxt, Seq(emptyParser, contentParser))
+  extends ChoiceParser(ctxt, Seq(emptyParser, contentParser))
 
 Review comment:
   I like this change to replace AltCompParser with ChoiceParser, they're 
virtually he same. One thing to consider though is that when the ChoiceParser 
fails it creates parser errors that reference "Choices". There's potential 
confusion there. The user might think the error is related to xs:choice in the 
schema but it's actually just us internally trying to parse two different 
things.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to