You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "shanedell (via GitHub)" <gi...@apache.org> on 2024/01/03 20:03:33 UTC
[PR] Ensure infoset file extension match output format. [daffodil-vscode]
shanedell opened a new pull request, #927:
URL: https://github.com/apache/daffodil-vscode/pull/927
Ensure infoset file extension match output format.
- Done by checking if the filepath does not end with the proper file extension. If it doesn't then:
- If the path ends with a different extension it is replaced with the extension of the output format.
- If the path does not end with a different extension then the extension for the output format is added to the end of the file path.
Closes #845
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927#discussion_r1445529018
##########
src/utils.ts:
##########
@@ -79,6 +80,30 @@ export async function onDebugStartDisplay(viewsToCheck: string[]) {
})
}
+function checkInfosetFileExtension(
+ infosetOutput: InfosetOutput,
+ infosetFormat: string
+) {
+ // If the infoset file path doesn't end with the infoset output format, update to end with infoset output format.
+ if (!infosetOutput.path.endsWith(`.${infosetFormat}`)) {
Review Comment:
Concern: what happens if `infosetOutput.type` is not `file`, and `path` doesn't exist? Can there be a test for this case?
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927#discussion_r1443390534
##########
src/utils.ts:
##########
@@ -79,6 +80,30 @@ export async function onDebugStartDisplay(viewsToCheck: string[]) {
})
}
+function checkInfosetFileExtension(
+ infosetOutput: InfosetOutput,
+ infosetFormat: string
+) {
+ // If the infoset file path doesn't end with the infoset output format, update to end with infoset output format.
+ if (!infosetOutput.path.endsWith(`.${infosetFormat}`)) {
Review Comment:
So this is the only place I believe is filtering off a regex. Other places are using `.exec` and some use `.test`. These perform different as `exec` give you a result and `test` just tells you true or false. I mean we could make wrappers around these but not sure how useful that is as you would just be calling `WrapperFunction` each time instead of `exec` or `test`. We could also make a call back function but that would also kind of just achieve the same thing.
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927#discussion_r1443342302
##########
src/utils.ts:
##########
@@ -79,6 +80,30 @@ export async function onDebugStartDisplay(viewsToCheck: string[]) {
})
}
+function checkInfosetFileExtension(
+ infosetOutput: InfosetOutput,
+ infosetFormat: string
+) {
+ // If the infoset file path doesn't end with the infoset output format, update to end with infoset output format.
+ if (!infosetOutput.path.endsWith(`.${infosetFormat}`)) {
Review Comment:
It looks like there's a couple of other instances where `RegExp` objects are being tested / exec'ed. Exporting this to a function might be helpful for readability and future us.
```ts
let fileExtensionSearchResult = regexTestAndFilter( . . . )
```
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927#discussion_r1445541451
##########
src/utils.ts:
##########
@@ -79,6 +80,30 @@ export async function onDebugStartDisplay(viewsToCheck: string[]) {
})
}
+function checkInfosetFileExtension(
+ infosetOutput: InfosetOutput,
+ infosetFormat: string
+) {
+ // If the infoset file path doesn't end with the infoset output format, update to end with infoset output format.
+ if (!infosetOutput.path.endsWith(`.${infosetFormat}`)) {
Review Comment:
I will add a check to only call `checkInfosetFileExtension` if the `infosetOutput.type == file`. But as for the path not existing that already gets handled and the directory is automatically created when we are creating the output file.
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927#discussion_r1443342883
##########
src/utils.ts:
##########
@@ -79,6 +80,30 @@ export async function onDebugStartDisplay(viewsToCheck: string[]) {
})
}
+function checkInfosetFileExtension(
+ infosetOutput: InfosetOutput,
+ infosetFormat: string
+) {
+ // If the infoset file path doesn't end with the infoset output format, update to end with infoset output format.
+ if (!infosetOutput.path.endsWith(`.${infosetFormat}`)) {
+ let fileExtensionSearchResult = new RegExp('(\\.).*$')
+ .exec(infosetOutput!.path)
+ ?.filter((fileExt) => fileExt !== '.')[0]
+
+ /**
+ * If search result is not undefined replace the file extension with the correct extension.
+ * Else append proper file extension to the end of the output path.
+ */
+ infosetOutput.path =
Review Comment:
_Same as the above_ [comment.](https://github.com/apache/daffodil-vscode/pull/927/files#r1443342302)
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell merged PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927#issuecomment-1887708385
@arosien I added a check to make sure we are only checking the infoset's file extension when the output type is file. I also added a warning message that will let the user know they don't have the proper file extension in the output filepath and that it will updated to automatically have the correct file extension.
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927#discussion_r1449228262
##########
src/utils.ts:
##########
@@ -79,6 +80,30 @@ export async function onDebugStartDisplay(viewsToCheck: string[]) {
})
}
+function checkInfosetFileExtension(
+ infosetOutput: InfosetOutput,
+ infosetFormat: string
+) {
+ // If the infoset file path doesn't end with the infoset output format, update to end with infoset output format.
+ if (!infosetOutput.path.endsWith(`.${infosetFormat}`)) {
Review Comment:
@stricklandrbls I can't really add this change as what I am doing with this regexp is very different from others amd I think adding a function like this would basically involve passing the RegExp and then running the `.test` or `.exec` function which I don't think is completely necessary.
--
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] Ensure infoset file extension match output format. [daffodil-vscode]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #927:
URL: https://github.com/apache/daffodil-vscode/pull/927#discussion_r1445528676
##########
src/utils.ts:
##########
@@ -79,6 +80,30 @@ export async function onDebugStartDisplay(viewsToCheck: string[]) {
})
}
+function checkInfosetFileExtension(
+ infosetOutput: InfosetOutput,
+ infosetFormat: string
+) {
+ // If the infoset file path doesn't end with the infoset output format, update to end with infoset output format.
+ if (!infosetOutput.path.endsWith(`.${infosetFormat}`)) {
Review Comment:
Concern: I don't think we should silently change the output file path without notifying the user. Perhaps we can log something or show a popup with a warning?
--
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