You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "JeremyYao (via GitHub)" <gi...@apache.org> on 2025/07/29 17:35:30 UTC
[PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
JeremyYao opened a new pull request, #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336
…bugger.
Closes #1029
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254892302
##########
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(
Review Comment:
Added error checking via the vscode window showing an error message and returning, skipping starting debugging
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2242235932
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -137,7 +137,7 @@ async function showTDMLSaveDialog(fileRequested, label, title) {
}
// Function for setting up the commands for Run and Debug file
-function createDebugRunFileConfigs(
+async function createDebugRunFileConfigs(
Review Comment:
I like your thoughts. If you do get to a point where you think we have to flip all of the non-async caller functions to async, please note it as a GitHub issue.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2242810769
##########
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
Review Comment:
We're passing config into the command, but we're only using config.tdmlConfig.path - why not just pass that in?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245719840
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
Yes it is the default, without this variable the `${command:AskForTDMLName}` will not function properly. So we either to need make it not the default or add it back in. The user can still set the value to be `${command:AskForTDMLName}` which once its seen on start up it would be parsed but maybe we don't want that anymore?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245480890
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
For this to be used again, we would need to call it like this: `${command:AskForTDMLName}` and I don't see a way to pass the argument in like that.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2252213792
##########
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:
@JeremyYao Update your code to do this for the tdmlConfig and config, also make sure 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`
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2252208089
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
@JeremyYao @michael-hoke Mean't keep this removed but update the `configurationAttributes`, `launch` config default for the `TDML Case Name` to be `""` instead of `${command:AskForTDMLName}`
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254896337
##########
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:
Changed src/classes/tdmlConfig.ts to use `string`. The code should now look like https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2246082218
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2243362285
##########
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(
Review Comment:
Successfully tested. There should also be an "error" check here. If the user presses escape on the quick pick, we will start the debugging session and pop another name/description quick pick later on. If the user cancels, the return value will be undefined, and we should skip starting the debugger
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#issuecomment-3138032223
From conversations with @michael-hoke, he will be incorporate changes in this PR for his PR for https://github.com/apache/daffodil-vscode/issues/1030 as changes in this PR and his PR may conflict with each other.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254914287
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -168,35 +168,62 @@ function createDebugRunFileConfigs(
tdmlConfig.action = tdmlAction
if (tdmlAction === 'execute') {
- tdmlConfig.name = '${command:AskForTDMLName}'
- tdmlConfig.description = '${command:AskForTDMLDescription}'
- tdmlConfig.path = targetResource.fsPath
+ tdmlConfig.path = targetResource.fsPath.toString()
+
+ if (
+ tdmlConfig.path &&
+ path.extname(tdmlConfig.path).toLowerCase() == '.tdml'
+ ) {
+ tdmlConfig.name =
+ tdmlConfig.name ||
+ (await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLName',
+ tdmlConfig?.path
+ ))
+
+ tdmlConfig.description =
+ tdmlConfig.description ||
+ (await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLDescription',
+ tdmlConfig?.path
+ ))
+
+ if (!tdmlConfig.name || !tdmlConfig.description) {
+ vscode.window.showErrorMessage(
+ 'Error: no TDML test cases were found! Stopping debugging.'
+ )
+ return
+ }
+ } else {
+ vscode.window.showErrorMessage(
+ "Error: TDML file specified doesn't have correct TDML file extension ().tdml)!"
+ )
+ return
+ }
Review Comment:
Removed .tdml file extension check. Removed ? from `tdmlConfig`s
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2240567199
##########
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(
Review Comment:
I don't see how this is any different than what we were doing before. We're `await`ing the command, and the debugger starts while the user is meant to be typing/selecting. When you run the 'Execute TDML' command, can you let the prompt sit for 30 seconds without the backend crashing? Does the debugger start before you've made your selection?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2252399074
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
Should we also change
``` JSON
"description": {
"type": "string",
"default": "${command:AskForTDMLDescription}",
"description": "TDML Case Description"
},
```
to
```JSON
"description": {
"type": "string",
"default": "${command:AskForTDMLDescription}",
"description": ""
},
```
as well?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2251652552
##########
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
Review Comment:
Adressed
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2242193516
##########
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
Review Comment:
Are you able to elaborate on the argument you're referring to? I'm open to the change but, I want to understand the reasoning.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#issuecomment-3135703245
### How I Tested This
1. Clone https://github.com/JeremyYao/daffodil-vscode/tree/jy/1029 and checkout the [jy/1029](https://github.com/JeremyYao/daffodil-vscode/tree/jy/1029) branch
2. run `git clean-fdx; yarn; yarn package`
3. Either package the extension and/or run it in debug mode
4. Make sure you have a .tdml file in your sampleWorkspace. For my tests, I used this
[sampleWorkspace.zip](https://github.com/user-attachments/files/21507181/sampleWorkspace.zip). It contains the tdml file and the DFDL schema I used in this example test case
5. Open up the .tdml file you want to use.
<img width="2059" height="993" alt="image" src="https://github.com/user-attachments/assets/d30f7819-da28-46f2-a496-36b0c88b9222" />
6. Enter in `Default Test Case` and `Generated by DFDL VSCode Extension`. You should notice that the debugger doesn't start until you have entered in values for the test case name and test case description
7. You should now be in a debugging session
<img width="2095" height="452" alt="image" src="https://github.com/user-attachments/assets/c31885de-b8b8-48f9-bb12-e521fc32e2e0" />
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2240562810
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -137,7 +137,7 @@ async function showTDMLSaveDialog(fileRequested, label, title) {
}
// Function for setting up the commands for Run and Debug file
-function createDebugRunFileConfigs(
+async function createDebugRunFileConfigs(
Review Comment:
This needs to be reverted. When you make a function `async`, any callers need to either `await` the function or use a thenable. If neither of those happen, execution will continue in parallel. Even if this works now, we could be creating issues for ourselves down the road when this starts causing issues.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2241142329
##########
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(
Review Comment:
I partially take this back. I think using the awaits here probably will be okay, but you'll have to use the then to set the name. I'm doing something very similar on my setup, and I'm still getting the timeout. I'll test explicitly with this code tomorrow to see if there are any differences.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "rthomas320 (via GitHub)" <gi...@apache.org>.
rthomas320 merged PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2251720457
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
Readded ` "AskForTDMLName"`, and set it to be `""`
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254955866
##########
src/utils.ts:
##########
@@ -441,11 +441,26 @@ export function getTDMLTestCaseItems(
const xml_obj = parser.parse(fileData)
// Read through TDML test cases and populate each TDML test case item if XML file is valid enough
- if (Array.isArray(xml_obj['testSuite']?.['parserTestCase'])) {
+ if (
+ // Otherwise, it's an array
+ Array.isArray(xml_obj['testSuite']?.['parserTestCase'])
+ ) {
return xml_obj['testSuite']['parserTestCase'].map((obj) => ({
name: obj['@_name'],
description: obj['@_description'],
}))
+ } else if (
+ // One item results in an object type
+ xml_obj['testSuite']?.['parserTestCase'] &&
+ typeof xml_obj['testSuite']?.['parserTestCase'] == 'object'
+ ) {
+ const obj = xml_obj['testSuite']?.['parserTestCase']
+ return [
+ {
+ name: obj['@_name'],
+ description: obj['@_description'],
+ },
+ ]
} else {
Review Comment:
From my perspective, chaining ?s and :s makes the code look harder to read and understand.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2255095755
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -399,51 +426,44 @@ export function activateDaffodilDebug(
context.subscriptions.push(
vscode.commands.registerCommand(
'extension.dfdl-debug.getTDMLName',
- async (config) => {
+ async (tdmlConfigPath) => {
// Config item gets passed
// get test case name options for dropdown
- const test_case_names: string[] = getTDMLTestCaseItems(
- config?.tdmlConfig?.path
- )
+ const test_case_names: string[] = getTDMLTestCaseItems(tdmlConfigPath)
Review Comment:
Addressed in https://github.com/apache/daffodil-vscode/pull/1336/commits/ea0b4013c405d3cdcf7991cbea3a051e2cd079b8
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245527262
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
Actually, aren't we still using this as the default value for config.tdmlConfig.name in package.json? If so, how is that going to be affected? How would that even work without the ability to pass in a variable?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245982076
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
I think if we set the default to an empty string, we have error handling already in place for it.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2241032836
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -137,7 +137,7 @@ async function showTDMLSaveDialog(fileRequested, label, title) {
}
// Function for setting up the commands for Run and Debug file
-function createDebugRunFileConfigs(
+async function createDebugRunFileConfigs(
Review Comment:
I think I have to change my mind on this one. I don't think there's a way to avoid making this async. However, this means we need to look at the callers. From what I see, the caller (activateDaffodilDebug) is non-async, but its caller (activate) is. This means we should be able to flip all of the non-async functions to async.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2246082218
##########
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:
@michael-hoke So are you saying to do something more along the line of the code below?
```ts
var tdmlConfig = <TDMLConfig>{}
if (tdmlAction) {
tdmlConfig.action = tdmlAction
if (tdmlAction === 'execute') {
tdmlConfig.path = targetResource.fsPath
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 })
```
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254963137
##########
src/utils.ts:
##########
@@ -441,11 +441,26 @@ export function getTDMLTestCaseItems(
const xml_obj = parser.parse(fileData)
// Read through TDML test cases and populate each TDML test case item if XML file is valid enough
- if (Array.isArray(xml_obj['testSuite']?.['parserTestCase'])) {
+ if (
+ // Otherwise, it's an array
+ Array.isArray(xml_obj['testSuite']?.['parserTestCase'])
+ ) {
return xml_obj['testSuite']['parserTestCase'].map((obj) => ({
name: obj['@_name'],
description: obj['@_description'],
}))
+ } else if (
+ // One item results in an object type
+ xml_obj['testSuite']?.['parserTestCase'] &&
+ typeof xml_obj['testSuite']?.['parserTestCase'] == 'object'
+ ) {
+ const obj = xml_obj['testSuite']?.['parserTestCase']
+ return [
+ {
+ name: obj['@_name'],
+ description: obj['@_description'],
+ },
+ ]
} else {
Review Comment:
For me, this is a case where using a ternary operator makes sense. It's relatively simple logic and we save a lot of repeated code. There are definitely times where ternaries make code harder to read, I just don't think this is one of them.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#issuecomment-3156103013
Just as a sanity check, can you try using a launch config where tdmlConfig.path is set to a non-existent file. The getTDMLTestCases function should be able to handle that properly, but I want to be sure.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#issuecomment-3156293050
> Just as a sanity check, can you try using a launch config where tdmlConfig.path is set to a non-existent file. The getTDMLTestCases function should be able to handle that properly, but I want to be sure.
There isn't a setting to set a tdmlConfig path for the execute action.
<img width="809" height="1195" alt="image" src="https://github.com/user-attachments/assets/ccc43069-9fcf-48db-b7b9-e1747221aab8" />
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#issuecomment-3161368191
> @JeremyYao Please squash the commits and then I will approve
Squashed just now
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2255063768
##########
src/utils.ts:
##########
@@ -441,11 +441,26 @@ export function getTDMLTestCaseItems(
const xml_obj = parser.parse(fileData)
// Read through TDML test cases and populate each TDML test case item if XML file is valid enough
- if (Array.isArray(xml_obj['testSuite']?.['parserTestCase'])) {
+ if (
+ // Otherwise, it's an array
+ Array.isArray(xml_obj['testSuite']?.['parserTestCase'])
+ ) {
return xml_obj['testSuite']['parserTestCase'].map((obj) => ({
name: obj['@_name'],
description: obj['@_description'],
}))
+ } else if (
+ // One item results in an object type
+ xml_obj['testSuite']?.['parserTestCase'] &&
+ typeof xml_obj['testSuite']?.['parserTestCase'] == 'object'
+ ) {
+ const obj = xml_obj['testSuite']?.['parserTestCase']
+ return [
+ {
+ name: obj['@_name'],
+ description: obj['@_description'],
+ },
+ ]
} else {
Review Comment:
Lets hear others' perspectives on this.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254532730
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
Readded `"AskForTDMLName": "extension.dfdl-debug.getTDMLName",`.
Set values under configurationAttributes -> launch to be
```JSON
"name": {
"type": "string",
"default": "",
"description": "TDML Case Name"
},
"description": {
"type": "string",
"default": "",
"description": "TDML Case Description"
},
"path": {
"type": "string",
"default": "${command:AskForTDMLPath}",
"description": "TDML Case Path"
}
```
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2242317927
##########
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:
Thanks for the notes.
### First Paragraph
Can you expand upon what you mean by `I'd prefer to see us modify tdmlConfig before startDebugging is called`?
My understanding of the block of code and rationales for my changes are as follows: `tdmlConfig` appears to be an object used for organizing the object we're passing into `getConfig()` (see attached code snippet below).
The if statements are a safety check and may be warranted in the event of an edge case where `config.tdmlConfig.path` point to a non-tdml file (example: a file w/o the .tdml extension).
```TypeScript
var tdmlConfig = <TDMLConfig>{}
if (tdmlAction) {
tdmlConfig.action = tdmlAction
if (tdmlAction === 'execute') {
tdmlConfig.path = targetResource.fsPath
}
}
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
)
}
vscode.debug.startDebugging(undefined, config, { noDebug: noDebug })
```
### Second paragraph
Could you point to where the filename checks are being handled currently? I can cross-verify and make changes accordingly. As you mentioned, `Unless somebody is going to manually override VSCode's file type selection, we're already automatically detecting TDML files and setting the file type to TDML`, the changes could be good to include in there for catching edge cases when someone overrides file type detection.
We should definitely include checks as you mentioned at the end. Accounting for the scenario the user attempts to execute `Execute TDML` for non-TDML files.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2242799791
##########
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:
It's a bad code smell - we're creating a tdmlConfig object, passing it to the "real" config object, then further modifying it after we've set it. It's better to do the modifications in one place instead of two.
It'd be a good idea to go down this small rabbit hole - look in package.json. We are using VSCode's built-in language detection for tdml (and also for dfdl). In package.json, you'll find the information on how VSCode is identifying these file types, and if you look at the language files for each one, you'll see how we're telling VSCode to handle things like syntax highlighting.
I don't believe we have any file checks in code (or need them) - pretty sure it's all here. We should have all of these checks already in place. If someone gives us a "bad" TDML file, the parsing is going to fail when we try to get them to select a test case. Even if they create their own launch config with a bad test case name/file, the scala side should also have checks to handle that.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2240555089
##########
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 don't like this block here. We're already doing some of these modifications up above - I'd prefer to see us modify tdmlConfig before startDebugging is called, then pass it into the config object without further modifications... or get rid of the above block, but I don't see a good reason to do that.
Also, those filename checks are explicitly done already. Unless somebody is going to manually override VSCode's file type selection, we're already automatically detecting TDML files and setting the file type to TDML. Then, we're limiting the 'Execute TDML' command to only be present when the file type is TDML... We should probably check that we're handling that case gracefully too
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2240562810
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -137,7 +137,7 @@ async function showTDMLSaveDialog(fileRequested, label, title) {
}
// Function for setting up the commands for Run and Debug file
-function createDebugRunFileConfigs(
+async function createDebugRunFileConfigs(
Review Comment:
This needs to be reverted. When you make a function `async`, any callers need to either `await` the function or use a thenable. If neither of those happen, execution order will change depending on how it's called. Even if this works now, we could be creating issues for ourselves down the road when this starts causing issues.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254890981
##########
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:
Addressed by moving this block of close to be integrated with the first if (tdmlAction === 'execute').
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254905328
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -168,35 +168,62 @@ function createDebugRunFileConfigs(
tdmlConfig.action = tdmlAction
if (tdmlAction === 'execute') {
- tdmlConfig.name = '${command:AskForTDMLName}'
- tdmlConfig.description = '${command:AskForTDMLDescription}'
- tdmlConfig.path = targetResource.fsPath
+ tdmlConfig.path = targetResource.fsPath.toString()
+
+ if (
+ tdmlConfig.path &&
+ path.extname(tdmlConfig.path).toLowerCase() == '.tdml'
+ ) {
+ tdmlConfig.name =
+ tdmlConfig.name ||
+ (await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLName',
+ tdmlConfig?.path
+ ))
+
+ tdmlConfig.description =
+ tdmlConfig.description ||
+ (await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLDescription',
+ tdmlConfig?.path
+ ))
+
+ if (!tdmlConfig.name || !tdmlConfig.description) {
+ vscode.window.showErrorMessage(
+ 'Error: no TDML test cases were found! Stopping debugging.'
+ )
+ return
+ }
+ } else {
+ vscode.window.showErrorMessage(
+ "Error: TDML file specified doesn't have correct TDML file extension ().tdml)!"
+ )
+ return
+ }
Review Comment:
You don't need to (and shouldn't) be checking file name for .tdml. What happens if they have a .tdml.xsd file, which is an extension that we recognize as valid TDML?
You also don't need to use tdmlConfig? here - you are initializing tdmlConfig above, so it will never be undefined here.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#issuecomment-3161339017
@JeremyYao Please squash the commits and then I will approve
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#issuecomment-3156313745
Well, not the issue I was going for, but an issue nonetheless.. I will make a new ticket for this, and we'll address it there. I did see the code to handle a bad path - just had to expand the review a bit. I will test and then approve.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2251621003
##########
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:
Addressed
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254950213
##########
src/utils.ts:
##########
@@ -441,11 +441,26 @@ export function getTDMLTestCaseItems(
const xml_obj = parser.parse(fileData)
// Read through TDML test cases and populate each TDML test case item if XML file is valid enough
- if (Array.isArray(xml_obj['testSuite']?.['parserTestCase'])) {
+ if (
+ // Otherwise, it's an array
+ Array.isArray(xml_obj['testSuite']?.['parserTestCase'])
+ ) {
return xml_obj['testSuite']['parserTestCase'].map((obj) => ({
name: obj['@_name'],
description: obj['@_description'],
}))
+ } else if (
+ // One item results in an object type
+ xml_obj['testSuite']?.['parserTestCase'] &&
+ typeof xml_obj['testSuite']?.['parserTestCase'] == 'object'
+ ) {
+ const obj = xml_obj['testSuite']?.['parserTestCase']
+ return [
+ {
+ name: obj['@_name'],
+ description: obj['@_description'],
+ },
+ ]
} else {
Review Comment:
I have changes incoming here due to the removal of the description. I just want to note the code I'm going to be putting in as it's very different than this - I don't think it makes sense to make any changes because of this (not even 100% sure that it would work with the description added back in, but it seems like it would). If I didn't have these description changes incoming, I'd probably want to push for something more concise.
```
const raw_items = xml_obj['testSuite']?.['parserTestCase']
const items = Array.isArray(raw_items)
? raw_items // Set items equal to the array of parserTestCase objects
: raw_items
? [raw_items] // Set items equal to an array of the single object found
: [] // Set items equal to an empty array
return items.map((item) => { name: item['@_name'], description: item['@_description'] }) // Map the parserTestCase objects to the expected return format
```
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254978271
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -168,35 +168,52 @@ function createDebugRunFileConfigs(
tdmlConfig.action = tdmlAction
if (tdmlAction === 'execute') {
- tdmlConfig.name = '${command:AskForTDMLName}'
- tdmlConfig.description = '${command:AskForTDMLDescription}'
- tdmlConfig.path = targetResource.fsPath
+ tdmlConfig.path = targetResource.fsPath.toString()
+
+ tdmlConfig.name =
+ tdmlConfig.name ||
+ (await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLName',
+ tdmlConfig.path
+ ))
+
+ tdmlConfig.description =
+ tdmlConfig.description ||
+ (await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLDescription',
+ tdmlConfig.path
+ ))
+
+ if (!tdmlConfig.name || !tdmlConfig.description) {
+ vscode.window.showErrorMessage(
Review Comment:
I'm not sure that we want this error message here. It makes sense to have one in the case where no test cases are found, but it doesn't make sense if the user presses Esc. Maybe we can meet in the middle and do a toast message in the bottom right, the same way we do for the Infoset being created?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2255069701
##########
src/utils.ts:
##########
@@ -441,11 +441,26 @@ export function getTDMLTestCaseItems(
const xml_obj = parser.parse(fileData)
// Read through TDML test cases and populate each TDML test case item if XML file is valid enough
- if (Array.isArray(xml_obj['testSuite']?.['parserTestCase'])) {
+ if (
+ // Otherwise, it's an array
+ Array.isArray(xml_obj['testSuite']?.['parserTestCase'])
+ ) {
return xml_obj['testSuite']['parserTestCase'].map((obj) => ({
name: obj['@_name'],
description: obj['@_description'],
}))
+ } else if (
+ // One item results in an object type
+ xml_obj['testSuite']?.['parserTestCase'] &&
+ typeof xml_obj['testSuite']?.['parserTestCase'] == 'object'
+ ) {
+ const obj = xml_obj['testSuite']?.['parserTestCase']
+ return [
+ {
+ name: obj['@_name'],
+ description: obj['@_description'],
+ },
+ ]
} else {
Review Comment:
I'm not asking you to make any changes in this PR. I was just noting the upcoming code changes here. Any conversation can be done on the upcoming PR.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245500810
##########
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(
Review Comment:
There should also be an "error" check if the quick pick doesn't have any values to display. It should tell the user that no TDML test cases were found and it should not do the debugging.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245536375
##########
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:
Not the idea place for this comment, but I'm not a fan of modifying the if statement at all beyond what it was before. We already have checks in place for valid values of the action, and the only way to get through the "*.tdml" check is for a user to manually change the type, and if that happens, this will be caught by the new error checking I suggested adding on an above comment.
The other thing to keep in mind here is that if we skip this part, the debugging starts and we have another check later (debugger.ts:~70) on that I've noticed sees the empty name and prompts for it again, but this time, there's no argument being passed in, so the picker is empty.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2240594505
##########
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
Review Comment:
I think there's a really strong argument to changing this from config to the path
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2240555089
##########
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 don't like this block here. We're already doing some of these modifications up above - I'd prefer to see us modify tdmlConfig before startDebugging is called, then pass it into the config object without further modifications... or get rid of the above block, but I don't see a good reason to do that.
Also, those filename checks are explicitly done already. Unless somebody is going to manually override VSCode's file type selection, we're already automatically detecting TDML files and setting the file type to TDML. Then, we're limiting the 'Execute TDML' command to only be present when the file type is TDML.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2242206138
##########
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(
Review Comment:
Sounds like a plan. I posted my testing methodology as a comment (https://github.com/apache/daffodil-vscode/pull/1336#issuecomment-3135703245) if you would like to refer to it.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2242235932
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -137,7 +137,7 @@ async function showTDMLSaveDialog(fileRequested, label, title) {
}
// Function for setting up the commands for Run and Debug file
-function createDebugRunFileConfigs(
+async function createDebugRunFileConfigs(
Review Comment:
These are good thoughts. If you do get to a point where you think we have to flip all of the non-async caller functions to async, please note it as a GitHub issue.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245749014
##########
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:
@michael-hoke I'm not sure what you mean by not being okay with the update to the if statement. The only change happening is making sure the type of the attributes of the TDMLConfig class are `string` and not `String`. This makes it so you don't have to do `typeof tdmlConfig.path === 'string'` or `typeof tdmlConfig.action === 'string'` as both `path` and `action` should only be either a `string` or `undefined`. So, we should not have to strictly check that type for either `path` and `action`, currently you have to do that since its `String | undefined`
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245754117
##########
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:
@michael-hoke I'm not sure what you mean by not being okay with the update to the if statement. The only change happening is making sure the type of the attributes of the TDMLConfig class are `string` and not `String`. This makes it so you don't have to do `typeof tdmlConfig.path === 'string'` or `typeof tdmlConfig.action === 'string'` as both `path` and `action` should only be either a `string` or `undefined`. So, we should not have to strictly check that type for either `path` and `action`, currently you have to do that since its `String | undefined`
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2245759377
##########
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:
I agree with the suggestion that you made. I don't think there should be any changes to the if statement from the original, especially if we're going to move it back up where everything else is already being checked.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2246086042
##########
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:
basically, yes, in addition to the string -> String updates
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2254953984
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -399,51 +426,44 @@ export function activateDaffodilDebug(
context.subscriptions.push(
vscode.commands.registerCommand(
'extension.dfdl-debug.getTDMLName',
- async (config) => {
+ async (tdmlConfigPath) => {
// Config item gets passed
// get test case name options for dropdown
- const test_case_names: string[] = getTDMLTestCaseItems(
- config?.tdmlConfig?.path
- )
+ const test_case_names: string[] = getTDMLTestCaseItems(tdmlConfigPath)
Review Comment:
If this returns an empty list, we shouldn't even be calling the getTDMLName (or Description) functions. We should be returning empty immediately, and the caller should see that and abort.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2252425076
##########
package.json:
##########
@@ -829,7 +829,6 @@
"variables": {
"AskForSchemaName": "extension.dfdl-debug.getSchemaName",
"AskForDataName": "extension.dfdl-debug.getDataName",
- "AskForTDMLName": "extension.dfdl-debug.getTDMLName",
Review Comment:
No.
You would want to change default to be an empty string, not description, but the PR that I'll be submitting after this one gets in purges the description from the extension, so this change wouldn't matter after that.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2251632947
##########
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
Review Comment:
That's a good point. I'll go ahead and modifying it to take in config.tdmlConfig.path
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Prompt for TDML test case name and description before starting the de… [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on code in PR #1336:
URL: https://github.com/apache/daffodil-vscode/pull/1336#discussion_r2255115470
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -168,35 +168,52 @@ function createDebugRunFileConfigs(
tdmlConfig.action = tdmlAction
if (tdmlAction === 'execute') {
- tdmlConfig.name = '${command:AskForTDMLName}'
- tdmlConfig.description = '${command:AskForTDMLDescription}'
- tdmlConfig.path = targetResource.fsPath
+ tdmlConfig.path = targetResource.fsPath.toString()
+
+ tdmlConfig.name =
+ tdmlConfig.name ||
+ (await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLName',
+ tdmlConfig.path
+ ))
+
+ tdmlConfig.description =
+ tdmlConfig.description ||
+ (await vscode.commands.executeCommand(
+ 'extension.dfdl-debug.getTDMLDescription',
+ tdmlConfig.path
+ ))
+
+ if (!tdmlConfig.name || !tdmlConfig.description) {
+ vscode.window.showErrorMessage(
Review Comment:
That's a good point. It doesn't align with the user pressing Esc on one of the dropdowns. I addressed your point in https://github.com/apache/daffodil-vscode/pull/1336/commits/7f69b20b7b31788c7bcc019f166c3d2ac598e9fb
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org