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

Environment variable integration #16

Closed
dundalek opened this issue Nov 3, 2017 · 3 comments
Closed

Environment variable integration #16

dundalek opened this issue Nov 3, 2017 · 3 comments

Comments

@dundalek
Copy link
Owner

dundalek commented Nov 3, 2017

As for now one can use the JS interop to access and set environment variables:

echo (str js/process.env.VAR_NAME)
;; or
echo (expand "$VAR_NAME")

;; change with
(aset js/process.env "VAR_NAME" value)

Maybe we want to introduce helper functions like (setenv) or (setenv!) and (getenv) ?

Another thing is setting an env variable only for the child process, alternative to bash:
NODE_ENV=production npm start

@mnewt
Copy link
Contributor

mnewt commented Nov 4, 2017

I created a pull request for (getenv) and (setenv). It seems omitting the '!' is the way you're going with other things. I agree with this, as it's cumbersome to type and this is a shell--lots of stuff is going to be mutable.

I think more needs to be done though. I haven't fully thought this through, but it seems closh will need shell local and exported (environment) variables just like other shells. Any thoughts on how to handle that?

As for setting an environment variable only for a child process, closh can do the same thing fish does and just rely on an external env command. E.g.

    $ env P=1 bash -c "echo $P"
    1

This works, but I can't help thinking there is a more elegant solution.

@dundalek
Copy link
Owner Author

dundalek commented Nov 5, 2017

I like the reasoning behind omitting the bang.

Sweet, that's a pretty nifty trick with the env command. I am glad it works so we don't need to add an additional function.

@dundalek
Copy link
Owner Author

dundalek commented Dec 1, 2017

Implemented in the PR

@dundalek dundalek closed this as completed Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants