Skip to content

Global envvars overwrites process.env only when there's no terminal attached.#4786

Closed
saml wants to merge 1 commit intomicrosoft:masterfrom
saml:master
Closed

Global envvars overwrites process.env only when there's no terminal attached.#4786
saml wants to merge 1 commit intomicrosoft:masterfrom
saml:master

Conversation

@saml
Copy link
Copy Markdown

@saml saml commented Mar 30, 2016

fixes #4779

Sometimes users want to launch code from terminal after
modifying environment variables.
When code is launched from terminal, it respects those modifications.

@msftclas
Copy link
Copy Markdown

Hi @saml, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

…tached

fixes microsoft#4779

Sometimes users want to launch `code` from terminal after
modifying environment variables.
When `code` is launched from terminal, it respects those modifications.
if (!tty.isatty(1)) {
// Only assign global environment variables when
// STDOUT is not attached to terminal.
assign(process.env, userEnv);
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of testing if vscode is run from terminal, different assign method could be used. Maybe:

assign(userEnv, process.env); // session's envvar trumps login shell's envvar
process.env = userEnv;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://nodejs.org/api/process.html#process_process_env

Assigning a property on process.env will implicitly convert the value to a string.

looks like I better not replace process.env like that unless I also modify userEnv's setter.

@msftclas
Copy link
Copy Markdown

@saml, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@joaomoreno
Copy link
Copy Markdown
Member

Hi @saml! Thanks for your contribution! Unfortunately, it fell through the cracks and I failed to look into it... Fortunately, and coincidentally, this month we fixed the issue in another way in #1033.

Sorry for not paying more attention, my fault.

@joaomoreno joaomoreno closed this May 24, 2016
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot pass environment variable that is already defined in shell's rc file.

3 participants