stevedlawrence commented on code in PR #806:
URL: https://github.com/apache/daffodil/pull/806#discussion_r910267908
##########
daffodil-cli/src/it/scala/org/apache/daffodil/debugger/TestCLIDebugger.scala:
##########
@@ -1440,5 +1440,114 @@ class TestCLIdebugger {
}
}
+ @Test def test_CLI_Debugger_parse_unparser_not_available(): Unit = {
+ val schemaFile =
Util.daffodilPath("daffodil-test/src/test/resources/org/apache/daffodil/section06/entities/charClassEntities.dfdl.xsd")
+ val inputFile =
Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/input/input3.txt")
+ val (testSchemaFile, testInputFile) = if (Util.isWindows)
(Util.cmdConvert(schemaFile), Util.cmdConvert(inputFile)) else (schemaFile,
inputFile)
+
+ val shell = if (Util.isWindows) Util.start("", envp = DAFFODIL_JAVA_OPTS)
else Util.start("")
+
+ try {
+ val cmd = String.format("%s -d parse -s %s %s", Util.binPath,
testSchemaFile, testInputFile)
+ shell.sendLine(cmd)
+ shell.expect(contains("(debug)"))
+
+ shell.sendLine("break cell")
+ shell.expect(contains("1: cell"))
+ shell.sendLine("display info parser")
+ shell.sendLine("display info unparser")
+ shell.sendLine("continue")
+
+ shell.expect(contains("parser: <Element
name='cell'><DelimiterStackParser>...</DelimiterStackParser></Element>"))
+ shell.expect(contains("unparser: not available"))
+
+ shell.sendLine("complete")
+
+ Util.expectExitCode(ExitCode.Success, shell)
+ } finally {
+ shell.close()
+ }
+ }
+
+ @Test def test_CLI_Debugger_parse_unparser_not_available_no_breaks(): Unit =
{
Review Comment:
I'm not sure a separate test for no breaks is needed. I don't think this
provides any real additional correctness. And integration tests are quite slow
so we want to minimize unnecessary tests.
In fact, this "no breaks" test is probably the correct way to write the
test, with the `expects` immediately after the associated `sendLine`. I would
suggest removing the above test and just keeping this one (with `no_breaks`
removed).
Same suggestion below for the parser not available tests.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala:
##########
@@ -1595,7 +1595,22 @@ class InteractiveDebugger(runner:
InteractiveDebuggerRunner, eCompilers: Express
val desc = "display the current Daffodil " + name
val longDesc = desc
def act(args: Seq[String], state: ParseOrUnparseState, processor:
Processor): DebugState.Type = {
- debugPrintln("%s: %s".format(name, processor.toBriefXML(2))) // only
2 levels of output, please!
+ state match {
+ case pstate: PState => {
+ if (name == "parser") {
+ debugPrintln("%s: %s".format(name, processor.toBriefXML(2)))
+ } else {
+ debugPrintln("unparser: not available")
+ }
+ }
+ case ustate: UState => {
+ if (name == "unparser") {
+ debugPrintln("%s: %s".format(name, processor.toBriefXML(2)))
+ } else {
+ debugPrintln("parser: not available")
+ }
+ }
+ }
Review Comment:
Just a thought, I wonder if there is value in adding a new `info processor`
command, which would print out either the parser or unparser depending on which
is available (i.e. the current behavior of the InfoParser and InfoUnparser
commands)?
And then the "not available" logic would move to the individual
parser/unparse commands. E.g.
```scala
class InfoParser ... {
def act(...) = {
state match {
case _: PState => debugPrintln("%s: %s".format(name,
processor.toBriefXML(2)))
case _ => debugPrintln("%s: not available".format(name))
}
}
}
```
InfoUnparser would be the same but would match on `UState` instead of
`PState`.
We could also deprecate info parser/unparser and just have the info
processor command if we wanted. I'm not sure there's much benefit from having
both when they do basically the same thing.
If there's consensus that this seems reasonable, we can either add it to
this PR, or just open a new feature ticket to implement some other day. I have
no strong preference.
--
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]
