stevedlawrence commented on code in PR #1501:
URL: https://github.com/apache/daffodil-vscode/pull/1501#discussion_r2478635776
##########
src/daffodilDebugger/daffodilJars.ts:
##########
Review Comment:
This comment says it doesn't bundle any of the daffodil jars. We might
update this to say it only bundles a single default version, other versions are
downloaded.
##########
src/daffodilDebugger/daffodilJars.ts:
##########
@@ -35,21 +41,44 @@ export async function checkIfDaffodilJarsNeeded(
'getContext'
)) as vscode.ExtensionContext
+ const daffodilPath = `daffodil/apache-daffodil-${daffodilVersion}-bin`
+ const extensionPath = path.join(
+ context.asAbsolutePath('./dist'),
Review Comment:
Does this path assume that the current working directory is in the extension
directory? Is that always true? I would think when a normal user runs the
extension that's not the case? Should this use `context.extensionPath` instead?
##########
src/daffodilDebugger/daffodilJars.ts:
##########
@@ -35,21 +41,44 @@ export async function checkIfDaffodilJarsNeeded(
'getContext'
)) as vscode.ExtensionContext
+ const daffodilPath = `daffodil/apache-daffodil-${daffodilVersion}-bin`
+ const extensionPath = path.join(
+ context.asAbsolutePath('./dist'),
+ daffodilPath
+ )
+
/**
* Global storage paths:
* Mac: /Users/<username>/Library/Application
Support/Code/User/globalStorage/asf.apache-daffodil-vscode
* Windows: %APPDATA%\Code\User\globalStorage\asf.apache-daffodil-vscode
* Linux:
/home/<username>/.config/Code/User/globalStorage/asf.apache-daffodil-vscode
*/
-
- const destFolder = path.join(context.globalStorageUri.fsPath, 'daffodil')
- const binFolder = path.join(
- destFolder,
- `apache-daffodil-${daffodilVersion}-bin`
+ const globalStoragePath = path.join(
+ context.globalStorageUri.fsPath,
+ daffodilPath
)
+ if (fs.existsSync(extensionPath)) {
+ return extensionPath
+ } else {
+ // downloadAndExtractToGlobalStorage only tries to download the Daffodil
CLI release file if
+ // the desired version's bin folder doesn't already exists.
+ await downloadAndExtractToGlobalStorage(daffodilVersion, globalStoragePath)
+ return globalStoragePath
+ }
+}
+
+export async function downloadAndExtractToGlobalStorage(
+ daffodilVersion: string,
+ binFolder: string
+): Promise<void> {
+ const binName = `apache-daffodil-${daffodilVersion}-bin`
+ const destFolder = binFolder
+ .replace(`/${binName}`, '') // for linux/mac
+ .replace(`\\${binName}`, '') // for windows
Review Comment:
Instead of stripping off binName, could we just never add it set
globalStoragePath above to
```ts
const globalStoragePath = path.join(
context.globalStorageUri.fsPath,
"daffodil"
)
```
So we never add bin path in the first place?
Or alternatively, maybe we don't even pass globalStoragePath to this
function. Just pass in the daffodil version, and this can do all the logic,
e.g.:
```ts
const destFolder = path.join(context.globalStorageUri.fsPath, "daffodil")
const binName = `apache-daffodil-${daffodilVersion}-bin`
const binFolder = path.join(destFolder, binName)
if (!fs.existsSync(binFolder)) {
await downloadAndExtract('Daffodil CLI JARs', url, destFolder);
}
return binFolder;
```
And then this is just called like
```
return await downloadAndExtractToGlobalStorage(daffodilVersion)
```
This is nice since callers don't have to make assumptions about how cached
versions are stored. All the logic is in the function, and it just returns a
path to use.
--
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]
