stevedlawrence commented on code in PR #898:
URL: https://github.com/apache/daffodil/pull/898#discussion_r1054367377
##########
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/SequenceChild.scala:
##########
@@ -438,11 +438,11 @@ class ScalarOrderedSequenceChild(sq: SequenceTermBase,
term: Term, groupIndex: I
/**
* Must deal with nils, emptyness and string/hexBinary exceptional behavior
- * including the behavior for dfdlx:emptyElementParsePolicy 'treatAsMissing'
which special cases
+ * including the behavior for dfdlx:emptyElementParsePolicy 'treatAsAbsent'
which special cases
Review Comment:
Should we remove the `x` in the `dfdlx` prefix here since the extension is
deprecated?
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceChildBases.scala:
##########
@@ -74,9 +74,9 @@ object ParseAttemptStatus {
*
* The EmptyRep for simpleTypes enables default values to be substituted at
parse time.
*
- * For simple types xs:string and xs:hexBinary, the property
dfdlx:emptyElementParsePolicy controls
+ * For simple types xs:string and xs:hexBinary, the property
dfdl:emptyElementParsePolicy controls
* whether the EmptyRep is allowed for strings and hexBinary. In required
positions, when
- * dfdlx:emptyElementParsePolicy is 'treatAsMissing', a required
string/hexBinary that has EmptyRep
+ * dfdl:emptyElementParsePolicy is 'treatAsAbsent', a required
string/hexBinary that has EmptyRep
* causes a Parse Error, and an optional EmptyRep causes nothing to be added
to the infoset (the empty string
* or hexBinary value is suppressed). When dfdlx:emptyElementParsePolicy is
'treatAsEmpty', a required
Review Comment:
`dfdlx` should be `dfdl` here.
##########
daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dfdlx.xsd:
##########
@@ -99,6 +99,7 @@
<xs:restriction base="xs:string">
<xs:enumeration value="treatAsEmpty" />
<xs:enumeration value="treatAsMissing" />
+ <xs:enumeration value="treatAsAbsent" />
Review Comment:
I don't think `treatAsAbsent` should be added here? This simple type
restriction is used only for the dfdx extension property, which uses
`treatAsMissing` instead of `treatAsAbsent`. If someone wants to use
`treatAsAbsent`, let's force them to use the non-extension property.
##########
daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml:
##########
@@ -194,11 +194,7 @@
representation="text"
lengthKind="delimited"
separatorPosition="infix"
- dfdlx:emptyElementParsePolicy="treatAsEmpty"/><!-- remove extension
proerty for IBM cross tests -->
- <!--
- Note: dfdlx:emptyElementParsePolicy should become regular DFDL
emptyElementParsePolicy
- once implemented in DAFFODIL-2496. The enum 'treatAsMissing' is renamed to
'treatAsAbsent'
- -->
+ emptyElementParsePolicy="treatAsEmpty"/><!-- remove extension property
for IBM cross tests -->
Review Comment:
Does IBM support this property now? Do we no longer need to remove this
property for cross tests to work?
##########
daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml:
##########
@@ -282,14 +278,10 @@
representation="text"
lengthKind="delimited"
separatorPosition="infix"
- dfdlx:emptyElementParsePolicy="treatAsMissing"/> <!-- remove extension
proerty for IBM cross tests -->
- <!--
- Note: dfdlx:emptyElementParsePolicy should become regular DFDL
emptyElementParsePolicy
- once implemented in DAFFODIL-2496. The enum 'treatAsMissing' is renamed to
'treatAsAbsent'
- -->
+ emptyElementParsePolicy="treatAsAbsent"/> <!-- remove extension property
for IBM cross tests -->
Review Comment:
Same question here, does this still need to be removed for IBM?
##########
daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/ByHandMixins.scala:
##########
@@ -432,7 +432,9 @@ sealed trait EmptyElementParsePolicy extends
EmptyElementParsePolicy.Value
object EmptyElementParsePolicy extends Enum[EmptyElementParsePolicy] {
case object TreatAsMissing extends EmptyElementParsePolicy
case object TreatAsEmpty extends EmptyElementParsePolicy
- override lazy val values = Array(TreatAsMissing, TreatAsEmpty)
+ case object TreatAsAbsent extends EmptyElementParsePolicy
+
+ override lazy val values = Array(TreatAsMissing, TreatAsEmpty, TreatAsAbsent)
def apply(name: String, context: ThrowsSDE): EmptyElementParsePolicy =
stringToEnum("emptyElementParsePolicy", name, context)
Review Comment:
Around line 470 in this file, we have this comment:
```scala
// This property is an extension, so we don't want to require users to
// add this property just to silence this warning, especially since the
// property might change in the future. So silently use a default without
// outputting a warning. We may want to turn this warning on when the
// property makes its way into the official DFDL spec.
//SDW(
// WarnID.EmptyElementParsePolicyError,
// "Property 'dfdlx:emptyElementParsePolicy' is required but not defined,
using tunable '%s' by default.",
// defaultEmptyElementParsePolicy)
```
Do we want to re-enable this warning on now that this property is in the
spec?
Do we also want to remove the defaulting behavior? I imagine no, since it
might break a number of schemas? I guess after having this warning on for some
number of releases we can flip the switch and require this property.
Do we want to add a value to DFDLGeneralFormat.dfdl.xsd? Is there an obvious
choice or is it really dependent on the file format?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
