feat: Convert list delimiters for PATH-style env variables #93

DanReyLop · 2017-03-22T23:49:29Z

In Windows, env variables that represent a list (such as PATH or NODE_PATH) separate their elements
using a semicolon(;), but in UNIX they're separated using a colon (:).

This commit adds a conversion layer, UNIX-style delimiters (:) will be converted to Windows-style delimiters (;) on Windows systems at runtime. To interpret a colon literally instead, preced it with a backslash, like this: "cross-env VAR=foo\\:bar"

BREAKING CHANGE: If an env variable has : in its value, it will be converted to ; on Windows systems. To keep the old functionality, you will need to escape those characters with a backslash.

Closes #80

PS: Sorry for the delay, I was on vacation for a while and I didn't feel like coming near a computer :)

codecov-io · 2017-03-22T23:50:59Z

Codecov Report

Merging #93 into next will not change coverage.
The diff coverage is 100%.

@@         Coverage Diff         @@
##           next    #93   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         2      3    +1     
  Lines        32     43   +11     
===================================
+ Hits         32     43   +11

Impacted Files	Coverage Δ
src/variable.js	`100% <100%> (ø)`
src/index.js	`100% <100%> (ø)`	⬆️
src/command.js	`100% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8679f4...021e1df. Read the comment docs.

kentcdodds

This is fantastic. Thank you!

I'm going to consider this a breaking change and merge it into next instead so it's lumped in with the other breaking changes queued up for our next major release. Thanks!

kentcdodds · 2017-03-22T23:51:45Z

.all-contributorsrc

+      "avatar_url": "https://avatars1.githubusercontent.com/u/1715800?v=3",
+      "profile": "https://daniel.blog",
+      "contributions": [
+        "code"


Oh, do you wanna add "test" to this?

kentcdodds · 2017-03-22T23:52:50Z

There are some merge conflicts with the next branch. If you could fix those, that'd be great.

To update the contributors table in the README, you can hand-fix the .all-contributorsrc file, then run: npm start generate

In Windows, env variables that represent a list (such as PATH or NODE_PATH) separate their elements using a semicolon(;), but in UNIX they're separated using a colon (:). This commit adds a conversion layer, so regardless of how the delimiter is written when calling cross-env, it will be converted to the correct platform delimiter at runtime. To interpret a colon/semicolon literally instead, preced it with a backslash, like this: "cross-env VAR=semi\\;colon\\:" BREAKING CHANGE: If an env variable has : or ; in its value, it will be converted to : on UNIX systems or ; on Windows systems. To keep the old functionality, you will need to escape those characters with a backslash. kentcdodds#80

DanReyLop · 2017-03-23T00:03:34Z

@kentcdodds Done. The conflicts were just in the all-contributors thing, easy to fix :)

It makes total sense to consider this a breaking change, I didn't see you had a separate branch for that.

Glad to be of (a very little) help :)

kentcdodds

Perfect! Thanks!

kentcdodds

Sorry, just realized one little detail. I hope it's not too inconvenient!

kentcdodds · 2017-03-23T00:08:17Z

src/variable.test.js

+  expect(varValueConvert('foo:bar')).toBe('foo;bar')
+})
+
+test(`converts a ; into a : on UNIX`, () => {


Actually, could we simplify things a little bit? @hgwood and I have decided that it would be nicer if cross-env allowed you to only use the unix notation for things and not convert the other way. That would make the implementation simpler and the API simpler. Could you back these things out and even add a test or two to make sure that ; is not converted whether on unix or not? Thanks!

No problem, done. That was my first approach but I followed the existing logic on command.js: https://github.com/kentcdodds/cross-env/blob/master/src/command.js#L15

You folks should also simplify that to be consistent :)

Yes we should. Want to do that? 😏

…-style), not the other way around

kentcdodds

Perfect! Thank you!

* feat: Convert list delimiters for PATH-style env variables In Windows, env variables that represent a list (such as PATH or NODE_PATH) separate their elements using a semicolon(;), but in UNIX they're separated using a colon (:). This commit adds a conversion layer, so regardless of how the delimiter is written when calling cross-env, it will be converted to the correct platform delimiter at runtime. To interpret a colon/semicolon literally instead, preced it with a backslash, like this: "cross-env VAR=semi\\;colon\\:" BREAKING CHANGE: If an env variable has : or ; in its value, it will be converted to : on UNIX systems or ; on Windows systems. To keep the old functionality, you will need to escape those characters with a backslash. #80 * chore: Add myself (DanReyLop) to the contributors list * Simplified logic. Now only : (UNIX-style) are converted to ; (Windows-style), not the other way around BREAKING CHANGE: You now must escape `:` to use it in a value of you don't want it to be swapped with `;` on Windows

kentcdodds approved these changes Mar 22, 2017

View reviewed changes

kentcdodds reviewed Mar 22, 2017

View reviewed changes

kentcdodds changed the base branch from master to next March 22, 2017 23:51

Daniel Rey Lopez added 2 commits March 22, 2017 23:58

chore: Add myself (DanReyLop) to the contributors list

03ec006

DanReyLop force-pushed the convert-list-separators branch from 39112ca to 03ec006 Compare March 23, 2017 00:00

kentcdodds approved these changes Mar 23, 2017

View reviewed changes

kentcdodds suggested changes Mar 23, 2017

View reviewed changes

Simplified logic. Now only : (UNIX-style) are converted to ; (Windows…

021e1df

…-style), not the other way around

kentcdodds approved these changes Mar 23, 2017

View reviewed changes

kentcdodds merged commit 293c73d into kentcdodds:next Mar 23, 2017

DanReyLop mentioned this pull request Mar 24, 2017

Handle platform-specific list delimiters #80

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Convert list delimiters for PATH-style env variables #93

feat: Convert list delimiters for PATH-style env variables #93

DanReyLop commented Mar 22, 2017 •

edited

Loading

codecov-io commented Mar 22, 2017 •

edited

Loading

kentcdodds left a comment

kentcdodds Mar 22, 2017

kentcdodds commented Mar 22, 2017

DanReyLop commented Mar 23, 2017

kentcdodds left a comment

kentcdodds left a comment

kentcdodds Mar 23, 2017

DanReyLop Mar 23, 2017

kentcdodds Mar 23, 2017

kentcdodds left a comment

feat: Convert list delimiters for PATH-style env variables #93

feat: Convert list delimiters for PATH-style env variables #93

Conversation

DanReyLop commented Mar 22, 2017 • edited Loading

codecov-io commented Mar 22, 2017 • edited Loading

Codecov Report

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds Mar 22, 2017

Choose a reason for hiding this comment

kentcdodds commented Mar 22, 2017

DanReyLop commented Mar 23, 2017

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds Mar 23, 2017

Choose a reason for hiding this comment

DanReyLop Mar 23, 2017

Choose a reason for hiding this comment

kentcdodds Mar 23, 2017

Choose a reason for hiding this comment

kentcdodds left a comment

Choose a reason for hiding this comment

DanReyLop commented Mar 22, 2017 •

edited

Loading

codecov-io commented Mar 22, 2017 •

edited

Loading