[GitHub] mbeckerle commented on a change in pull request #100: Moved pushDiscriminator to fix separators bug.
mbeckerle commented on a change in pull request #100: Moved pushDiscriminator to fix separators bug. URL: https://github.com/apache/incubator-daffodil/pull/100#discussion_r210262368 ## File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala ## @@ -231,7 +225,6 @@ abstract class OrderedSequenceParserBase( // discarded. // if (!isFirstIteration) pstate.discard(priorState) - pstate.reset(beforeArrayState) Review comment: removed. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] mbeckerle commented on a change in pull request #100: Moved pushDiscriminator to fix separators bug.
mbeckerle commented on a change in pull request #100: Moved pushDiscriminator to fix separators bug. URL: https://github.com/apache/incubator-daffodil/pull/100#discussion_r210260927 ## File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala ## @@ -231,7 +225,6 @@ abstract class OrderedSequenceParserBase( // discarded. // if (!isFirstIteration) pstate.discard(priorState) Review comment: oops. yes I think so. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] mbeckerle commented on a change in pull request #100: Moved pushDiscriminator to fix separators bug.
mbeckerle commented on a change in pull request #100: Moved pushDiscriminator to fix separators bug. URL: https://github.com/apache/incubator-daffodil/pull/100#discussion_r210260854 ## File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala ## @@ -155,8 +148,7 @@ abstract class OrderedSequenceParserBase( // val priorState = if (isFirstIteration) { -beforeArrayState = pstate.mark("before all occurrences") -beforeArrayState +pstate.mark("before all occurrences") } else { pstate.mark("before second/subsequent occurrence") } Review comment: The messages stored in the pstate.mark are different, and that's useful for debugging. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] mbeckerle commented on a change in pull request #100: Moved pushDiscriminator to fix separators bug.
mbeckerle commented on a change in pull request #100: Moved pushDiscriminator to fix separators bug. URL: https://github.com/apache/incubator-daffodil/pull/100#discussion_r210124141 ## File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceParsers.scala ## @@ -299,6 +299,18 @@ final class OrderedSeparatedSequenceParser( val bitPosBeforeSeparator = pstate.bitPos0b + pstate.pushDiscriminator Review comment: The pushD gives a clean false dis criminaltor at start. The only way that disc is set is when we parse The element Itself - which happens after the infix sep as you pointed out - so there can be a disc set after a postfix sep fails but not prefix or infix. I am considering moving the push/ pop into the base class method so it will be centralized and not even an issue here. This is an automated message from the Apache Git Service. To respond to the message, please log on 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