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]

Reply via email to