Resolve env vars in settings.json

[mnkiefer]

Feature contribution

• Resolves environment variables in .vscode/settings.json of the form ${env:...}

Use cases

• To be able to enter path for shared config file in vscode-eslint
  Fixes Use of environment variables in options values vscode-eslint#1425
• Many others requested since 2016
  Fixes Support variables when resolving values in settings #2809 (comment)

Implementation

• In src/vs/platform/configuration/common/configuration.ts addToValueTree(), added
• Borrowed logic from src/vs/workbench/services/configurationResolver/common/variableResolver.ts for resolveWithEnvironment() since importing it results in violated imports warning:
  Imports violates 'vs/base/common/** or vs/base/parts/*/common/** or vs/platform/*/common/** ...
• Logic kept identical except for small alterations:
• replaceAsync() replaced by replaceSync() to keep addToValueTree()/ toValueTree() synchronous
• evaluateSingleVariable() simplified to only cover for 'env' case but could be extended

Tests

• addToValueTree: add key with value ${env:...}
  { 'a': 1, 'b': '${env:VALUE_FOR_B} }' => { 'a': 1, 'b': '2' }
• addToValueTree: add key with value string containing ${env:...}
  {'a': 'string1', 'b': 'string${env:VALUE_FOR_B}'} => {'a': 'string1', 'b': 'string2}

[ghost]

All CLA requirements met.

[sandy081]

Thanks for your effort on this contribution. I am sorry that this solution is not an ideal/best because because of following reasons

• This will impact our start up performance
• Env variables are open ended, some times it needs user input
• This breaks existing behaviour
• It is not required in platform

[mnkiefer]

Thanks for your effort on this contribution. I am sorry that this solution is not an ideal/best because because of following reasons

• This will impact our start up performance
• Env variables are open ended, some times it needs user input
• This breaks existing behaviour
• It is not required in platform

@sandy081: Thank you for your reply - could you please elaborate on some of the points you mentioned?

1. Regarding the performance, I'm sure the code could be rewritten in such a way that the effect is minimal, this was only a first draft/suggestion, for example the resolving process could be made async.
2. If type env variables are an issue, maybe supporting command variables (or others) could be an option?
3. Please elucidate on the breaking behaviour you foresee.
4. I'm not sure what the requirements in platform are, but for many extensions, being able to resolve variables in the project settings would be of great help.

In all of the points I'm willing to contribute given the right direction/pointers/test scenarios from your side.
Please advise.

[sandy081]

Regarding the performance, I'm sure the code could be rewritten in such a way that the effect is minimal, this was only a first draft/suggestion, for example the resolving process could be made async.

Any code impacts our start up performance because, configuration service is one of the first services that is being initialised in the very beginning and is sitting in the critical path of start up perf.

If type env variables are an issue, maybe supporting command variables (or others) could be an option?

It is possible but it won't makes the feature complete.

Please elucidate on the breaking behaviour your foresee.

All callers of configuration service now have to stop resolving the variables and also the values before and after might not be same.

I'm not sure what the requirements in platform are, but for many extensions, being able to resolve variables in the project settings would be of great help.

That's true and hence this cannot be implemented in the configuration service.

TBH I do not have very good answer for how to implement this. If you ask me this has to be exposed as an API just like this service - IConfigurationResolverService

CC @alexr00

[alexr00]

Feature request for having this as API: #140056

[mnkiefer]

@sandy081: Thank you for your explanation. I understand now that this feature should be implemented not as part of the configuration service in the platform layer, but as a separate service API in the VS Code workbench?

@alexr00: Would it suffice if I added the kind of resolve functionality described to the ExtHostConfigProvider getConfiguration() method?

[alexr00]

@MJochum thank you for your interest in working on this issue, but we will not accept a PR for #140056 at this time, as I'm not sure that it's the best approach for resolving variables.

[mnkiefer]

@alexr00: No problem, thanks for getting back to me!

Please let me know whenever a feasible solution/approach for this has been decided on in the future (and if help is wanted). 👍