[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #482: Update debugger support of found delimiters/fields/diffs

[GitBox <gi...@apache.org>]

mbeckerle commented on a change in pull request #482:
URL: https://github.com/apache/incubator-daffodil/pull/482#discussion_r565463159



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1638,6 +1716,15 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
           }
           DebugState.Pause
         }
+
+        def diff(prestate: StateForDebugger, state: ParseOrUnparseState): Boolean = {
+          if (prestate.arrayPos != state.arrayPos) {
+            debugPrintln("%s: %d -> %d".format(name, prestate.arrayPos, state.arrayPos), "  ")

Review comment:
       I see now that this change set does not include any CLI tests that drive this new info-diff system, and that's why codecov is complaining. You should add at least one test that just does info diff on a contrived example that exercises all of these diffs. 

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1528,37 +1542,91 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
         }
       }
 
-      object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs {
+      object InfoFoundDelimiter extends DebugCommand with DebugCommandValidateZeroArgs with InfoDiffable {
         val name = "foundDelimiter"
         override lazy val short = "fd"
         val desc = "display the current found delimiter"
         val longDesc = desc
+
+        private def getDelimOpt(s: StateForDebugger): Option[String] = {
+          if (s.delimitedParseResult.isDefined) {
+            val pr = s.delimitedParseResult.get
+            val delim = Misc.remapStringToVisibleGlyphs(pr.matchedDelimiterValue.get)
+            Some(delim)
+          } else {
+            None
+          }
+        }
+
         def act(args: Seq[String], prestate: StateForDebugger, state: ParseOrUnparseState, processor: Processor): DebugState.Type = {
-          state match {
-            case pstate: PState => {
-              if (pstate.delimitedParseResult.isDefined) {
-                val pr = pstate.delimitedParseResult.get
-                debugPrintln("%s:".format(name))
-                debugPrintln("foundField: %s".format(Misc.remapStringToVisibleGlyphs(pr.field.get)), "  ")
-                debugPrintln("foundDelimiter: %s".format(Misc.remapStringToVisibleGlyphs(pr.matchedDelimiterValue.get)), "  ")
-              } else {
-                debugPrintln("%s: nothing found".format(name))
-              }
-            }
-            case ustate: UState => {
-              // TODO
+          val delimOpt = getDelimOpt(state)
+          val str = delimOpt.getOrElse("nothing found")
+          debugPrintln("%s: %s".format(name, str))

Review comment:
       Codecov is also complaining about this. Seems like this may be spurious? You showed examples of this. Tests of this would have to be under the daffodil-cli/src/test directory.

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -1320,21 +1337,40 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
           }
           DebugState.Pause
         }
+
+        def diff(prestate: StateForDebugger, state: ParseOrUnparseState): Boolean = {
+          if (prestate.bitLimit0b != state.bitLimit0b) {
+            debugPrintln("%s: %d -> %d".format(name, prestate.bitLimit0b, state.bitLimit0b), "  ")

Review comment:
       codecov says this is uncovered. Probably should have a test that outputs ALL diffs. 




----------------------------------------------------------------
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:
users@infra.apache.org