[GitHub] [daffodil-vscode] michael-hoke commented on a diff in pull request #625: Add conditional defaults to TDML properties

["michael-hoke (via GitHub)" <gi...@apache.org>]

michael-hoke commented on code in PR #625:
URL: https://github.com/apache/daffodil-vscode/pull/625#discussion_r1187634611


##########
src/utils.ts:
##########
@@ -120,12 +130,16 @@ export function getConfig(
     tdmlConfig: tdmlConfig
       ? tdmlConfig
       : {
-          action: defaultConf.get('tdmlAction', 'none'),
-          name: defaultConf.get('tdmlName', '${command:AskforTDMLName}'),
-          description: defaultConf.get(
-            'tdmlDescription',
-            '${command:AskForTDMLDescription}'
-          ),
+          action: tdmlAction,
+          name: tdmlPropsNeeded
+            ? defaultConf.get('tdmlName', '${command:AskforTDMLName}')
+            : defaultConf.get('tdmlName', 'tdmlNamePlaceholder}'),

Review Comment:
   > Hmmm. Why create a different text placeholder string?
   > 
   The root issue of #622 is that VSCode is trying to resolve ${command:AskforTDMLName} even in cases where it shouldn't, i.e. when a user runs a launch config with TDMLAction set to 'none'. The idea of this placeholder is to make it so that configs that are created won't use the command as the default value, which would require users to modify the default launch config to make it work for the default use case. On the other hand, if we run into a config with a TDMLAction that isn't none, we want the command to be the default value.
   
   At best, this only reduces user impact for #622, so we're still looking into ways to fix it.
   
   Thanks for the other suggestions - I don't want to try to overcrowd 1.3.0, but these will be helpful when we look to improve upon the TDML functionality in upcoming releases.
   



-- 
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