stevedlawrence commented on code in PR #1452:
URL: https://github.com/apache/daffodil-vscode/pull/1452#discussion_r2416939951
##########
debugger/src/templates/bash-template:
##########
@@ -263,10 +269,13 @@ run() {
java_opts="${JAVA_OPTS}"
fi
+ # add daffodil jars to classpath
+ daffodil_version="${daffodil_version:-3.11.0}"
Review Comment:
There's a number of places where "3.11.0" is used as a default. I'm not sure
how easy it would be to consolidate that to a single place to make changing the
default easier, but maybe have a convention that they all have the same comment
or something to make it clear where it needs to change?
Or alternatively, maybe make it so most places require the version be passed
in and it's error if not provided. Then you only need a default defined in the
VS code extension part. Things like the bash script and DAPodil are likely
always going to be executed by the extension which we can ensure always
provides the options.
##########
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 {
+ const a1 = v1.split('.').map(Number)
+ const a2 = v2.split('.').map(Number)
+ const len = Math.max(a1.length, a2.length)
+
+ for (let i = 0; i < len; i++) {
+ const n1 = a1[i] ?? 0
+ const n2 = a2[i] ?? 0
+ if (n1 < n2) return -1
+ if (n1 > n2) return 1
+ }
+
+ return 0
+}
+
+export async function getDaffodilVersionKey(
Review Comment:
Feels like we shouldn't ever really need the actual version key. For
example, if we want to get the scala binary version we could do this:
```ts
function getDaffodilScalaVersion(daffodilVersion: String): String {
for ([range,scalaVersion] in dfdlScalaVersions) {
if (semver.satifies(dfdlVersion, range)) {
return scalaVersion;
}
}
}
```
This way we are free to change the smeversions if we ever need to. For
example, maybe some future verion of daffodil requires Scala 3.7 and that
introduces some binary incompatabilities. We could then add a new matrix for
3.7, and a new dfdlScalaVersions entry.
--
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]
