shanedell commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2244219363
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
Don't delete this as it could still be used.
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -410,13 +425,12 @@ export function activateDaffodilDebug(
.map((obj) => obj.name) as string[]
// dropdown
- return await vscode.window
- .showQuickPick(test_case_names, {
- placeHolder: 'Test Case Name',
- })
- .then((value) => {
- return value // return selected dropdown value
- })
+ // Await showQuickPick directly and return the result
+ const selection = await vscode.window.showQuickPick(test_case_names, {
+ placeHolder: 'Test Case Name',
+ })
+
+ return selection
Review Comment:
Can't this just be:
```suggestion
// Await showQuickPick directly and return the result
return await vscode.window.showQuickPick(test_case_names, {
placeHolder: 'Test Case Name',
})
```
I am not sure if you really need that comment.
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -436,14 +450,15 @@ export function activateDaffodilDebug(
),
] as string[]
- // dropdown
- return await vscode.window
- .showQuickPick(test_case_unique_descriptions, {
+ // Await showQuickPick directly and return the result
Review Comment:
Can't this just be:
```ts
// Await showQuickPick directly and return the result
return await vscode.window.showQuickPick(test_case_names, {
placeHolder: 'Test Case Descriptio',
})
```
I am not sure if you really need that comment.
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -168,35 +168,50 @@ function createDebugRunFileConfigs(
tdmlConfig.action = tdmlAction
if (tdmlAction === 'execute') {
- tdmlConfig.name = '${command:AskForTDMLName}'
- tdmlConfig.description = '${command:AskForTDMLDescription}'
tdmlConfig.path = targetResource.fsPath
}
}
- vscode.debug.startDebugging(
- undefined,
- getConfig({
- name: 'Run File',
- request: 'launch',
- type: 'dfdl',
- schema: {
- path: tdmlConfig.action === 'execute' ? '' : targetResource.fsPath,
- rootName: null,
- rootNamespace: null,
- },
- data:
- tdmlConfig.action === 'execute' ? '' : '${command:AskForDataName}',
- debugServer: false,
- infosetFormat: 'xml',
- infosetOutput: {
- type: 'file',
- path: '${workspaceFolder}/' + infosetFile,
- },
- tdmlConfig: tdmlConfig,
- }),
- { noDebug: noDebug }
- )
+ const config = getConfig({
+ name: 'Run File',
+ request: 'launch',
+ type: 'dfdl',
+ schema: {
+ path: tdmlConfig.action === 'execute' ? '' : targetResource.fsPath,
+ rootName: null,
+ rootNamespace: null,
+ },
+ data:
+ tdmlConfig.action === 'execute' ? '' : '${command:AskForDataName}',
+ debugServer: false,
+ infosetFormat: 'xml',
+ infosetOutput: {
+ type: 'file',
+ path: '${workspaceFolder}/' + infosetFile,
+ },
+ tdmlConfig: tdmlConfig,
+ })
+
+ // If we're calling execute TDML from a .tdml file
+ // set the name and the description of the TDML test case we want
+ // before running the debugger
+ if (
+ typeof config.tdmlConfig?.path === 'string' &&
+ path.extname(config.tdmlConfig?.path).toLowerCase() == '.tdml' &&
+ typeof config.tdmlConfig?.action === 'string' &&
+ config.tdmlConfig?.action.toLowerCase() == 'execute'
+ ) {
+ config.tdmlConfig.name = await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLName',
+ config
+ )
+
+ config.tdmlConfig.description = await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLDescription',
+ config
+ )
+ }
Review Comment:
I agree with @michael-hoke and I believe the main fix for what he is
referring to do is this:
```typescript
var tdmlConfig = <TDMLConfig>{}
if (tdmlAction) {
tdmlConfig.action = tdmlAction
if (tdmlAction === 'execute') {
tdmlConfig.path = targetResource.fsPath
}
}
// If we're calling execute TDML from a .tdml file
// set the name and the description of the TDML test case we want
// before running the debugger
if (
typeof tdmlConfig.path === 'string' &&
path.extname(tdmlConfig.path).toLowerCase() == '.tdml' &&
typeof tdmlConfig.action === 'string' &&
tdmlConfig.action.toLowerCase() == 'execute'
) {
tdmlConfig.name = await vscode.commands.executeCommand(
'extension.dfdl-debug.getTDMLName',
tdmlConfig
)
tdmlConfig.description = await vscode.commands.executeCommand(
'extension.dfdl-debug.getTDMLDescription',
tdmlConfig
)
}
const config = getConfig({
name: 'Run File',
request: 'launch',
type: 'dfdl',
schema: {
path: tdmlConfig.action === 'execute' ? '' : targetResource.fsPath,
rootName: null,
rootNamespace: null,
},
data:
tdmlConfig.action === 'execute' ? '' : '${command:AskForDataName}',
debugServer: false,
infosetFormat: 'xml',
infosetOutput: {
type: 'file',
path: '${workspaceFolder}/' + infosetFile,
},
tdmlConfig: tdmlConfig,
})
vscode.debug.startDebugging(undefined, config, { noDebug: noDebug })
```
I also don't like this if:
```ts
if (
typeof tdmlConfig.path === 'string' &&
path.extname(tdmlConfig.path).toLowerCase() == '.tdml' &&
typeof tdmlConfig.action === 'string' &&
tdmlConfig.action.toLowerCase() == 'execute'
)
```
I think it would be best to update `src/classes/tdmlConfig.ts` to use
`string` and not `String`. This would be would also be a good change for the
parameters `runOrDebug` and `tdmlAction`. Doing this makes the if statement
become:
```ts
if (
tdmlConfig.path &&
path.extname(tdmlConfig.path).toLowerCase() == '.tdml' &&
tdmlConfig.action &&
tdmlConfig.action.toLowerCase() == 'execute'
)
```
Which makes sure that `path` and `action` are defined before using them, we
should not have to check the type of the attribute.
--
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]
