stevedlawrence commented on a change in pull request #440:
URL: https://github.com/apache/incubator-daffodil/pull/440#discussion_r506615302
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/ChoiceTermRuntime1Mixin.scala
##########
@@ -86,11 +86,15 @@ trait ChoiceTermRuntime1Mixin { self: ChoiceTermBase =>
// in the arriving infoset for unparse.
//
val optDefaultBranch = {
- val optEmpty: Option[Term] =
- groupMembers.find { gm =>
+ val optEmpty: Option[Term] = {
+ val emptyBranches = groupMembers.filter { gm =>
val ies = gm.identifyingEventsForChoiceBranch
ies.pnes.isEmpty // empty event list makes it the default, not
simply isOpen
}
+ if (emptyBranches.length > 1)
+ SDW(WarnID.MultipleChoiceBranches, "Multiple choice branches are
empty, defaulting to first empty branch")
Review comment:
According to
[DAFFODIL-2324](https://issues.apache.org/jira/browse/DAFFODIL-2324), there is
an erratum that clarifies the right behavior for how empty branches should be
unparsed. That might not be folded into https://daffodil.apache.org/docs/dfdl/
yet. I don't think the spec on our website has been updated in a while.
My concer is that that "defaulting to the first empty branch" might not be
clear, especially since "defaulting" means something different.
Looking to the Erratum for inspriation, 5.60 says:
>Section 15.1.3 Insert at end of 1st paragraph.
"If the next element to unparse does not identify any branch of the choice,
or there is no next element to unparse, then there must be a choice branch with
no required elements and the first such branch would be selected for unparsing.
A choice branch could consist only of a nest of model groups with no actual
element content or only optional element content."
>
>Section 9.4.3.2 Insert at end of final paragraph.
"If no choice branch is selected, then there must be a choice branch with no
required elements, and the first such branch would be selected."
So rather than "default" you really need something like "element does not
identify a branch" or something. I'm not sure of a way to say that cleanly.
Which is way rather than say a branch is the "default" (which isn't accurate
and potentially confusing), I just suggested saysing X branches can't be
unparsed. Is sort of like one is the negative of the other. Open to other
suggestions though. Succinct but descriptive diagnostic messages aren't easy.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
