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

Handling platform specific delimiters #3

Closed
hekard2l opened this issue Feb 21, 2017 · 18 comments
Closed

Handling platform specific delimiters #3

hekard2l opened this issue Feb 21, 2017 · 18 comments

Comments

@hekard2l
Copy link
Contributor

This isn't necessarily an issue, but it'd be nice to handle platform specific delimiters for environment variables that contain a list of entries.

$PATH and $NODE_PATH are examples of entry lists. NODE_PATH documentation.

On Windows the delimiter is ; (semicolon) and is : (colon) on most other platforms.

Would there be a clean way to replace : with ; on Windows without disrupting users that actually want to insert : on Windows?

@hekard2l
Copy link
Contributor Author

I think using $: in place of ; (Windows) and : (*nix) would work nicely. I've implemented it and cannot see any drawbacks, because environment variable names are primarily alphanumeric.

ie.

cross-var NODE_PATH=$NODE_PATH$:folder ..

@elijahmanor
Copy link
Owner

Good point, I haven't used cross-env along with cross-var yet, but I can see how that would be a problem. I'm currently working on a separate branch that restructures the code and adds unit tests, but I can look into your PRs into the existing code base and then make sure they get applied to the restructured code too. I'll check out your stuff tonight... thanks for your feedback!

@elijahmanor
Copy link
Owner

So, I love the idea of this, but I'm not sold on the $: delimiter just yet.

cross-var NODE_PATH=$NODE_PATH$:folder ..

At first glance it makes me think (although I know it's not true) that NODE_PATH needs dollar signs before and after to resolve $NODE_PASH$ which isn't the case, but for some reason it is throwing me off.

I'd like to briefly consider another delimiter. Any other suggestions? How about @DanReyLop or @kentcdodds

@kentcdodds
Copy link

Honestly, it seems to me like that would belong to a separate package as well.. I realize that this can get a little rediculous:

separate-package cross-var cross-env NODE_PATH=... etc.

But I really believe in the linux philosophy of doing one thing well. And I believe that once such a package exists, you could create a final package that ties them all together quite nicely:

cross-platform NODE_PATH=... etc.

@DanReyLop
Copy link

DanReyLop commented Mar 2, 2017

I'd like to briefly consider another delimiter. Any other suggestions?

How about printf-style? Assume that every occurence of : in a variable value is a delimiter. If you want it to be interpreted as a literal :, then escape it like \\: (or ::). I realize that would be a breaking change, but it would provide for the most natural way to use this.

And I believe that once such a package exists, you could create a final package that ties them all together quite nicely

Yes please, that! I use cross-env and cross-var and the amount of noise in my npm scripts is already ridiculous. For example:
"start": "cross-var cross-env NODE_PATH=$NODE_PATH$:server$:client$:. cross-env CALYPSO_ENV=development cross-env NODE_ENV=development npm run start:env",

Ideally I would want to write this:
"start": "env NODE_PATH=$NODE_PATH:server:client:. CALYPSO_ENV=development NODE_ENV=development npm run start:env",

No cross-env before each variable, no separate commands for env and var adapters, shorter command name. I'm all in for trying to make something like that happen.

PS: Congrats @kentcdodds on your 1000 stars on cross-env!

@kentcdodds
Copy link

@DanReyLop, have you considered using nps? You could make a function that could handle this for you and really clean up your scripts (and perhaps such a function could be included in nps-utils :)

@kentcdodds
Copy link

I just realized that cross-env should be able to do what cross-var does. See kentcdodds/cross-env#32

@DanReyLop
Copy link

I just realized that cross-env should be able to do what cross-var does. See kentcdodds/cross-env#32

Nice, I didn't know that! Should we consider cross-var deprecated then? No offense to @elijahmanor, but I think we all can agree that having cross-var just provide a subset of the cross-env functionality doesn't have any advantage, it's just confusing.

Looking at the code (haven't tried it yet), cross-env has this exact same issue (doesn't adapt PATH-style separators), so this discussion is still relevant, just in other repo :)

have you considered using nps? You could make a function that could handle this for you and really clean up your scripts (and perhaps such a function could be included in nps-utils :)

Didn't know about nps either :). It looks really cool, just the ability to add comments and descriptions makes it very compelling to use. However, that file is not what other JS developers are used to (they expect everything on package.json), and having to run every script like npm start realScriptName is a bit weird. I'll keep that in mind if package.json gets too complex, altough at that point maybe it would be better for us to go all-in with grunt or gulp.

@kentcdodds
Copy link

However, that file is not what other JS developers are used to (they expect everything on package.json)

True, but people get used to it 🤷‍♂️

having to run every script like npm start realScriptName is a bit weird.

If you install nps globally then it's just nps and you can use prefixes so it's even less typing than npm ;-) (and even if you don't install nps globally, npm start t.w is still shorter than npm run test:watch)

maybe it would be better for us to go all-in with grunt or gulp.

I think that grunt and gulp make things more complex than they need to be. I'd much rather use CLIs to accomplish my tasks and if something's more complex than can be represented in a single script I break out to an actual node script. I've not felt like grunt or gulp could have simplified my scripts since I stopped using them over two years ago. :)

But to each their own!

As to supporting this specific issue in cross-env, we can file an issue there and talk about it. But I'm not going to go with $: for the same reason @elijahmanor mentioned. We'll need to come up with something else...

@DanReyLop
Copy link

But to each their own!

Exactly. I'm not saying nps is bad or anything like that, just that it's even more different than using plain npm scripts. For context, right now our build system uses a Makefile, and I'm experimenting to get rid of that (I use Windows so that's a pain). I feel that the more I deviate from "the norm", it will be more unlikely that'll be accepted :)

That's why this is not urgent for me, for now it's just a little experiment branch.

As to supporting this specific issue in cross-env, we can file an issue there and talk about it. But I'm not going to go with $: for the same reason @elijahmanor mentioned. We'll need to come up with something else...

I'll create an issue later today, we'll surely figure something out, this feature is very useful.

@elijahmanor
Copy link
Owner

Well, I didn't realize cross-env did what cross-var already did! @kentcdodds

The one thing cross-var still does that I don't think cross-env does yet is to allow for complex commands that include pipes and redirects (like the following)

"build:css": "cross-var \"node-sass src/index.scss | postcss -c .postcssrc.json | cssmin > public/$npm_package_version/index.min.css\""

Which is why I created the alternate syntax of having the first argument being a string that will be executed once all the replacements have been made. I wonder if that could be a feature of cross-env as I know it is currently a Known Limitation

@kentcdodds
Copy link

Well, I didn't realize cross-env did what cross-var already did!

Me either! 😅 my bad!

You're right, cross-env doesn't support that! Maybe it should! And that would resolve the known limitation issue as well, you're right!

@elijahmanor
Copy link
Owner

@kentcdodds I would be cool to deprecate cross-var if cross-env could support complex statements. I'll create an issue on cross-env tonight and we can start the conversation.

@cvillerm
Copy link

One benefit I find with cross-var over cross-env is that cross-var works with environment variables defined "externally", such as environment variables defined by npm (e.g. npm_package_name). I can use it to customize my scripts commands (e.g. use application name).

For instance, with:

"name": "my-nice-app",
scripts: {
    "cross_env": "cross-env echo $npm_package_name",
    "cross_var": "cross-var echo $npm_package_name"
}

This results in the following on Windows (with Windows cmd, CygWin bash or VSCode bash):

$ npm run cross_env
%npm_package_name%

$ npm run cross_var
my-nice-app

where I would expect to see my-nice-app

@kentcdodds
Copy link

That sounds more like a cross-env bug than a benefit over cross-env :) If you could please file that on cross-env (a pull request would be awesome too, even if it just added a failing test), then we can get that resolved :)

@cvillerm
Copy link

@kentcdodds , Ok, based on the description of cross-env, I wasn't sure if it was supposed to work with environment variables defined outside of a cross-env command. Let me create an issue for cross-envand see what others think about it.
Thanks.

@elijahmanor
Copy link
Owner

closing this issue since it's going to be addressed in cross-env. looks like this repo will be deprecated soon once the issues are resolved. thanks everyone!

@cvillerm
Copy link

To close the loop, as discussed at kentcdodds/cross-env#91, using environment variables defined "outside of" this command (e.g. for environment variables defined by npm such as pm_package_name) works fine with cross-var, but not with cross-env, so this still gives some value to cross-var

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants