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: [email protected] With regards, Apache Git Services
