Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support variables when resolving values in settings #2809

Open
OlsonDev opened this issue Feb 9, 2016 · 199 comments
Open

Support variables when resolving values in settings #2809

OlsonDev opened this issue Feb 9, 2016 · 199 comments
Labels
config VS Code configuration, set up issues feature-request Request for new features or functionality
Milestone

Comments

@OlsonDev
Copy link

OlsonDev commented Feb 9, 2016

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
@joaomoreno joaomoreno added the feature-request Request for new features or functionality label Feb 9, 2016
@joaomoreno joaomoreno removed their assignment Feb 9, 2016
@joaomoreno joaomoreno added this to the Backlog milestone Feb 9, 2016
@joaomoreno joaomoreno added the help wanted Issues identified as good community contribution opportunities label Feb 9, 2016
@mjbvz
Copy link
Collaborator

mjbvz commented Mar 4, 2016

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
Copy link
Author

OlsonDev commented Mar 5, 2016

@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
Copy link
Member

bpasero commented Mar 7, 2016

Related: #3759

@bpasero bpasero changed the title settings.json paths don't resolve environment variables Support environment variables when resolving values in settings Mar 7, 2016
@bpasero bpasero removed their assignment Mar 7, 2016
@mjbvz
Copy link
Collaborator

mjbvz commented Mar 7, 2016

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
Copy link
Author

OlsonDev commented Mar 8, 2016

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?

@mjbvz
Copy link
Collaborator

mjbvz commented Mar 8, 2016

Yes, thanks. I've updated the comment with the correct file name.

Resolving the environment variables only when they are actually needed would be best, so that always pick up the latest values.

@bpasero
Copy link
Member

bpasero commented Jul 4, 2016

@DonJayamanne is starting to look into this from #8555

@bpasero bpasero removed the help wanted Issues identified as good community contribution opportunities label Jul 4, 2016
@pcgeek86
Copy link

@bpasero Given the latest comment in #8555, what should the expectation be for having the ability to specify environment variables within user and workspace JSON configurations?

@DonJayamanne
Copy link
Contributor

@pcgeek86, this capability hasn't been added yet.
The PR you mentioned was closed without merging the changes due to some other ongoing changes mentioned by @bpasero in his last comment

@bpasero
Copy link
Member

bpasero commented Aug 31, 2016

@DonJayamanne for your case (Python I think?), did you find a workaround to support this?

If we add variables support to settings, we need to think about validation. If a setting only allows for certain string values, putting in a variable would always result in warnings. I guess this problem already exists today for launch.json and task.json.

@DonJayamanne
Copy link
Contributor

@bpasero, yes for Python extension i do have a work around, with support for $workspaceRoot and $env.. (env variables) in settings.json.
Here's an example:

"python.pythonPath":"${workspaceRoot}/env/bin/python"

However, there's one problem.
The debugger can now reference settings defined in settings.json (via the following PR using the syntax ${config.python.pythonPath}).
Unfortunately the variable ${workspaceRoot} doesn't get resolved in the debugger. I cannot implement a work around either as the workspace Root path isn't available in the debugger either.

@DonJayamanne
Copy link
Contributor

@bpasero , upon further analysis, I believe the bug is in configVariables.
The problem is the debugger is getting the resolved value from settings.json, however the resolved value needs to be resolved once again to get the tokens (such as ${workspaceRoot}) replaced. I'll raise a separate issue for that.

@jcready
Copy link

jcready commented Nov 17, 2023

The weird thing is that ${workspaceFolder} and ${workspaceFolderBasename} work.

This doesn't work for dbaeumer.vscode-eslint (which, despite its name is actually maintained by Microsoft) when inside our workspace file settings:

"eslint.options": {
    "configFile": "${workspaceFolder}/eslint/eslintrc.js",
},

The extension just throws an error like:

Cannot find module /home/me/repo/${workspaceFolder}/eslint/eslintrc.js

And of course the maintainers of that extension (Microsoft, the same maintainers of vscode) claim this is an issue with vscode: microsoft/vscode-eslint#1231

I don't care which Microsoft fixes the issue, but please do so.

@GitMensch
Copy link
Contributor

I do care - the "vscode Microsoft", because when @dbaeumer "fixes" (actually: implements) that in the eslint extension, then this only works there, while a solution in vscode can fix this for every extension.

@OreoLamp
Copy link

This is still sorely needed, I need to be able to point vscode to the current JAVA_HOME environment variable in settings.json in multiple places. I use multiple different versions of java in different projects and the environment variable needs to update with them, which at the moment is just not possible without external tools.

@Zemnmez
Copy link
Contributor

Zemnmez commented Jan 7, 2024

I'm surprised given the language in https://code.visualstudio.com/docs/editor/variables-reference#_is-variable-substitution-supported-in-user-and-workspace-settings The predefined variables are supported in a select number of setting keys in settings.json files such as the terminal cwd, env, shell and shellArgs values. Some settings like window.title have their own variables: that it's not supported directly by vscode.

@stellarpower
Copy link

It's not uncommon for me to open the same folder, once inside a development container, and once on the root of my machine. The way settings are shared between the two is a benefit and a curse; being able at least to access environment variables in the same way as I can in the launch JSON would help a lot in terms of managing configs.

@47rooks
Copy link

47rooks commented Feb 15, 2024

I too would like to see config variables usable in settings.json and in *.code-workspace for multi-root projects. I think they should be resolved transparently in variables owned by plugins.

@g-berthiaume
Copy link

g-berthiaume commented Feb 19, 2024

+1

I personally want to use the $userHome.
If anybody has a workaround for that, let me know.

@romkell
Copy link

romkell commented Feb 21, 2024

+1

Wanted to substitute ${env:<some-env-var>}.

@geekley
Copy link

geekley commented Mar 4, 2024

Is there any technical reason that would prevent doing it?

Probably the fact each extension can have it's own custom settings with its own format, and you can't just easily force this syntax globally, so each extension would need to be prepared to deal with it somehow, individually.

Unless, of course you used something like, e.g.:

"some.setting": "value is passed verbatim, including literally ${env:example}",
"some.setting$": "value substitutes ${env:example} variables generically"

@GitMensch
Copy link
Contributor

GitMensch commented Mar 4, 2024

you can't just easily force this syntax globally, so each extension would need to be prepared to deal with it somehow, individually

I disagree - if people use the official namespaces and variables like ${env:..}, ${workspaceFolder:..}, ${config:..}, ${userhome}, then:

  • users expect to have them working as in the vscode documentation
  • maintainers currently try to resolve them on their own to try to meet this expectation which should have been declared as vscode's responsibility in the first place

That would be no problem as after a change the variables will be auto-resolved before the extension code "sees" it, so they won't resolve (commonly only partial) as before and can drop code for that later.
The same extension used with an older vscode version will "see" the unresolved variables and resolve it (or not) on its own.
"Special" variables with different namepaces/varnames will be passed unchanged, so the extension still has the option to resolve as it sees fit.

Therefore:

  • no adjustments in extensions is needed "up front" (with the sole exception that if they "break" the kind-of contract and used predefined names for something else)
  • a change will globally enable every extension out there to resolve the predefined names (which are documented since years, which is one of the reasons users expect this to "just work") by just reading its setting, not only "some selected settings" but "every setting for every extension"

@romkell
Copy link

romkell commented Mar 4, 2024

Therefore:

* no adjustments in extensions is needed "up front" (with the sole exception that if they "break" the kind-of contract and used predefined names for something else)

In that case, for backward compatibility, the settings.json could define an self consumed (which by default is "true")

{
    "settingsJson.variableExpand.enable": "false"
    
    "some.more.settings": "value"
}

which disables the common variable expansion on settings.json level before passing to the extensions.
That mechanism could even be extended to work with a validity scope of "between {}", hence on several levels, to be usable for different extensions independently.

Not exactly the most beautiful mechanism in general, but would only have be used where variable names clash.

@davidolrik
Copy link

Not exactly the most beautiful mechanism in general, but would only have be used where variable names clash.

If extension variable names clash with any of the official namespaces and variables, the extension should break and the extention author should fix their use to be in line with the documentation.

@geekley
Copy link

geekley commented Mar 5, 2024

no adjustments in extensions is needed "up front" (with the sole exception that if they "break" the kind-of contract and used predefined names for something else)

the extension should break and the extension author should fix their use to be in line with the documentation.

No. Current documentation says it's supported in "some select settings" not that it's meant to be supported in every setting. If this changes, it's VSCode that's breaking stuff, and you can't just carelessly break extensions like this.
That's exactly why I said you can't force it globally easily, pretty sure a bunch of extensions is already using the ${...} syntax for whichever purpose they want, which may or may not clash. Every author will need to opt-in to the changes somehow, because VSCode never wants to break any compatibility.

However, I do agree with the points made that most would likely transition smoothly.

So I guess the best course of action would be to expect every extension to be able to support this, but to avoid compatibility breakage, it would need just a different API for the extension to get the setting.
So IMO the best would be to deprecate vscode.workspace.getConfiguration(section, scope) in favor of either:

  • a different overload like getConfiguration(section, scope, substituteVars = false), or
  • preferably a different method name like getConfig(section, scope, substituteVars = true) for simplicity (i.e., a default of true for the parameter as an additional push to make them avoid the deprecated method and to not require more complex code)

@romkell
Copy link

romkell commented Mar 5, 2024

Not exactly the most beautiful mechanism in general, but would only have be used where variable names clash.

If extension variable names clash with any of the official namespaces and variables, the extension should break and the extention author should fix their use to be in line with the documentation.

Agree. This is the clean way. Suggestion was just to show a backward compat. mode/mechansim to allow a transition.

@romkell
Copy link

romkell commented Mar 5, 2024

So IMO the best would be to deprecate vscode.workspace.getConfiguration(section, scope) in favor of either:

* a different overload like `getConfiguration(section, scope, substituteVars = false)`, or

* preferably a different method name like `getConfig(section, scope, substituteVars = true)` for simplicity (i.e., a default of true for the parameter as an additional push to make them avoid the deprecated method and to not require more complex code)

A new API is a good solution, if the old API will be removed within a reasonable time frame, to make the transition happen.
With that respect I see substituteVars = false|true in the new API as very critical. If there is a choice for the extensions in the future, some will use it this way, others the other way. Not something a user wants to try and error for each extension.
The users would have to do the transition too and adjust their settings.json's from e.g. ${workspaceFolder} to
${extensionA.workspaceFolder} or similar.

@GitMensch
Copy link
Contributor

Every author will need to opt-in to the changes somehow, because VSCode never wants to break any compatibility.

I disagree.

For the second point: note that several extensions that weren't updated do not work since several versions because there is a compatibility break if you don't update some dependencies to newer versions, if you do that as the extension author it leads to the need to adjust the code to cater for changed ABI.

For the first point: that will lead to a lot of extensions not getting the expected result (also: there are a bunch of extensions that aren't changed any more, we should not create an unneeded reason for those to not work in the future, which your adjusted ABI effectively means).

Not changing the ABI is definitely better, as noted nothing will break as long as:

  • the translation leaves unknown ${...} as-is
  • extension authors did not use the documented variables in a way that is different to the documented result

@AndreiZibrov
Copy link

AndreiZibrov commented Jun 21, 2024

nor ${workspaceFolderBasename} nor ${workspaceRootFolderName} still are not resolving in settings.json on latest vscode 1.90.1

Instead of resolved value the "${workspaceFolderBasename}" appears in the logs of any extension (in my case clangd)

And for example, the ${workspaceFolder} from the same group just works.
The difference between them is only the path.basename(workspaceFolder) call or there is something else?
image

These variables have been introduced around 2019.

Is this proper thread for this information or yet another issue is required for it?
Please share which information could be helpful to resolve it?

Version: 1.90.1
Commit: 611f9bf
Date: 2024-06-11T21:01:24.262Z
Electron: 29.4.0
ElectronBuildId: 9593362
Chromium: 122.0.6261.156
Node.js: 20.9.0
V8: 12.2.281.27-electron.0
OS: Windows_NT x64 10.0.22631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config VS Code configuration, set up issues feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.