From 38814238a5c5b82394745b32620c334b82592e23 Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Thu, 13 Apr 2017 18:38:47 +0200 Subject: [PATCH] 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 | 10 ++++++++++ README.md | 4 ++-- src/index.js | 2 +- src/variable.js | 14 +++++++++++--- src/variable.test.js | 28 +++++++++++++++++++--------- 5 files changed, 43 insertions(+), 15 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index 362f38c..86278db 100644 --- a/.all-contributorsrc +++ b/.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" + ] } ] } diff --git a/README.md b/README.md index 024c6aa..c29569d 100644 --- a/README.md +++ b/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]): | [
Kent C. Dodds](https://kentcdodds.com)
[πŸ’»](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) | [
Ya Zhuang ](https://zhuangya.me)
πŸ”Œ [πŸ“–](https://github.com/kentcdodds/cross-env/commits?author=zhuangya) | [
James Harris](https://wopian.me)
[πŸ“–](https://github.com/kentcdodds/cross-env/commits?author=wopian) | [
compumike08](https://github.com/compumike08)
[πŸ›](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) | [
Daniel RodrΓ­guez Rivero](https://github.com/danielo515)
[πŸ›](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) | [
Jonas Keinholz](https://github.com/inyono)
[πŸ›](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) | [
Hugo Wood](https://github.com/hgwood)
[πŸ›](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) | | :---: | :---: | :---: | :---: | :---: | :---: | :---: | -| [
Thiebaud Thomas](https://github.com/thomasthiebaud)
[πŸ›](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) | [
Daniel Rey LΓ³pez](https://daniel.blog)
[πŸ’»](https://github.com/kentcdodds/cross-env/commits?author=DanReyLop) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=DanReyLop) | [
Amila Welihinda](http://amilajack.com)
πŸš‡ | +| [
Thiebaud Thomas](https://github.com/thomasthiebaud)
[πŸ›](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) | [
Daniel Rey LΓ³pez](https://daniel.blog)
[πŸ’»](https://github.com/kentcdodds/cross-env/commits?author=DanReyLop) [⚠️](https://github.com/kentcdodds/cross-env/commits?author=DanReyLop) | [
Amila Welihinda](http://amilajack.com)
πŸš‡ | [
Paul Betts](https://twitter.com/paulcbetts)
[πŸ›](https://github.com/kentcdodds/cross-env/issues?q=author%3Apaulcbetts) [πŸ’»](https://github.com/kentcdodds/cross-env/commits?author=paulcbetts) | This project follows the [all-contributors][all-contributors] specification. Contributions of any kind welcome! diff --git a/src/index.js b/src/index.js index 0e223d0..130f388 100644 --- a/src/index.js +++ b/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 } diff --git a/src/variable.js b/src/variable.js index 92d66a7..febc0a2 100644 --- a/src/variable.js +++ b/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)) } diff --git a/src/variable.test.js b/src/variable.test.js index b6fa3a4..11f3d63 100644 --- a/src/variable.test.js +++ b/src/variable.test.js @@ -19,12 +19,22 @@ 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`, () => { @@ -32,24 +42,24 @@ test(`doesn't convert already valid separators`, () => { 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`, () => {