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/05 19:59:30 UTC
[PR] Rename setting program to schema [daffodil-vscode]
shanedell opened a new pull request, #929:
URL: https://github.com/apache/daffodil-vscode/pull/929
Rename setting program to schema
Closes #928
--
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] Rename setting program to schema [daffodil-vscode]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #929:
URL: https://github.com/apache/daffodil-vscode/pull/929#discussion_r1443377147
##########
src/launchWizard/launchWizard.js:
##########
@@ -64,7 +64,7 @@ function getConfigValues() {
'openInfosetDiffView'
).checked
const openInfosetView = document.getElementById('openInfosetView').checked
- const program = document.getElementById('program').value
+ const schema = document.getElementById('schema').value
Review Comment:
Hmmm. Can you help me understand why we have both a launchWizard.js and a launchWizard.ts file? I thought ts was the VSCode typed flavor of javascript, and that its use subsumed javascript inside VSCode.
My understanding is obviously superficial. Can you help me understand why we need both in this situation?
--
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] Rename setting program to schema [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #929:
URL: https://github.com/apache/daffodil-vscode/pull/929#discussion_r1443567141
##########
src/launchWizard/launchWizard.js:
##########
@@ -64,7 +64,7 @@ function getConfigValues() {
'openInfosetDiffView'
).checked
const openInfosetView = document.getElementById('openInfosetView').checked
- const program = document.getElementById('program').value
+ const schema = document.getElementById('schema').value
Review Comment:
I also I believe the reason it touched so much is because we based this on [vscode-mock-debug](https://github.com/microsoft/vscode-mock-debug). We mostly followed what they did which I believe starts with a `program` setting. So I believe that was where most of the references came from.
--
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] Rename setting program to schema [daffodil-vscode]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on PR #929:
URL: https://github.com/apache/daffodil-vscode/pull/929#issuecomment-1888098430
@shanedell @mbeckerle FYI there is a mechanism to deprecate configuration settings, which emits a warning, although the deprecated property needs to be supported. See https://code.visualstudio.com/api/references/contribution-points#Configuration-property-schema under "deprecationMessage".
It would look like this in our usage:
--
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] Rename setting program to schema [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell merged PR #929:
URL: https://github.com/apache/daffodil-vscode/pull/929
--
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] Rename setting program to schema [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #929:
URL: https://github.com/apache/daffodil-vscode/pull/929#discussion_r1443567141
##########
src/launchWizard/launchWizard.js:
##########
@@ -64,7 +64,7 @@ function getConfigValues() {
'openInfosetDiffView'
).checked
const openInfosetView = document.getElementById('openInfosetView').checked
- const program = document.getElementById('program').value
+ const schema = document.getElementById('schema').value
Review Comment:
I also I believe the reason it touched so much is because we initially based this repository on [vscode-mock-debug](https://github.com/microsoft/vscode-mock-debug). We mostly followed what they did which I believe starts with a `program` setting. So I believe that was where most of the references came from.
--
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] Rename setting program to schema [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on PR #929:
URL: https://github.com/apache/daffodil-vscode/pull/929#issuecomment-1879314446
> +1
>
>
>
> This is obviously correct/ok.
>
>
>
> I have to say, the fact that this change required 14 files to be edited was a bit shocking.
>
>
>
> Somehow I feel an opportunity to generate a bunch of artifacts from a common spec has been missed. Maybe not, but that was my visceral reaction.
>
>
>
>
Yes, so the reason for the TS and JS files is because the JS file directly interacts with the DOM/UI of the WebView. For the scripting of WebViews it only directly support JS and not TS, so we use JS to interact with UI and then forward that information to the TS. We also use JS to update the UI based on information sent back to the JS from the TS. So, overall JS is used mostly for DOM and UI interaction and the TS does everything else but they communicate back and forth with each other using the VSCode API. The JS version of that API in this case has a lot less capabilities than the VSCode in the TS and that is why we don't use only JS for this as well.
Does that help or make it more confusing?
--
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] Rename setting program to schema [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #929:
URL: https://github.com/apache/daffodil-vscode/pull/929#discussion_r1443565989
##########
src/launchWizard/launchWizard.js:
##########
@@ -64,7 +64,7 @@ function getConfigValues() {
'openInfosetDiffView'
).checked
const openInfosetView = document.getElementById('openInfosetView').checked
- const program = document.getElementById('program').value
+ const schema = document.getElementById('schema').value
Review Comment:
@mbeckerle Yes, so the reason for the TS and JS files is because the JS file directly interacts with the DOM/UI of the WebView. For the scripting of WebViews it only directly support JS and not TS, so we use JS to interact with UI and then forward that information to the TS. We also use JS to update the UI based on information sent back to the JS from the TS. So, overall JS is used mostly for DOM and UI interaction and the TS does everything else but they communicate back and forth with each other using the VSCode API. The JS version of that API in this case has a lot less capabilities than the VSCode API in the TS and that is why we don't use only JS for this as well.
Does that help or make it more confusing?
--
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] Rename setting program to schema [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on code in PR #929:
URL: https://github.com/apache/daffodil-vscode/pull/929#discussion_r1443565989
##########
src/launchWizard/launchWizard.js:
##########
@@ -64,7 +64,7 @@ function getConfigValues() {
'openInfosetDiffView'
).checked
const openInfosetView = document.getElementById('openInfosetView').checked
- const program = document.getElementById('program').value
+ const schema = document.getElementById('schema').value
Review Comment:
@mbeckerle Yes, so the reason for the TS and JS files is because the JS file directly interacts with the DOM/UI of the WebView. For the scripting of WebViews it only directly support JS and not TS, so we use JS to interact with UI and then forward that information to the TS. We also use JS to update the UI based on information sent back to the JS from the TS. So, overall JS is used mostly for DOM and UI interaction and the TS does everything else but they communicate back and forth with each other using the VSCode API. The JS version of that API in this case has a lot less capabilities than the VSCode API in the TS and that is why we don't use only JS for this as well.
Did that help?
--
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