rioj7 / command-variable

Visual Studio Code extension for variable substitution via ${command:commandID}
54 stars 10 forks source link

bugfix: jsonRepl doesn't correctly default when not present. #93

Closed yyc closed 3 months ago

yyc commented 3 months ago

Seems like this was a regression introduced in 2146b74e83267657ed56889ff8d0adf12f55f4a7

To replicate this bug:

Given services.txt

lmnop
galactus
bingo
rgs

in my launch.json

        {
            "id": "pickService",
            "type": "command",
            "command": "extension.commandvariable.pickStringRemember",
            "args": {
                "key": "service",
                "description": "Select a service to debug",
                "fileName": "${workspaceFolder}/.vscode/debug_options/services.txt",
                "pattern": {
                    "regexp": "^(.+)$", // default
                    "label": "$1" // default
                }
            }
        },

the prompt won't show anything. I think this is because the default value of getProperty(pattern, 'json') here is undefined, and that gets passed around through dataStructSubstitution and convertString until it eventually gets to running this in convertString:

"lmnop".replace(new RegExp("^(.+)$", undefined) // which returns string "undefined"

classic javascript. And on line 963 -965

 if (option.json !== undefined && isString(option.json) && option.json.length > 0) {
              option.value = JSON.parse(option.json);
}

the condition is true, and so we try to JSON.parse("undefined") and get a parse error. I think maybe you'll want to catch JSON SyntaxErrors in some way or handle it gracefully? I'll leave that up to you.

Trying the same config with

                "pattern": {
                    "regexp": "^(.+)$", // default
                    "label": "$1" // default
                    "json": ""
                }

works as expected.

rioj7 commented 3 months ago

@yyc Your PR is a workaround for the actual bug.

The dataStructSubstitution() function was initially not designed for values of undefined.

Fixed in v1.65.3