stevedlawrence commented on a change in pull request #86: Adding value of 'bcd' to binaryCalendarRep URL: https://github.com/apache/incubator-daffodil/pull/86#discussion_r205421540
########## File path: daffodil-core/src/main/scala/org/apache/daffodil/grammar/ElementBaseGrammarMixin.scala ########## @@ -496,8 +497,14 @@ trait ElementBaseGrammarMixin case LengthKind.Implicit => implicitBinaryLengthInBits case LengthKind.Explicit if (lengthEv.isConstant) => explicitBinaryLengthInBits() case LengthKind.Explicit => -1 // means must be computed at runtime. - case LengthKind.Delimited if (binaryNumberRep == BinaryNumberRep.Binary) => subsetError("lengthKind='delimited' only supported for packed binary formats.") - case LengthKind.Delimited => -1 // only for packed binary data, length must be computed at runtime. + case LengthKind.Delimited => primType match { + case PrimType.DateTime | PrimType.Date | PrimType.Time => + if (binaryCalendarRep == BinaryCalendarRep.BinaryMilliseconds || binaryCalendarRep == BinaryCalendarRep.BinarySeconds) + subsetError("lengthKind='delimited' only supported for packed binary formats.") Review comment: Should this be a normal schemaDefinitionError instead of subsetError? I believe subsetErrors are for things that are required by the spec but we just have chosen not to implement. The spec says dfdl:lengthKind can be delimited for "elements of number or calendar simple type with dfdl:representation 'binary' that have a packed decimal representation". So it sounds lik ebinarySec/Mil should never be allowed. This was incorrect before this patch, but since you're already updated this code it would be good to fix. ---------------------------------------------------------------- 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