mbeckerle commented on code in PR #704:
URL: https://github.com/apache/daffodil-vscode/pull/704#discussion_r1265388259
##########
src/launchWizard/launchWizard.js:
##########
@@ -298,15 +322,22 @@ async function updateConfigValues(config) {
document.getElementById('useExistingServer').checked =
config.useExistingServer
document.getElementById('dataEditorPort').value = parseInt(
- config.dataEditorPort
+ config.dataEditor.port
)
- document.getElementById('dataEditorLogFile').value = config.dataEditorLogFile
+ document.getElementById('dataEditorLogFile').value =
+ config.dataEditor.logging.file
document.getElementById('dataEditorLogLevel').value =
- config.dataEditorLogLevel
+ config.dataEditor.logging.level
+
+ document.getElementById('dfdlDebuggerLogFile').value =
+ config.dfdlDebugger.file
+ document.getElementById('dfdlDebuggerLogLevel').value =
+ config.dfdlDebugger.level
updateInfosetOutputType()
updateTDMLAction()
+ await clearClasspathList()
Review Comment:
Add comments explaining what you are doing here.
It looks like you are clearing the table of debug classpath entries,
then if the config.daffodilDebugClasspath, which is a string, is Non empty,
you are reinitializing the table of classpath entries from it? But why? In what
situation would this be necessary?
The relationship of the classpath, concatenated into a string with ":"
separators, and this daffodilDebugClasspathTable is not clear. Which takes
priority over the other?
You could rename some functions to make it clear these are all talking about
the debugClasspath, and you currently have clear and update, when really I
think you have only one public operation: set the table to match the current
stringified debug classpath, which involves clearing the table then setting it
to match the string.
But I'm not even sure why you are doing that. The user fills in the table
from the UI.
Is this hedging so that if they edit the launch.json themselves and update
the daffodilDebugClasspath that you can detect that and use it as the starting
point?
If so, please say so explicitly. Also, is it possible for the user to modify
the launch.json debug classpath table also, in which case those changes would
be lost. Or is that not visible to 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]