stevedlawrence commented on code in PR #1452:
URL: https://github.com/apache/daffodil-vscode/pull/1452#discussion_r2416730701


##########
build.sbt:
##########
@@ -221,3 +183,134 @@ lazy val xjcSettings =
       cachedFun(Set(daffodilLibJar)).toSeq
     }
   )
+
+lazy val `daffodil-debugger` = project
+  .in(file("."))
+  .settings(commonSettings, ratSettings)
+  .settings(publish / skip := true)
+  .aggregate(debuggers.projectRefs: _*)
+
+/** Since using projectMatrix, there will be a debugger, debugger2_12 and 
debugger3 target. The debugger target is for
+  * Daffodil 3.11.0 and Scala 2.13. The debugger2_12 target is for Daffodil 
3.10.0 and Scala 2.12. The debugger3 target
+  * is for Daffodil 4.0.0 and Scala 3. (only availabe when using JDK 17+)
+  *
+  * When running something like "sbt test" that will run all targets. To use a 
single target do one of: sbt
+  * debugger/test OR sbt debugger2_12/test OR sbt debugger3/test. Based on 
which version of the debugger you are
+  * targeting.
+  */
+lazy val debuggers = {
+  val debugger = (projectMatrix in (file("debugger")))
+    .enablePlugins(BuildInfoPlugin, JavaAppPackaging, UniversalPlugin, 
ClasspathJarPlugin, SbtXjcPlugin)

Review Comment:
   Is the ClasspathJarPlugin still needed now that you do your own classpath 
logic? Or is that still used for the daffodil-debugger classpath, and then we 
just append the daffodil jars to that?



##########
src/daffodilDebugger/utils.ts:
##########
@@ -48,7 +57,45 @@ export const shellArgs = (port: number, isAtLeastJdk17: 
boolean) => {
         ['-J--add-opens', '-Jjava.base/java.lang=ALL-UNNAMED']
       )
     : []
-  return ['--listenPort', `${port}`].concat(extraArgs)
+  return [
+    '--listenPort',
+    `${port}`,
+    '--listenTimeout',
+    `${timeout}`,
+    '--daffodilVersion',
+    daffodilVersion,
+  ].concat(extraArgs)
+}
+
+function compareVersions(v1: string, v2: string): number {

Review Comment:
   We should use semver.satisfies or semver.compare for these version 
comparisions



##########
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:
   Is this none case hit if the user doesn't provide a `daffodilVersion` 
option? Since that option defaults to 3.11.0, seems like we would still want to 
output that version so it's clear what's version is actually being used, even 
if it wasn't specified.
   
   Maybe define a constant somewhere like 'val DaffodilDefaultVersion = 
"3.11.0"`, and then the above becomes something like:
   
   ```scala
   val header = s"""
     ...
     JVM version: ...
     Daffodil version: ${daffodilVersion.getOrElse(DefaultDaffodilVersion)}
     ...
     """
   ```
   
   Though, based on my above comment I would expect daffodil to already be on 
the classpath and you can do something like
   
   ```scala
   import org.apache.daffodil.lib.Misc
   ...
   
   """Daffodil version: ${Misc.getDaffodilVersion}"""
   ```



-- 
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]

Reply via email to