You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "michael-hoke (via GitHub)" <gi...@apache.org> on 2025/08/08 18:37:00 UTC
[PR] Remove Description from being needed in the TDML config [daffodil-vscode]
michael-hoke opened a new pull request, #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354
Closes #1030
Also removed Description from the launch config, even for generate. It doesn't really make sense to have a field where a user specifies text to go into a file that may be overwritten/deleted. We should expect the users to copy/append the generated TDML file, then they will need to go into that file to create a Zip. At that point, they are able to see and edit the descriptions. Right now, it has to be edited in the XML, however, we will eventually have support for doing that in the GUI.
Removed dead TDML append-related code in the backend and fixed a bug where execute wouldn't work from the TDML GUI.
--
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] Remove Description from being needed in the TDML config [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354#discussion_r2270649368
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -143,12 +144,17 @@ async function createDebugRunFileConfigs(
tdmlAction: string | undefined,
runLast = false
) {
- let targetResource = resource
+ let targetResource: vscode.Uri | undefined = resource
Review Comment:
Can `targetResource` have the type defined in the function parameters as `vscode.Uri | undefined` to be consistent with the `tdmlAction: string | undefined` parameter in this function?
--
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] Remove Description from being needed in the TDML config [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on PR #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354#issuecomment-3179633495
> The reasons you bring up are exactly why we are leaving it in the GUI - we want the user to be able to supply meaningful information about each test case.
>
> As for adding it back into the launch config, there are two reasons that we shouldn't want to do that:
>
> * Names are the only way that the TDML spec states are used to identify a test case. The reason that the description was in there originally was that we were using both.
> * We will be supplying a way for the description to be edited in the GUI. Right now, this is slated for 1.5.0, but it's possible that we bring it in to 1.4.2 depending on timelines. At the moment, a user could switch back to the XML view to modify the description. Although this isn't ideal, we also don't have the main use case (zip) completed yet, so most users aren't going to be wanting to interact with TDML files yet.
Makes sense. Thanks for expanding upon why the TDML Description field was removed from the launch config web view.
Approved the 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] Remove Description from being needed in the TDML config [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354#discussion_r2270656819
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -143,12 +144,17 @@ async function createDebugRunFileConfigs(
tdmlAction: string | undefined,
runLast = false
) {
- let targetResource = resource
+ let targetResource: vscode.Uri | undefined = resource
let noDebug = runOrDebug === 'run'
- if (!targetResource && vscode.window.activeTextEditor) {
- targetResource = vscode.window.activeTextEditor.document.uri
+ if (!targetResource) {
Review Comment:
I would suggest shorthanding this assignment as:
```ts
targetResource = vscode.window.activeTextEditor
? vscode.window.activeTextEditor.document.uri
: TDMLProvider.getDocumentUri()
```
This conditional also has 2 validity checks but what would happen if both conditions are `false`?
--
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] Remove Description from being needed in the TDML config [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell merged PR #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354
--
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] Remove Description from being needed in the TDML config [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on PR #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354#issuecomment-3176395488
<img width="434" height="239" alt="image" src="https://github.com/user-attachments/assets/82855fc3-542e-4694-9c79-f435f4185386" />
<img width="438" height="190" alt="image" src="https://github.com/user-attachments/assets/e07c9ef0-e160-4454-b943-38eada7a1391" />
Confirmed description is gone from the launch config
--
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] Remove Description from being needed in the TDML config [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354#discussion_r2270676955
##########
src/adapter/activateDaffodilDebug.ts:
##########
@@ -143,12 +144,17 @@ async function createDebugRunFileConfigs(
tdmlAction: string | undefined,
runLast = false
) {
- let targetResource = resource
+ let targetResource: vscode.Uri | undefined = resource
Review Comment:
I like the idea of this, but this does seem to open up a can of worms that I would really prefer to avoid in this PR. This call to this function (and, therefore the argument) comes from a function set up with the vscode.commands.registerCommand() function. The VSCode API will automatically provide a Uri as an argument to registered functions. However, it is apparently possible for the Uri to be undefined, which we are not handling in any of these functions.
That being said, I believe that we are likely restricting the use of these commands to cases where the Uri is always going to be defined (file opened and active in window). So, I'd agree that we should be handling the possibility of an undefined Uri, just in case there are holes in how we are exposing the commands to the user, but I think this should be a larger fix than what you've suggested.
--
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] Remove Description from being needed in the TDML config [daffodil-vscode]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on PR #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354#issuecomment-3179612757
The reasons you bring up are exactly why we are leaving it in the GUI - we want the user to be able to supply meaningful information about each test case.
As for adding it back into the launch config, there are two reasons that we shouldn't want to do that:
- Names are the only way that the TDML spec states are used to identify a test case. The reason that the description was in there originally was that we were using both.
- We will be supplying a way for the description to be edited in the GUI. Right now, this is slated for 1.5.0, but it's possible that we bring it in to 1.4.2 depending on timelines. At the moment, a user could switch back to the XML view to modify the description. Although this isn't ideal, we also don't have the main use case (zip) completed yet, so most users aren't going to be wanting to interact with TDML files yet.
--
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] Remove Description from being needed in the TDML config [daffodil-vscode]
Posted by "JeremyYao (via GitHub)" <gi...@apache.org>.
JeremyYao commented on PR #1354:
URL: https://github.com/apache/daffodil-vscode/pull/1354#issuecomment-3179507550
Tested execute TDML on a TDML file w/ 1 test case and 2 test cases using
[sampleWorkspace.zip](https://github.com/user-attachments/files/21737738/sampleWorkspace.zip)
. Worked as expected
--
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