[GitHub] [daffodil] tuxji commented on a diff in pull request #898: Add emptyElementParsePolicy to main DFDL namespace.

[GitBox <gi...@apache.org>]

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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org