Support variables when resolving values in settings

[OlsonDev]

Hi,

I was just reading the latest updates and it says one can install typescript@next globally and then set typescript.tsdk so VS Code can use the appropriate version/installation. In a team environment, I'd like to put that setting in our project, something like:

.vscode/settings.json:

{
    "typescript.tsdk": "%APPDATA%/npm/node_modules/typescript/lib"
}

The problem is restarting VS Code results in an error:

The path c:\Projects\Derp\%APPDATA%\npm\node_modules\typescript\lib doesn't point to a valid tsserver install. TypeScript language features will be disabled.

Now the setting needs to be per-person because I highly doubt my teammates have tsc installed in C:\Users\Olson\AppData\Roaming\npm\node_modules\typescript\lib 😉

Could we get environment variables evaluated on that and all other settings that involve paths?

I haven't tested other paths, but I see these in the Default Settings:

• git.path
• markdown.styles
• json.schemas
• typescript.tsdk
• php.validate.executablePath

[mjbvz]

I was looking into this a little, but it'd be helpful to understand what the behavior should look like on non windows systems. Mainly, should we use the same environment variable %SYNTAX% or try to emulate the host platform?

I'd argue that having VS Code be consistent across multiple systems is more important than constancy with the host platform, but I'm curious to see if anyone has any objections to that. We could use $SYNTAX or something else, across all platforms too.

[OlsonDev]

@mjbvz I thought about this when I first opened the issue but neglected to mention it because I'm indifferent. However, I do agree consistency within VS Code is likely more beneficial than with the OS.

Visual Studio seems to use $(PATH) syntax. I think that would make the implementation easier because, ya know, having an end delimiter tells you exactly what to look up instead of having to filter down Object.keys(process.env) one character at a time.

I guess my vote goes toward $(PATH) syntax for consistency within Microsoft IDEs. 👍

[bpasero]

Related: #3759

[mjbvz]

Just looking through the code, this best approach seems to be using the existing SystemVariables (or extracting its functionality) class since SystemVariables::resolve takes a string and substitutes in environment variables for any ${VAR} occurrences (see AbstractSystemVariables::__resolveString).

Another approach may be to update newConfigFile() from platform/configuration/model.ts to resolve the environment variables when setting the config values, but this would not pick up changes to environment variables after the config is saved.

[OlsonDev]

Another approach may be to update newConfigFile from platform/configuration/common.ts to resolve the environment variables when setting the config values.

You mean platform/configuration/common/model.ts, I assume?