-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
export Base.withenv #10914
export Base.withenv #10914
Conversation
Looks great. |
@test readall(setenv(`sh -c "echo \$TEST"`,"TEST"=>"Hello World")) == "Hello World\n" | ||
@test (withenv("TEST"=>"Hello World") do | ||
readall(`sh -c "echo \$TEST"`); end) == "Hello World\n" | ||
@test readall(setenv(`sh -c "pwd"`;dir="/")) == readall(setenv(`sh -c "cd / && pwd"`)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this last one's going to fail on windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe dir=".."
instead of /
?
All of the uses of
Theoretically, |
…s; support similar arguments in setenv for consistency, and fix the setenv tests (closes JuliaLang#10836)
I use |
At the least the way in which |
Okay. |
I do dislike underscores, but I feel the need to mention that we have |
Related:
|
As discussed in #10836, this exports
Base.withenv
(renamed fromwith_env
for consistency withsetenv
); it now takes zero or morevar=>val
arguments to specify the changes to the environment.For consistency, I added a
setenv
method takingvar=>val
arguments (Pair...
). However, the main usage pattern forsetenv
seems to be tocopy(ENV)
and modify/set one environment variable. Maybesetenv
should be deprecated entirely in favor ofwithenv
?Note also that the
setenv
test results weren't actually being checked.