-
Notifications
You must be signed in to change notification settings - Fork 48
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
Support cross platform environment variables #127
Comments
BTW: This works, but is pretty ugly:
.. but I don't have a windows environment to check this works there. It may simply work on mac/linux anyway. |
Just to clarify, #65 mentions the cross-env package too. This sounds like a reasonable feature for shx (shelljs already exposes this under If we want to support this, we need to decide how far we should go:
@backspaces what do you think fits the needs for most users? |
Yes, the code above is part of my package.json which I used to store configuration information for scripts. Looks like this:
So in my use-case, I would like
To be expanded to
I think that would be the basic support case. I'm not sure if you'd need a way to set env vars. I haven't run across that. |
Oh, I see. I wasn't familiar that npm package attributes were set as environmental variables. Thanks for explaining. Another thing we have to worry about: shx rm -rf $npm_package_mkdirs
# This is an 'rm' with 3 directory arguments
shx rm -rf "$npm_package_mkdirs"
# This has 1 directory argument (contains spaces in the name) Would it be reasonable to support only single-word package variables? |
I'm not sure what you mean by that. If it means my 3 dir example fails, then nope! :) But seriously, I believe the usual expansion of $foo is simply the value, spaces and all, of the $foo env var. Your second examples is fine for when the spaces are really part of the file name. As for spaces in the dir names, couldn't the programmer simply quote them like this: Note that env vars can come from lots of different files, not just package.json. AND they needn't be env vars but simply vars like so in npm scripts:
This is likely not cross-platform, however. But wouldn't Maybe the issue is node support. I found this: |
To simplify:
|
Try this: $ export foo='first_file.txt "second file with spaces.txt"'
$ for file in $foo; do echo $file; done
first_file.txt
"second
file
with
spaces.txt"
$ # Yikes! I was hoping for 1 file per line Side note: this is why bash has arrays. But you can't put arrays in env vars, so that isn't useful for shx.
Non-env vars are infeasible: these are scoped to the current shell, so shx (which runs as a subprocess) can't look-up their value.
The issue is that $ export foo='bar baz' Consider how this looks to the
(Pretend that the Our first big problem is that every case looks the same on Windows. If we can't distinguish, then which behavior do we choose? If we assume any of the cases, then we've broken the others. |
Sigh. Brilliant explanation, thank you. My current solution uses But I believe their problem is different: they mainly convert between $foo and %foo% formats and they let the unix/windows shells handle it. Historically: I got here by initially using bash backticks to create arguments for commands:
where pkgkey simply reads in package.json, converts it to a JS object and returns the key's value as a string. It's a hack, but see below. However, I soon found that backticks don't work in windows. So finally discovered env vars as a solution. I don't suppose backticks make sense for shx? Here's pkgkey.js:
|
Hmm. Just a thought: would backticks be easier to implement than env vars? |
They're accepting a full command string. So
Not exactly our purpose. We're trying to expose shelljs command implementations to the CLI, not execute arbitrary commands. If you're not opposed to writing a script, how about using shelljs directly? // package.json
{
...
"mkdirs": [ "first dir", "second dir", "third dir" ],
...
} // clean.js (hook this up to `npm run clean`)
var shell = require('shelljs');
var dirList = require('./package.json').mkdirs;
var ret = shell.rm('-rf', ...dirList);
if (ret.code === 0) shell.mkdir(...dirList); |
Nice approach! I may do that. I use shelljs in nearly all my node utilities. Like the es6 usage, thanks. And didn't know I could require a json file, that is cool. |
Yeah, unfortunately it seems like there's no obvious way to support env variables without running into these edge cases. It's a good feature request, and it would've been really nice if we had a path forward. For tough cases (and this appears to be one) we still need to resort back to ShellJS. While a short script isn't as elegant as a shell one-liner, it offers more flexibility and power. |
@nfischer I would guess upwards of 90% of use-cases are super simple single word variables, e.g. Can we not start with this very simple support and work our way up to more complex edge cases as we go? The limitations can be documented and the edge-cases can be addressed incrementally. I think this is a great tool and it certainly made my life easier, but, for me, this one small thing would make a lot of difference. |
The issue is that supporting that case would break other cases. Basically,
Yes, we can document our behavior. But just to reiterate, we cannot address edge cases incrementally: we face fundamental limitations which are impossible to resolve. If we were to implement this, the best case we could achieve is:
Basically, we have to assume what the user intends, which means sometimes we'll assume incorrectly. We could provide switches to assume the opposite way, but I'm not promising these would be good ideas or reasonable to support. |
@nfischer: btw, the example you show of How do bash programmers solve it? A common solution is I know I typically do that:
Produces:
For files, consider this directory, bar with two files "a" and "b c"
And
.. handles file names with spaces. I think shx's ls does fine: So basically, I'd say env vars with spaces are the user's problem, there are MANY way to handle them, just don't use spaces for both separators and in file names. So export foo="a:b c" would use : for separator and spaces in file names. Here's how to say what $foo does in the shx/shelljs world: exactly the same as process.env.foo does. Period. If I need to deal with edge cases, that's my concern. Just like with bash! : ) |
Can we just start with node_env? Pretty much every single user of this package will be using node_env sometime in their scripts, and it's a pain that on OSX and Linux it's 'node_env="production"' and on Windows it's 'set node_env="production"'. |
What about setting the env vars in the same line, without having to restort to cross-env? Eg:
|
@goerwin we don't support executing arbitrary commands at the moment, so I don't think there's any benefit to defining variables in-line (although I can see the benefit of combining these features together). Other than that, this is subject to the same concerns I outlined above (distinguishing between |
Add a cross-env feature which resolves environment variables in a standard way.
Example: I like to use env vars so that configuration information can be stored in package.json. I believe the following will fail on windows but works fine on mac:
The text was updated successfully, but these errors were encountered: