Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #19634 from Veykril/push-mnpmxxrprymo
feat: Allow unsetting env vars in `server.extraEnv` config
| -rw-r--r-- | editors/code/package.json | 3 | ||||
| -rw-r--r-- | editors/code/src/bootstrap.ts | 10 | ||||
| -rw-r--r-- | editors/code/src/config.ts | 34 | ||||
| -rw-r--r-- | editors/code/src/ctx.ts | 9 | ||||
| -rw-r--r-- | editors/code/src/util.ts | 2 |
5 files changed, 38 insertions, 20 deletions
diff --git a/editors/code/package.json b/editors/code/package.json index b05be450cb..a282eea999 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -543,7 +543,8 @@ "additionalProperties": { "type": [ "string", - "number" + "number", + "null" ] }, "default": null, diff --git a/editors/code/src/bootstrap.ts b/editors/code/src/bootstrap.ts index c63c6f2f7c..bddf195803 100644 --- a/editors/code/src/bootstrap.ts +++ b/editors/code/src/bootstrap.ts @@ -187,8 +187,16 @@ async function hasToolchainFileWithRaDeclared(uri: vscode.Uri): Promise<boolean> export async function isValidExecutable(path: string, extraEnv: Env): Promise<boolean> { log.debug("Checking availability of a binary at", path); + const newEnv = { ...process.env }; + for (const [k, v] of Object.entries(extraEnv)) { + if (v) { + newEnv[k] = v; + } else if (k in newEnv) { + delete newEnv[k]; + } + } const res = await spawnAsync(path, ["--version"], { - env: { ...process.env, ...extraEnv }, + env: newEnv, }); if (res.error) { diff --git a/editors/code/src/config.ts b/editors/code/src/config.ts index ba1c3b01de..f36e18a73d 100644 --- a/editors/code/src/config.ts +++ b/editors/code/src/config.ts @@ -213,12 +213,13 @@ export class Config { get serverExtraEnv(): Env { const extraEnv = - this.get<{ [key: string]: string | number } | null>("server.extraEnv") ?? {}; + this.get<{ [key: string]: { toString(): string } | null } | null>("server.extraEnv") ?? + {}; return substituteVariablesInEnv( Object.fromEntries( Object.entries(extraEnv).map(([k, v]) => [ k, - typeof v !== "string" ? v.toString() : v, + typeof v === "string" ? v : v?.toString(), ]), ), ); @@ -398,6 +399,7 @@ export function prepareVSCodeConfig<T>(resp: T): T { // FIXME: Merge this with `substituteVSCodeVariables` above export function substituteVariablesInEnv(env: Env): Env { + const depRe = new RegExp(/\${(?<depName>.+?)}/g); const missingDeps = new Set<string>(); // vscode uses `env:ENV_NAME` for env vars resolution, and it's easier // to follow the same convention for our dependency tracking @@ -405,15 +407,16 @@ export function substituteVariablesInEnv(env: Env): Env { const envWithDeps = Object.fromEntries( Object.entries(env).map(([key, value]) => { const deps = new Set<string>(); - const depRe = new RegExp(/\${(?<depName>.+?)}/g); - let match = undefined; - while ((match = depRe.exec(value))) { - const depName = unwrapUndefinable(match.groups?.["depName"]); - deps.add(depName); - // `depName` at this point can have a form of `expression` or - // `prefix:expression` - if (!definedEnvKeys.has(depName)) { - missingDeps.add(depName); + if (value) { + let match = undefined; + while ((match = depRe.exec(value))) { + const depName = unwrapUndefinable(match.groups?.["depName"]); + deps.add(depName); + // `depName` at this point can have a form of `expression` or + // `prefix:expression` + if (!definedEnvKeys.has(depName)) { + missingDeps.add(depName); + } } } return [`env:${key}`, { deps: [...deps], value }]; @@ -454,11 +457,10 @@ export function substituteVariablesInEnv(env: Env): Env { do { leftToResolveSize = toResolve.size; for (const key of toResolve) { - const item = unwrapUndefinable(envWithDeps[key]); - if (item.deps.every((dep) => resolved.has(dep))) { - item.value = item.value.replace(/\${(?<depName>.+?)}/g, (_wholeMatch, depName) => { - const item = unwrapUndefinable(envWithDeps[depName]); - return item.value; + const item = envWithDeps[key]; + if (item && item.deps.every((dep) => resolved.has(dep))) { + item.value = item.value?.replace(/\${(?<depName>.+?)}/g, (_wholeMatch, depName) => { + return envWithDeps[depName]?.value ?? ""; }); resolved.add(key); toResolve.delete(key); diff --git a/editors/code/src/ctx.ts b/editors/code/src/ctx.ts index 1149523622..e55754fb9f 100644 --- a/editors/code/src/ctx.ts +++ b/editors/code/src/ctx.ts @@ -213,7 +213,14 @@ export class Ctx implements RustAnalyzerExtensionApi { this.refreshServerStatus(); }, ); - const newEnv = Object.assign({}, process.env, this.config.serverExtraEnv); + const newEnv = { ...process.env }; + for (const [k, v] of Object.entries(this.config.serverExtraEnv)) { + if (v) { + newEnv[k] = v; + } else if (k in newEnv) { + delete newEnv[k]; + } + } const run: lc.Executable = { command: this._serverPath, options: { env: newEnv }, diff --git a/editors/code/src/util.ts b/editors/code/src/util.ts index 83b8abe777..410b055100 100644 --- a/editors/code/src/util.ts +++ b/editors/code/src/util.ts @@ -14,7 +14,7 @@ export function assert(condition: boolean, explanation: string): asserts conditi } export type Env = { - [name: string]: string; + [name: string]: string | undefined; }; class Log { |