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 for ${workspaceFolder} and other variables in config #1245

Closed
wants to merge 1 commit into from

Conversation

zephraph
Copy link

@zephraph zephraph commented Jan 25, 2025

Context

In some contexts of VSCode configuration there are a list of variables that can be substituted for values. Unfortunately VSCode doesn't support substituting those in user extensions out of the box.

Other extensions end up implementing full variable resolution themselves.

I've added a function that should be fairly easy to expand in coverage which starts adding support for these variable substitutions. Right now it's only being applied to deno.path but could be expanded to other config properties.

Motivation

I'm using mise to manage Deno versions. There's a mise vscode extension that allows you to use symlinks in the .vscode directory of your repo to link all your tools. This is helpful if you're sharing vscode configuration with a team. To accomplish this, it saves the paths using workspaceRoot.

Here's an example of the config in vsocde that it generates:

  "python.defaultInterpreterPath": "${workspaceFolder}/.vscode/mise-tools/python",
  "debug.javascript.defaultRuntimeExecutable": {
    "pwa-node": "${workspaceFolder}/.vscode/mise-tools/node"
  },
  "ruff.path": [
    "${workspaceFolder}/.vscode/mise-tools/ruff"
  ],
  "deno.path": "${workspaceFolder}/.vscode/mise-tools/deno",
  "mise.configureExtensionsUseSymLinks": true,

Note that deno.path uses ${workspaceFolder}. While it works for other extensions (python, ruff, node) it was failing for deno. At first I thought this extension just wasn't playing well with the symlinks (b/c I naively thought the path resolution for configuration was being handled automatically by vscode). Digging deeper, I realized that wasn't the case. This PR will resolve the issues I encountered and allow folks to use other variable substitutions for paths.

Areas of improvement

  • I really want to write tests for this, but there's little in the way of testing infra in the repo. I didn't want to set up something new because that felt outside the scope of this change.
  • If there's a variable that's unsupported, we should likely show an error or warning to users to indicate such.

@CLAassistant
Copy link

CLAassistant commented Jan 25, 2025

CLA assistant check
All committers have signed the CLA.

@zephraph zephraph force-pushed the support-workspace-folder branch 2 times, most recently from 90bfb74 to 0b746e2 Compare January 25, 2025 15:55
@nayeemrmn
Copy link
Collaborator

Most settings are processed server-side at https://github.com/denoland/deno/tree/v2.1.7/cli/lsp.

We're likely to let this be an upstream issue for vscode. One reason this wouldn't work otherwise is if you specify ${workspaceFolder} in your workspace-scoped settings, one would expect it to create different values for each workspace-folder-scope it's queried for. That needs first-class support since the language server talks to vscode directly when querying config.

Thanks for opening this PR but we're going to leave this for vscode.

@nayeemrmn nayeemrmn closed this Jan 26, 2025
@zephraph
Copy link
Author

zephraph commented Jan 27, 2025

It's unlikely support for this will ever land upstream. The original issue for this was opened in 2016. I certainly empathize with the complexity around multi-workspace setups. This is a best effort implementation. The ${workspaceFolder:<name>} semantics would allow for specifying specific folders in the workspace.

I'm willing to work through a more robust implementation if there's appetite for it.

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.

3 participants