stevedlawrence commented on code in PR #1452:
URL: https://github.com/apache/daffodil-vscode/pull/1452#discussion_r2416857850
##########
debugger/src/main/scala/org.apache.daffodil.debugger.dap/DAPodil.scala:
##########
@@ -434,29 +436,55 @@ object DAPodil extends IOApp {
.withDefault(4711),
Opts
.option[Duration]("listenTimeout", "duration to wait for a DAP client
connection (default: 10s)")
- .withDefault(10.seconds)
- ).mapN(Options)
+ .withDefault(10.seconds),
+ Opts
+ .option[String]("daffodilVersion", "version of Daffodil to use
(default: 3.11.0)")
+ .withDefault("3.11.0")
+ ).mapN(Options.apply)
implicit val logger: Logger[IO] = Slf4jLogger.getLogger
- val header =
- s"""|
- |******************************************************
- |A DAP server for debugging Daffodil schema processors.
- |
- |Build info:
- | version: ${BuildInfo.version}
- | daffodilVersion: ${BuildInfo.daffodilVersion}
- | scalaVersion: ${BuildInfo.scalaVersion}
- | sbtVersion: ${BuildInfo.sbtVersion}
- |Runtime info:
- | JVM version: ${System.getProperty("java.version")}
(${System.getProperty("java.home")})
- |******************************************************""".stripMargin
+ def getHeader(daffodilVersion: Option[String]): String = {
+ val header =
+ s"""|
+ |******************************************************
+ |A DAP server for debugging Daffodil schema processors.
+ |
+ |Build info:
+ | version: ${BuildInfo.version}
+ | scalaVersion: ${BuildInfo.scalaVersion}
+ | sbtVersion: ${BuildInfo.sbtVersion}
+ |Runtime info:
+ | JVM version: ${System.getProperty("java.version")}
(${System.getProperty("java.home")})
+
|******************************************************""".stripMargin
+
+ daffodilVersion match {
+ case Some(version) =>
+ header.replace(
+ s"Runtime info:",
+ s"Runtime info:\n Daffodil Version: ${version}"
+ )
+ case None => header
+ }
+ }
+
Review Comment:
It's probably worth adding a comment here why we download the CLI jars.
Maybe something like:
The daffodil debugger doesn't bundle any of daffodil jars because we want to
support many versions of daffodil and do not want to bundle them all. The CLI
zip is an official daffodil release that contains all the daffodil jars (and
their transitive dependencies) that the debugger needs for a particular
Daffodil version. The debugger doesn't need all the jars that it bundles (e.g.
daffodil-cli.jar) but those will just be ignored.
##########
debugger/src/main/scala/org.apache.daffodil.debugger.dap/DAPodil.scala:
##########
@@ -434,29 +436,55 @@ object DAPodil extends IOApp {
.withDefault(4711),
Opts
.option[Duration]("listenTimeout", "duration to wait for a DAP client
connection (default: 10s)")
- .withDefault(10.seconds)
- ).mapN(Options)
+ .withDefault(10.seconds),
+ Opts
+ .option[String]("daffodilVersion", "version of Daffodil to use
(default: 3.11.0)")
Review Comment:
I"m surprised the daffodilVersion is passed as an option. Shouldn't daffodil
already be on the classpath when DAPodil is executed, so this wouldn't be able
to change the daffodil version?
##########
src/tests/suite/daffodilDebugger.test.ts:
##########
@@ -91,66 +104,141 @@ suite('Daffodil Debugger', () => {
const pid = await d.processId
await killProcess(pid)
}
+
// No need to deleted the debugging server because upon re-run, webpack
cleans and re-extracts it.
- if (fs.existsSync(XML_INFOSET_PATH)) fs.rmSync(XML_INFOSET_PATH)
- if (fs.existsSync(JSON_INFOSET_PATH)) fs.rmSync(JSON_INFOSET_PATH)
+ ;(await getDebuggerVersionsToTest()).forEach((version) => {
+ const xmlPath = XML_INFOSET_PATH.replace('.xml', `${version}.xml`)
+ const jsonPath = JSON_INFOSET_PATH.replace('.json', `${version}.json`)
+ if (fs.existsSync(xmlPath)) fs.rmSync(xmlPath)
+ if (fs.existsSync(jsonPath)) fs.rmSync(jsonPath)
+ })
})
+})
- test('should output xml infoset', async () => {
- await vscode.debug.startDebugging(
- undefined,
- getConfig({
- name: 'Run',
- request: 'launch',
- type: 'dfdl',
- schema: {
- path: TEST_SCHEMA,
- },
- data: DATA,
- debugServer: 4711,
- infosetFormat: 'xml',
- infosetOutput: {
- type: 'file',
- path: XML_INFOSET_PATH,
- },
- tdmlConfig: tdmlConf,
- dataEditor: dataEditor,
- dfdlDebugger: dfdlDebuggers[0],
- }),
- {
- noDebug: true,
- }
- )
+// Gets all debugger version to test based on if JDK is >= 17
+async function getDebuggerVersionsToTest(): Promise<Array<string>> {
Review Comment:
I'd suggest calling this getDaffodilVersionsToTest. We're still testing the
same debugger version, just with different versions of Daffodil.
##########
debugger/src/main/scala/org.apache.daffodil.debugger.dap/Parse.scala:
##########
@@ -114,7 +104,7 @@ object Parse {
val parse =
IO.interruptibleMany(
dp.parse(
- new InputSourceDataInputStream(data),
+ Support.getInputSourceDataInputStream(data),
infosetOutputter
Review Comment:
Mentioned above, but this probably wants to change to be something like
`Support.getInfosetInputter(...)`
##########
debugger/src/main/scala/org.apache.daffodil.debugger.dap/Utils.scala:
##########
@@ -70,3 +72,64 @@ object DataLeftOverUtils {
leftOverBitsText + dataHex + dataText
}
}
+
+/** Download and extract utils */
+object DownloadExtractUtils {
Review Comment:
Feels to me like this logic belongs in the extension or bash/bat files. I
imagine VS Code also has some nice way to show download progress (or cancel the
download), which this doesn't have.
--
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]
