mbeckerle commented on code in PR #1577:
URL: https://github.com/apache/daffodil/pull/1577#discussion_r2439996281
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala:
##########
@@ -81,6 +82,20 @@ trait ElementBaseGrammarMixin
}
}
+ final lazy val checkDelimitedLengthEVDP: Unit = {
+ if (
+ optionEmptyValueDelimiterPolicy.isDefined
+ && emptyValueDelimiterPolicy != EmptyValueDelimiterPolicy.Both
+ && (hasInitiator || hasTerminator)
+ ) {
+ SDW(
+ WarnID.EmptyValueDelimiterPolicyWarning,
+ "dfdl:emptyValueDelimiterPolicy='%s' will be ignored as it's only
implemented for 'both'",
Review Comment:
This should be an SDE shouldn't it? Do we think there are schemas where it
is using EVDP terminator or initiator currently and it is depending on it
behaving as "none". I.e.,. do we have to be backward compatible with this?
If so then I think we need to add a tunable for whether to be strict about
this or not, but I think this is an SDE (subset error) or an SDW based on that
tunable, which can be set to make it a warning now, but advise it will be
changed in the future to be strict, or we'll implement the initiator and
terminator modes of this.
We need this idiom a lot, and are going to need it more and more, so perhaps
it is time to create a simpler way to define these sort of checks so that all
you have to do is name the thing "EmptyValueDelimiterPolicy" the prediate, and
the warning and error rmessage texts. The creation of a corresponding tunable
name, warnID, and check method should all just happen.
Or perhaps you have to write the SDW and SDE message construction as
methods, and a documentation string, but the tunable, the WarnID, XSD for
validation of config, CLI options, etc., all get created automatically.
The only other thing we should have to do is figure out what object has to
call it. i.e., add the line
```
requiredEvaluationsIfActivated(checkDelimitedLengthEVDP)
```
The propgen module could do this. It really should support the need for
rule-based property error/warning checking, and tightening/loosening
restrictions in the implementation about various properties, at least in the
case where it's about exactly one property.
Probably that's a code cleanup/simplifying effort - clearly it's a different
ticket than this one, but we should point to this PR (with the tunable added)
as part of that ticket as a way of highlighting the issue.
##########
daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala:
##########
@@ -50,6 +50,10 @@ class TestSepTests extends TdmlTests {
@Test def test_sep_ssp_never_6 = test
@Test def test_sep_ssp_never_7 = test
+ // DAFFODIL-2205 - EmptyValueDelimiterPolicy only works with both
Review Comment:
Add quotes around the word "both"
--
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]
