feat: don't change colons on non-PATH/NODE_PATH on win32 (#107)

* Add a whitelist for variables to : => ; ify

* Plumb the name through

* Write some tests

* Dox, linter fascism

* Much better!

* Contributors

BREAKING CHANGES: We no longer swap `:` for `;` on windows for any variables other than `PATH` and `NODE_PATH`.

.all-contributorsrc

@@ -111,6 +111,16 @@
       "contributions": [
         "infra"
       ]
+    },
+    {
+      "login": "paulcbetts",
+      "name": "Paul Betts",
+      "avatar_url": "https://avatars1.githubusercontent.com/u/1396?v=3",
+      "profile": "https://twitter.com/paulcbetts",
+      "contributions": [
+        "bug",
+        "code"
+      ]
     }
   ]
 }

README.md

@@ -11,7 +11,7 @@ Run scripts that set and use environment variables across platforms
 [![downloads][downloads-badge]][npm-stat]
 
 [![MIT License][license-badge]][LICENSE]
-[![All Contributors](https://img.shields.io/badge/all_contributors-10-orange.svg?style=flat-square)](#contributors)
+[![All Contributors](https://img.shields.io/badge/all_contributors-11-orange.svg?style=flat-square)](#contributors)
 [![PRs Welcome][prs-badge]][prs]
 [![Donate][donate-badge]][donate]
 [![Code of Conduct][coc-badge]][coc]
@@ -114,7 +114,7 @@ Thanks goes to these people ([emoji key][emojis]):
 <!-- ALL-CONTRIBUTORS-LIST:START - Do not remove or modify this section -->
 | [<img src="https://avatars.githubusercontent.com/u/1500684?v=3" width="100px;"/><br /><sub>Kent C. Dodds</sub>](https://kentcdodds.com)<br />[💻](https://github.com/kentcdodds/cross-env/commits?author=kentcdodds) [📖](https://github.com/kentcdodds/cross-env/commits?author=kentcdodds) 🚇 [⚠️](https://github.com/kentcdodds/cross-env/commits?author=kentcdodds) | [<img src="https://avatars1.githubusercontent.com/u/499038?v=3" width="100px;"/><br /><sub>Ya Zhuang </sub>](https://zhuangya.me)<br />🔌 [📖](https://github.com/kentcdodds/cross-env/commits?author=zhuangya) | [<img src="https://avatars3.githubusercontent.com/u/3440094?v=3" width="100px;"/><br /><sub>James Harris</sub>](https://wopian.me)<br />[📖](https://github.com/kentcdodds/cross-env/commits?author=wopian) | [<img src="https://avatars1.githubusercontent.com/u/8941730?v=3" width="100px;"/><br /><sub>compumike08</sub>](https://github.com/compumike08)<br />[🐛](https://github.com/kentcdodds/cross-env/issues?q=author%3Acompumike08) [📖](https://github.com/kentcdodds/cross-env/commits?author=compumike08) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=compumike08) | [<img src="https://avatars1.githubusercontent.com/u/2270425?v=3" width="100px;"/><br /><sub>Daniel Rodríguez Rivero</sub>](https://github.com/danielo515)<br />[🐛](https://github.com/kentcdodds/cross-env/issues?q=author%3Adanielo515) [💻](https://github.com/kentcdodds/cross-env/commits?author=danielo515) [📖](https://github.com/kentcdodds/cross-env/commits?author=danielo515) | [<img src="https://avatars2.githubusercontent.com/u/1508477?v=3" width="100px;"/><br /><sub>Jonas Keinholz</sub>](https://github.com/inyono)<br />[🐛](https://github.com/kentcdodds/cross-env/issues?q=author%3Ainyono) [💻](https://github.com/kentcdodds/cross-env/commits?author=inyono) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=inyono) | [<img src="https://avatars3.githubusercontent.com/u/1656170?v=3" width="100px;"/><br /><sub>Hugo Wood</sub>](https://github.com/hgwood)<br />[🐛](https://github.com/kentcdodds/cross-env/issues?q=author%3Ahgwood) [💻](https://github.com/kentcdodds/cross-env/commits?author=hgwood) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=hgwood) |
 | :---: | :---: | :---: | :---: | :---: | :---: | :---: |
-| [<img src="https://avatars0.githubusercontent.com/u/3715715?v=3" width="100px;"/><br /><sub>Thiebaud Thomas</sub>](https://github.com/thomasthiebaud)<br />[🐛](https://github.com/kentcdodds/cross-env/issues?q=author%3Athomasthiebaud) [💻](https://github.com/kentcdodds/cross-env/commits?author=thomasthiebaud) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=thomasthiebaud) | [<img src="https://avatars1.githubusercontent.com/u/1715800?v=3" width="100px;"/><br /><sub>Daniel Rey López</sub>](https://daniel.blog)<br />[💻](https://github.com/kentcdodds/cross-env/commits?author=DanReyLop) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=DanReyLop) | [<img src="https://avatars2.githubusercontent.com/u/6374832?v=3" width="100px;"/><br /><sub>Amila Welihinda</sub>](http://amilajack.com)<br />🚇 |
+| [<img src="https://avatars0.githubusercontent.com/u/3715715?v=3" width="100px;"/><br /><sub>Thiebaud Thomas</sub>](https://github.com/thomasthiebaud)<br />[🐛](https://github.com/kentcdodds/cross-env/issues?q=author%3Athomasthiebaud) [💻](https://github.com/kentcdodds/cross-env/commits?author=thomasthiebaud) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=thomasthiebaud) | [<img src="https://avatars1.githubusercontent.com/u/1715800?v=3" width="100px;"/><br /><sub>Daniel Rey López</sub>](https://daniel.blog)<br />[💻](https://github.com/kentcdodds/cross-env/commits?author=DanReyLop) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=DanReyLop) | [<img src="https://avatars2.githubusercontent.com/u/6374832?v=3" width="100px;"/><br /><sub>Amila Welihinda</sub>](http://amilajack.com)<br />🚇 | [<img src="https://avatars1.githubusercontent.com/u/1396?v=3" width="100px;"/><br /><sub>Paul Betts</sub>](https://twitter.com/paulcbetts)<br />[🐛](https://github.com/kentcdodds/cross-env/issues?q=author%3Apaulcbetts) [💻](https://github.com/kentcdodds/cross-env/commits?author=paulcbetts) |
 <!-- ALL-CONTRIBUTORS-LIST:END -->
 
 This project follows the [all-contributors][all-contributors] specification. Contributions of any kind welcome!

src/index.js

@@ -52,7 +52,7 @@ function getEnvVars(envSetters) {
     envVars.APPDATA = process.env.APPDATA
   }
   Object.keys(envSetters).forEach(varName => {
-    envVars[varName] = varValueConvert(envSetters[varName])
+    envVars[varName] = varValueConvert(envSetters[varName], varName)
   })
   return envVars
 }

src/variable.js

@@ -1,14 +1,21 @@
 import isWindows from 'is-windows'
 
+const pathLikeEnvVarWhitelist = new Set(['PATH', 'NODE_PATH'])
+
 /**
  * This will transform UNIX-style list values to Windows-style.
  * For example, the value of the $PATH variable "/usr/bin:/usr/local/bin:."
  * will become "/usr/bin;/usr/local/bin;." on Windows.
  * @param {String} varValue Original value of the env variable
+ * @param {String} varName Original name of the env variable
  * @returns {String} Converted value
  */
-function replaceListDelimiters(varValue) {
+function replaceListDelimiters(varValue, varName = '') {
   const targetSeparator = isWindows() ? ';' : ':'
+  if (!pathLikeEnvVarWhitelist.has(varName)) {
+    return varValue
+  }
+
   return varValue.replace(/(\\*):/g, (match, backslashes) => {
     if (backslashes.length % 2) {
       // Odd number of backslashes preceding it means it's escaped,
@@ -42,8 +49,9 @@ function resolveEnvVars(varValue) {
 /**
  * Converts an environment variable value to be appropriate for the current OS.
  * @param {String} originalValue Original value of the env variable
+ * @param {String} originalName Original name of the env variable
  * @returns {String} Converted value
  */
-export default function varValueConvert(originalValue) {
-  return resolveEnvVars(replaceListDelimiters(originalValue))
+export default function varValueConvert(originalValue, originalName) {
+  return resolveEnvVars(replaceListDelimiters(originalValue, originalName))
 }

src/variable.test.js

@@ -19,37 +19,47 @@ test(`doesn't affect simple variable values`, () => {
 
 test(`doesn't convert a ; into a : on UNIX`, () => {
   isWindowsMock.__mock.returnValue = false
-  expect(varValueConvert('foo;bar')).toBe('foo;bar')
+  expect(varValueConvert('foo;bar', 'PATH')).toBe('foo;bar')
 })
 
-test(`converts a : into a ; on Windows`, () => {
+test(`doesn't convert a ; into a : for non-PATH on UNIX`, () => {
+  isWindowsMock.__mock.returnValue = false
+  expect(varValueConvert('foo;bar', 'FOO')).toBe('foo;bar')
+})
+
+test(`doesn't convert a ; into a : for non-PATH on Windows`, () => {
+  isWindowsMock.__mock.returnValue = true
+  expect(varValueConvert('foo;bar', 'FOO')).toBe('foo;bar')
+})
+
+test(`converts a : into a ; on Windows if PATH`, () => {
   isWindowsMock.__mock.returnValue = true
-  expect(varValueConvert('foo:bar')).toBe('foo;bar')
+  expect(varValueConvert('foo:bar', 'PATH')).toBe('foo;bar')
 })
 
 test(`doesn't convert already valid separators`, () => {
   isWindowsMock.__mock.returnValue = false
   expect(varValueConvert('foo:bar')).toBe('foo:bar')
 })
 
-test(`doesn't convert escaped separators on Windows`, () => {
+test(`doesn't convert escaped separators on Windows if PATH`, () => {
   isWindowsMock.__mock.returnValue = true
-  expect(varValueConvert('foo\\:bar')).toBe('foo:bar')
+  expect(varValueConvert('foo\\:bar', 'PATH')).toBe('foo:bar')
 })
 
 test(`doesn't convert escaped separators on UNIX`, () => {
   isWindowsMock.__mock.returnValue = false
-  expect(varValueConvert('foo\\:bar')).toBe('foo:bar')
+  expect(varValueConvert('foo\\:bar', 'PATH')).toBe('foo:bar')
 })
 
 test(`converts a separator even if preceded by an escaped backslash`, () => {
   isWindowsMock.__mock.returnValue = true
-  expect(varValueConvert('foo\\\\:bar')).toBe('foo\\\\;bar')
+  expect(varValueConvert('foo\\\\:bar', 'PATH')).toBe('foo\\\\;bar')
 })
 
-test(`converts multiple separators`, () => {
+test(`converts multiple separators if PATH`, () => {
   isWindowsMock.__mock.returnValue = true
-  expect(varValueConvert('foo:bar:baz')).toBe('foo;bar;baz')
+  expect(varValueConvert('foo:bar:baz', 'PATH')).toBe('foo;bar;baz')
 })
 
 test(`resolves an env variable value`, () => {