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

Allow EnvironmentVariableCollection to unset variables #185200

Open
mkhl opened this issue Jun 15, 2023 · 2 comments
Open

Allow EnvironmentVariableCollection to unset variables #185200

mkhl opened this issue Jun 15, 2023 · 2 comments
Assignees
Labels
api feature-request Request for new features or functionality terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc.
Milestone

Comments

@mkhl
Copy link
Contributor

mkhl commented Jun 15, 2023

EnvironmentVariableCollection currently support modifying a variable, or deleting a modification in that collection, but not unsetting an environment variable.

Sometimes we want to unset variables in the direnv extension but there is no API for it (and if we force null or undefined through the current API they get automatically stringified to "null" and "undefined").
The closest we can do is set them to the empty string (what POSIX calls null), but then some programs insist on distinguishing empty from undefined variables.
(N.B. Shells make this intentionally hard, for example the bash man page omits the expansion modifiers that distinguish null and unset variables.)

This could be done either by allowing null and/or undefined as the replacement value for replace, or with a new method on the API.

Related issue: direnv/direnv-vscode#527, and previously NixOS/nix#6409

Related commits: direnv/direnv-vscode@9905fe7, and previously direnv/direnv-vscode@79a4841

@roblourens roblourens assigned Tyriar and meganrogge and unassigned roblourens Jun 17, 2023
@meganrogge meganrogge added feature-request Request for new features or functionality terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc. labels Jun 20, 2023
@vscodenpa vscodenpa added this to the Backlog Candidates milestone Jun 20, 2023
@vscodenpa
Copy link

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@Tyriar Tyriar added the api label Jun 22, 2023
@Tyriar
Copy link
Member

Tyriar commented Jun 22, 2023

Here's a natural proposal:

export interface EnvironmentVariableMutator {
	readonly value: string | undefined;
}

export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
	/**
	* @param value The value to replace the variable with, undefined will unset the variable.
	*/
	replace(variable: string, value: string | undefined): void;
}

But this would be a breaking API change and so would a separate EnvironmentVariableCollection.delete API as it would have to change the Iterable part. So this ask is a bit trickier than it looks.

Here's something that would work but it's a little more clunky:

export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
	/**
	 * Whether to unset instead of set when mutators replace a variable with the empty string.
	 */
	unsetEmpty: boolean;
}

@Tyriar Tyriar modified the milestones: Backlog Candidates, Backlog Jun 22, 2023
@meganrogge meganrogge modified the milestones: Backlog, July 2023 Jun 26, 2023
@meganrogge meganrogge removed their assignment Jul 12, 2023
@Tyriar Tyriar modified the milestones: July 2023, Backlog Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api feature-request Request for new features or functionality terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc.
Projects
None yet
Development

No branches or pull requests

5 participants