Skip to content

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

Closed
yyc wants to merge 2 commits intorioj7:masterfrom
yyc:patch-1
Closed

bugfix: jsonRepl doesn't correctly default when not present.#93
yyc wants to merge 2 commits intorioj7:masterfrom
yyc:patch-1

Conversation

@yyc
Copy link

@yyc yyc commented Aug 30, 2024

Seems like this was a regression introduced in 2146b74

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.

@yyc yyc changed the title Fixes a bug: jsonRepl doesn't correctly default when not present. bugfix: jsonRepl doesn't correctly default when not present. Aug 30, 2024
@rioj7 rioj7 closed this in 6453d62 Aug 30, 2024
@rioj7
Copy link
Owner

rioj7 commented Aug 30, 2024

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants