mbeckerle commented on code in PR #1219:
URL: https://github.com/apache/daffodil/pull/1219#discussion_r1571246583
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala:
##########
@@ -346,7 +346,7 @@ object ChoiceBranchImpliedSequence {
* handled as a degenerate sequence having only one element decl within it.
*/
final class ChoiceBranchImpliedSequence private (rawGM: Term)
- extends SequenceTermBase(rawGM.xml, rawGM.optLexicalParent, rawGM.position)
+ extends SequenceTermBase(<sequence />, rawGM.optLexicalParent,
rawGM.position)
Review Comment:
To me ChoiceBranchImpliedSequence is a natural thing to have. Taking it out
would require adding conditional logic to deal with choice branches that are
just single non-sequence terms (i.e., elements - which can be repeating, and
can have separators - by inheriting properties) in many places where now it
just uses SequenceTermBase and doesn't care where the sequence came from.
Tricking it to have dummy empty sequence XML is a natural fix to me.
An alternative I did consider way back, was to make all such implied
sequences be actual manifest sequences. This would be done when parsing the
schema in the ModelGroup code I think. When it parses a choice it would look
for a branch that is not a sequence, and create (and nest) the XML properly at
that point before parsing the schema.
That would establish the invariant that there are NO implied sequences for
choices, they're all in the schema, albeit by inserting them. But I think this
would also be sub-optimal, as errors could complain about a sequence that the
user knows nothing of.
I think a ChoiceBranchImpliedSequence class is the right device to enable
error messages to refer properly to the content of a choice branch, using
sequence-like language when sensible, and avoiding such language when it is
not.
--
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]
