stevedlawrence commented on code in PR #1452:
URL: https://github.com/apache/daffodil-vscode/pull/1452#discussion_r2417049466
##########
src/daffodilDebugger/utils.ts:
##########
@@ -59,33 +106,59 @@ export async function runDebugger(
dfdlDebugger: DFDLDebugger,
createTerminal: boolean = false
): Promise<vscode.Terminal> {
- const dfdlVersion = daffodilVersion(filePath)
+ const dfdlScalaVersions = daffodilScalaVersions(filePath)
+
+ if (
+ !dfdlDebugger.timeout.endsWith('s') &&
+ !dfdlDebugger.timeout.endsWith('m') &&
+ !dfdlDebugger.timeout.endsWith('s')
+ ) {
+ vscode.window.showErrorMessage(
+ `DFDL Debugger Timeout ${dfdlDebugger.timeout} does not end in either s
for seconds, m for minutes or h for hours.
+ Appending s to end of timeout string.`
+ )
+ dfdlDebugger.timeout = `${dfdlDebugger.timeout}s`
+ }
+
+ // Locates the $JAVA_HOME, or if not defined, the highest version available.
+ const javaHome: IJavaHomeInfo | undefined = await getJavaHome()
+
+ const isAtLeastJdk17: boolean = parseFloat(javaHome?.version ?? '0') >= 17
+ outputChannel.appendLine(
+ `[DEBUG] Choosing java home at ${javaHome?.path}, version
${javaHome?.version}, is at least JDK 17: ${isAtLeastJdk17}`
+ )
+
+ let dfdlVersion = dfdlDebugger.version
+ let dfdlVersionKey = await getDaffodilVersionKey(dfdlVersion)
+ let scalaVersion = dfdlScalaVersions[dfdlVersionKey]
+
+ outputChannel.appendLine(
+ `[INFO] Using Scala ${scalaVersion} + Daffodil ${dfdlVersion} debugger`
+ )
+
+ /**
+ * The Scala 3 with Daffodil >= 4.0.0 debugger can only be ran on JDK 17 or
greater. So if the java version
+ * being used is less than 17, fallback to the Scala 2.13 and Daffodil
3.11.0 debugger and notify the user.
+ */
+ if (dfdlVersionKey == '>=4.0.0' && !isAtLeastJdk17) {
Review Comment:
Suggest we just to a semver check, e.g.
```ts
if (semver.satifies(dfdlVersion, ">=4.0.0)) {
...
}
```
We don't really need the versionKey for that.
##########
package.json:
##########
@@ -3,7 +3,11 @@
"displayName": "Apache Daffodil™ Extension for Visual Studio Code",
"description": "Apache Daffodil™ Extension for Visual Studio Code providing
DFDL syntax highlighting, DFDL code completion, DFDL schema debugging, and data
editor",
"version": "1.4.2-SNAPSHOT",
- "daffodilVersion": "3.11.0",
+ "daffodilScalaVersions": {
+ "<=3.10.0,<3.11.0": "2.12",
+ ">=3.11.0,<4.0.0": "2.13",
+ ">=4.0.0": "3"
+ },
Review Comment:
Thoughts on changing the commas to spaces. Then these keys are actual
semantic version ranges and you can use the `semver` library (already a
dependency) to check if the user configured daffodil version is within any of
the ranges and you don't have to do your own parsing.
##########
package.json:
##########
@@ -685,8 +689,18 @@
},
"dfdlDebugger": {
"type": "object",
- "description": "Configuration for debugger. Settings are
logging (level and file)",
+ "description": "Configuration for debugger. Settings are
version (any valid Daffodil version), timeout and logging (level and file)",
"properties": {
+ "version": {
+ "type": "string",
+ "default": "3.11.0",
+ "description": "Debugger version to use. Any Daffodil
version is valid"
Review Comment:
Suggest we call this "Daffodil version to use".
##########
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,
Review Comment:
I wonder if instead of this being `--daffodilVersion`, maybe it just wants
to be something like `--daffodilPath` and it's just a path to the daffodi lib
directory containing daffodil jars? That way the extension can choose to put
the jar files where ever it wants, pass in the right path, and then things just
work. This way if the extenion ever changes where it stores things you don't
have to update the bat/bash files. They'll just add whatever path you give it
to the classpath.
##########
src/daffodilDebugger/daffodil.ts:
##########
@@ -71,8 +71,18 @@ export interface BuildInfo {
scalaVersion: string
}
-export function getDaffodilVersion(filePath: fs.PathLike) {
- return jsoncParse(fs.readFileSync(filePath).toString())['daffodilVersion']
+export interface DaffodilScalaVersions {
+ d3_10_0: string
+ d3_11_0: string
+ d4_0_0: string
+}
Review Comment:
I don't see these "d3_10_0", etc. used anywhere. I'm guessing I just don't
know how type script interfaces work. Can you explain what these are for? I
would expect these to be something like "2_12" , "2_13", "3" or something, but
maybe that's not how they are used?
--
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]
