[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r398093676
##
File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/Term.scala
##
@@ -578,14 +563,13 @@ trait Term
protected def possibleFirstChildTerms: Seq[Term]
/*
- * Returns list of Elements that could be the first child in the infoset
of this model group or element.
- */
+ * Returns list of Elements that could be the first child in the infoset of
this model group or element.
+ */
final lazy val possibleFirstChildElementsInInfoset: Seq[ElementBase] =
LV('possibleFirstChildElementsInInfoset) {
Review comment:
Good idea.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r397988578
##
File path:
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/ChoiceTermRuntime1Mixin.scala
##
@@ -96,20 +91,23 @@ trait ChoiceTermRuntime1Mixin { self: ChoiceTermBase =>
// get recomputed, but it can appear with a stale value (left-over from
parse)
// in the arriving infoset for unparse.
//
- val optDefaultBranch = {
-
+ val optDefaultBranchIfHidden = {
Review comment:
Naming. The "ifHidden" suffix is misleading. This is the default branch,
hidden or 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r397994308 ## File path: daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ExpressionEvaluatingUnparsers.scala ## @@ -135,6 +135,9 @@ class TypeValueCalcUnparser(typeCalculator: TypeCalculator, repTypeUnparser: Unp val origInfosetElement = ustate.currentInfosetNode val tmpInfosetElement = Infoset.newElement(repTypeRuntimeData).asInstanceOf[DISimple] +if (ustate.withinHiddenNest) Review comment: It's possible you don't need this. These temp elements are transient for repType. But it doesn't hurt, and it is consistent that these be marked hidden. So I'd leave it in. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r397975275 ## File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/Term.scala ## @@ -578,14 +563,13 @@ trait Term protected def possibleFirstChildTerms: Seq[Term] Review comment: Unused. Can be deleted. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r397999636 ## File path: daffodil-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequenceGroup.tdml ## @@ -1406,9 +1406,9 @@ - choice - hidden group - outputValueCalc + Schema Definition Error + hidden choice + must be fully defaultable or define dfdl:outputValueCalc Review comment: Better to be less specific about the error messages so that minor changes to them may not break the test. I would just have the error checks look for "defaultable" and "outputValueCalc" somewhere in the message. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r397997231
##
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/PartialNextElementResolver.scala
##
@@ -76,12 +76,8 @@ trait NextElementResolver { self: InfosetInputter =>
// set of possible elements.
//
maybeERD = resolver.maybeNextElement(name, nameSpace, hasNamespace)
-//
-// If the ERD is marked hidden, then we never expect any events
corresponding
-// to its elements.
-//
-if (maybeERD.isDefined && maybeERD.get.isHidden)
- maybeERD = Nope // clobber it back to Nope
+// we address the cases where ERD is in a hidden context where the
elements are assigned
Review comment:
Rephrase. "The cases where are addressed where..."
I had a hard time following the comment in the current wording.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r397971675
##
File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/Term.scala
##
@@ -578,14 +563,13 @@ trait Term
protected def possibleFirstChildTerms: Seq[Term]
/*
- * Returns list of Elements that could be the first child in the infoset
of this model group or element.
- */
+ * Returns list of Elements that could be the first child in the infoset of
this model group or element.
+ */
final lazy val possibleFirstChildElementsInInfoset: Seq[ElementBase] =
LV('possibleFirstChildElementsInInfoset) {
Review comment:
This method seems to be unused and can be deleted. I thought it was deleted,
but it does appear still on master.
The method couldBeLastElementInModelGroup, which has an abstract definition
below, is also not used and it should be deleted. (The implementations of these
are in ModelGroup and ElementBase.)
Ditto for possibleFirstChildTerms above.
Ditto for isSequendceChild above (line 393)
Ditto for possibleNextSiblingTerms below.
I can't put comments directly on the lines, but I walked through this file
looking for unused methods.
hasLaterRequiredSiblings is used only by unit tests, so that AND the unit
tests looking at it can go away.
Same for hasPriorRequiredSiblings.
priorSibling is unused and can be deleted.
Wow, that's quite a bit of code that can be deleted now.
(double check all these - I checked them on master. It's unlikely, but
possible your PR has added a usage of one of these. IDEA's Find Usages is
great.)
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r393887361
##
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/ChoiceCombinator.scala
##
@@ -257,9 +267,8 @@ case class ChoiceCombinator(ch: ChoiceTermBase,
alternatives: Seq[Gram])
if (mapValues.isEmpty)
new NadaUnparser(null)
else {
- val firstBranchUnparser = alternatives(0).unparser
new ChoiceCombinatorUnparser(ch.modelGroupRuntimeData, eventUnparserMap,
-choiceLengthInBits, optDefaultableBranchUnparser, firstBranchUnparser)
+choiceLengthInBits, optDefaultableBranchUnparser)
Review comment:
+1 then.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r392523492
##
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/ChoiceCombinator.scala
##
@@ -217,27 +217,37 @@ case class ChoiceCombinator(ch: ChoiceTermBase,
alternatives: Seq[Gram])
}
override lazy val unparser: Unparser = {
-val optDefaultableBranchUnparser = if (ch.isHidden) {
- // this call is necessary since it will throw an SDE if no choice branch
- // was defaultable
- ch.childrenThatAreRecursivelyNotDefaultableOrOVC
+/*
+ * Since it's impossible to know the hiddenness for terms at this level
(unless
+ * they're a hiddenGroupRef), we always attempt to find a defaultable
unparser.
+ * If we find one, and at runtime, we're in a hidden state, we can use
that. If we
+ * are not in a hidden state, it doesn't get used. If we don't find one,
and we encounter
+ * a hidden group ref here, we know we'll run into that issue at runtime,
so we SDE.
+ */
+val optDefaultableBranchUnparser = {
+ val defaultableBranches = ch.groupMembers.find {
_.isFullyDefaultableOrOVC }
+ if (defaultableBranches.nonEmpty) {
+val defaultableBranchRD = defaultableBranches.get.runtimeData
- val defaultableBranches = ch.groupMembers.filter {
_.childrenThatAreRecursivelyNotDefaultableOrOVC.length == 0 }
- Assert.invariant(defaultableBranches.length > 0)
- if (ch.isHidden && defaultableBranches.length > 1) {
-SDW(WarnID.ChoiceInsideHiddenGroup, "xs:choice inside a hidden group
has unparse ambiguity: multiple branches exist with all children either
defaulable or have the dfdl:outputValueCalc property set. The first branch will
be chosen during unparse. Defaultable branches are:\n%s",
- defaultableBranches.mkString("\n"))
+// Choices inside a hidden group ref are slightly different because we
+// will never see events for any of the branches. Instead, we will just
+// always pick the branch in which every thing is defaultble or OVC.
+val defaultableBranchUnparser =
alternatives.find(_.context.runtimeData =:= defaultableBranchRD).get.unparser
+Some(defaultableBranchUnparser)
+ } else {
+ch match {
+ case cgr: GroupRef if cgr.isHidden => {
+/*
+ * This is a requirement for terms inside a hidden group. If we're
a hidden group ref, we know
+ * our descendants need this requirements, so it's a SDE if they
don't meet it.
Review comment:
"need this requirements"... ambiguous. Did you mean "meets this requirement"
or "requires this" ?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r392525136
##
File path:
daffodil-cli/src/it/scala/org/apache/daffodil/debugger/TestCLIDebugger.scala
##
@@ -900,10 +900,14 @@ class TestCLIdebugger {
}
@Test def test_CLI_Debugger_InfoHidden_1() {
-val schemaFile =
Util.daffodilPath("daffodil-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequencesWithHiddenRefs.dfdl.xsd")
-val inputFile =
Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/input/test_isHidden1.txt")
-val (testSchemaFile, testInputFile) = if (Util.isWindows)
(Util.cmdConvert(schemaFile), Util.cmdConvert(inputFile)) else (schemaFile,
inputFile)
-
+val schemaFile = Util.daffodilPath(
Review comment:
Really bugs me that every one of these tests (not just your new ones) has
this repeated isWindows conditionalization.
We really should be able to abstract this away so that tests don't have this
baggage over and over.
These same 6 lines of code are repeated a lot.
```
val (testSchemaFile, testInputFile) = if (Util.isWindows) {
(Util.cmdConvert(schemaFile),
Util.cmdConvert(inputFile.getAbsolutePath))
} else {
(schemaFile, inputFile)
}
val shell = if (Util.isWindows) Util.start("", envp =
DAFFODIL_JAVA_OPTS) else Util.start("")
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r392523694
##
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/ChoiceCombinator.scala
##
@@ -257,9 +267,8 @@ case class ChoiceCombinator(ch: ChoiceTermBase,
alternatives: Seq[Gram])
if (mapValues.isEmpty)
new NadaUnparser(null)
else {
- val firstBranchUnparser = alternatives(0).unparser
new ChoiceCombinatorUnparser(ch.modelGroupRuntimeData, eventUnparserMap,
-choiceLengthInBits, optDefaultableBranchUnparser, firstBranchUnparser)
+choiceLengthInBits, optDefaultableBranchUnparser)
Review comment:
Before committing this, we really must test mil-std-2045 and vmf. These have
been turning up many problems related to choices for me.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r391066153
##
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/ModelGroupGrammarMixin.scala
##
@@ -68,9 +69,16 @@ trait ModelGroupGrammarMixin
case c: ChoiceTermBase => DelimiterStackCombinatorChoice(c, content)
case s: SequenceTermBase => DelimiterStackCombinatorSequence(s,
content)
}
- } else { groupContentDef }
+ } else {
+groupContent
+ } //TODO: confirm this works; changed from groupContentDef
-finalContent
+ val mgrd = self.modelGroupRuntimeData
+ if (mgrd.isHidden) {
Review comment:
:+1:
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r390413928
##
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/ProcessorStateBases.scala
##
@@ -480,6 +480,17 @@ abstract class ParseOrUnparseState protected (
res
}
+ private var _hiddenDepth = 0
+
+ def incrementHiddenDef = {
+_hiddenDepth += 1
+ }
+ def decrementHiddenDef = {
+_hiddenDepth -= 1
+ }
+
+ def withinHiddenNest: Boolean = _hiddenDepth > 0
Review comment:
That would be good to do.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r390396869
##
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/ProcessorStateBases.scala
##
@@ -480,6 +480,17 @@ abstract class ParseOrUnparseState protected (
res
}
+ private var _hiddenDepth = 0
+
+ def incrementHiddenDef = {
+_hiddenDepth += 1
+ }
+ def decrementHiddenDef = {
+_hiddenDepth -= 1
+ }
+
+ def withinHiddenNest: Boolean = _hiddenDepth > 0
Review comment:
The word "nest" here is important. The concept here is that the runtime is
beneath/inside a hidden group, but not directly/statically, but anywhere within
the dynamic context nest.
I'm open to other names, but the goal is to denote the dynamic extent of the
hidden group.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r389908595
##
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/ModelGroupGrammarMixin.scala
##
@@ -68,9 +69,16 @@ trait ModelGroupGrammarMixin
case c: ChoiceTermBase => DelimiterStackCombinatorChoice(c, content)
case s: SequenceTermBase => DelimiterStackCombinatorSequence(s,
content)
}
- } else { groupContentDef }
+ } else {
+groupContent
+ } //TODO: confirm this works; changed from groupContentDef
-finalContent
+ val mgrd = self.modelGroupRuntimeData
+ if (mgrd.isHidden) {
Review comment:
Good point. This is the schema compiler, so we shouldn't be asking the
runtime data objects, but the DSOM object.
The schema compiler creates the runtime data objects, but shouldn't use
them. What they contain is in service of the runtime.
Interestingly, it's possible that ModelGroupRuntimeData doesn't need an
isHidden member. Whether the parser/unparser is traversing a hidden group ref
or not is represented by way of there being a HiddenGroupParser/Unparser
constructed. So possibly we don't even need ModelGroupRuntimeData.isHidden.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r389901113
##
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##
@@ -1497,6 +1482,17 @@ class InteractiveDebugger(runner:
InteractiveDebuggerRunner, eCompilers: Express
}
}
+ object InfoHidden extends DebugCommand with DebugCommandValidateZeroArgs
{
+val name = "hidden"
+override lazy val short = "hid"
+val desc = "display whether or not we're within a hidden state"
Review comment:
"... within a hidden group." would better match DFDL spec terminology.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r389898693
##
File path:
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/SequenceTermRuntime1Mixin.scala
##
@@ -76,7 +77,8 @@ trait ChoiceBranchImpliedSequenceRuntime1Mixin { self:
ChoiceBranchImpliedSequen
None,
Maybe.Nope,
Maybe.Nope,
- Maybe.Nope)
+ Maybe.Nope,
+ isHidden)
Review comment:
This should always be false here. I.e., a ChoiceBranchImpliedSequence cannot
be created for a group ref. So you can write
```
{
Assert.invariant(!isHidden)
false
}
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r389899025
##
File path:
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ChoiceAndOtherVariousUnparsers.scala
##
@@ -29,69 +29,62 @@ import org.apache.daffodil.util.MaybeInt
class ChoiceCombinatorUnparser(
mgrd: ModelGroupRuntimeData,
eventUnparserMap: Map[ChoiceBranchEvent, Unparser],
- choiceLengthInBits: MaybeInt)
+ choiceLengthInBits: MaybeInt,
+ optDefaultableBranchUnparser: Option[Unparser],
+ firstBranchUnparser: Unparser)
extends CombinatorUnparser(mgrd)
with ToBriefXMLImpl {
override def nom = "Choice"
override lazy val runtimeDependencies = Vector()
- override lazy val childProcessors = eventUnparserMap.map { case (k, v) => v
}.toSeq.toVector
+ override lazy val childProcessors = (eventUnparserMap.map { case (k, v) => v
}.toSeq ++ optDefaultableBranchUnparser).toVector
Review comment:
line too long. Please break at the ++
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r389890072
##
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/ModelGroupGrammarMixin.scala
##
@@ -68,9 +69,16 @@ trait ModelGroupGrammarMixin
case c: ChoiceTermBase => DelimiterStackCombinatorChoice(c, content)
case s: SequenceTermBase => DelimiterStackCombinatorSequence(s,
content)
}
- } else { groupContentDef }
+ } else {
+groupContent
+ } //TODO: confirm this works; changed from groupContentDef
Review comment:
Can we remove this TODO? I think we know this works now.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #332: Refactor isHidden
mbeckerle commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r389894066
##
File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/Term.scala
##
@@ -732,9 +728,8 @@ trait Term
final def possibleFirstChildElementsInInfoset: Seq[ElementBase] =
LV('possibleFirstChildElementsInInfoset) {
val pfct = possibleFirstChildTerms
val firstChildren = pfct.flatMap {
- case e: ElementBase if e.isHidden => Nil
case e: ElementBase => Seq(e)
- case s: SequenceTermBase if s.isHidden => Nil
+ case gr: GroupRef if gr.asModelGroup.isHidden => Nil
Review comment:
The more I read this code the more I want to suggest changing this member
name from isHidden to isHiddenGroupRef.
That seems easier than the other clarifying change which would be to move
isHidden onto GroupRef and remove it from ModelGroup.
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:
us...@infra.apache.org
With regards,
Apache Git Services
