tuxji commented on code in PR #898:
URL: https://github.com/apache/daffodil/pull/898#discussion_r1054441591
##########
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:
Mike, please grep (or rg) your checkout thoroughly for all such remaining
places (the dfdlx prefix where dfdl should be used and the treatAsMissing value
where treatAsAbsent should be used). We prefer PR authors to perform such
searches rather than leave it to reviewers to look for overlooked places. In
places where we have to keep "dfdlx" or "treatAsMissing" to avoid breaking
schemas immediately, I suggest adding "// deprecated" as a comment so we can
locate such places when removing all old properties/values in a future cleanup.
##########
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:
[DAFFODIL-2496](https://issues.apache.org/jira/browse/DAFFODIL-2496) says
the new official emptyElementParsePolicy should, if not defined, generate a
suppressable warning, and get a default 'treatAsEmpty'. Yes, we should
re-enable this warning; no, we should not remove the defaulting behavior; yes,
we want to add the property to DFDLGeneralFormat.dfdl.xsd and give it the same
'treatAsEmpty' default value. Schema authors hate to get any new warnings that
would force them to change schemas or suppress the warning, especially since we
already have been applying the default 'treatAsEmpty' behavior all this time.
Schemas which have been defining all the properties themselves (not the best
practice) will hit this warning and have to ignore the warning, suppress the
warning, or define 'dfdl:emptyElementParsePolicy' but schemas which have been
importing DFDLGeneralFormat.dfdl.xsd will produce no warnings and need no
changes (unless they were defining 'dfdlx:emptyElementParsePolicy' an
d now need to change the prefix 'dfdlx' or the value 'treatAsMissing').
Be aware that re-enabling this warning might cause new warnings in some test
cases, so further changes might be needed.
##########
daffodil-test/src/test/resources/org/apache/daffodil/usertests/MultipartBody.dfdl.xsd:
##########
@@ -54,7 +54,7 @@
<xsd:element name="BodyPart"
dfdl:lengthKind="delimited"
minOccurs="1" maxOccurs="unbounded"
type="xsd:string"
dfdl:occursCountKind="implicit"
- dfdlx:emptyElementParsePolicy="treatAsEmpty" />
+ dfdl:emptyElementParsePolicy="treatAsEmpty" />
Review Comment:
Previous lines were indented with tabs and the changed line is indented with
spaces, which makes the changed line seem to be indented differently than the
previous lines in this diff. An automatic formatter would make this problem go
away, but I suggest re-indenting this line with tabs to keep it consistent with
the previous lines for now.
##########
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:
Unfortunately,
https://www.ibm.com/docs/en/integration-bus/10.0?topic=dfdl-unsupported-features
does not say anything about emptyElementParsePolicy.
--
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]
