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

["mbeckerle (via GitHub)" <gi...@apache.org>]

mbeckerle commented on code in PR #625:
URL: https://github.com/apache/daffodil-vscode/pull/625#discussion_r1187619693


##########
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? 
   
   Also, what is a tdmlName - there are too many kinds of names - the TDML file's name, the testSuite name, the test itself name,... so use a longer more explicit identifier. 
   
   Watch out also for "config", as there is a testSuite defaultConfig and each test has a config also, and a testSuite can have multiple named defineConfig children. 
   
   Suggest name "tdmlVSCodeConfig" would keep things clearer. 
   
   Ideally, this should prompt you for a TDML file name, and then store it here, i.e., be sticky about the selection. 



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