Skip to content
This repository has been archived by the owner on Jan 6, 2021. It is now read-only.

Remove env variables that don't exist from the converted commands in Windows. #149

Merged
merged 5 commits into from
Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const jestConfig = require('kcd-scripts/config').jest

jestConfig.coveragePathIgnorePatterns = jestConfig.coveragePathIgnorePatterns.concat(
['/bin/'],
['/bin/']
)
module.exports = jestConfig
36 changes: 27 additions & 9 deletions src/__tests__/command.js
Original file line number Diff line number Diff line change
@@ -1,56 +1,74 @@
import isWindowsMock from 'is-windows'
import commandConvert from '../command'

const env = {
test: 'a',
test1: 'b',
test2: 'c',
test3: 'd',
'empty_var': ''
}

beforeEach(() => {
isWindowsMock.__mock.reset()
})

test(`converts unix-style env variable usage for windows`, () => {
isWindowsMock.__mock.returnValue = true
expect(commandConvert('$test')).toBe('%test%')
expect(commandConvert('$test', env)).toBe('%test%')
})

test(`leaves command unchanged when not a variable`, () => {
expect(commandConvert('test')).toBe('test')
expect(commandConvert('test', env)).toBe('test')
})

test(`doesn't convert windows-style env variable`, () => {
isWindowsMock.__mock.returnValue = false
expect(commandConvert('%test%')).toBe('%test%')
expect(commandConvert('%test%', env)).toBe('%test%')
})

test(`leaves variable unchanged when using correct operating system`, () => {
isWindowsMock.__mock.returnValue = false
expect(commandConvert('$test')).toBe('$test')
expect(commandConvert('$test', env)).toBe('$test')
})

test(`is stateless`, () => {
// this test prevents falling into regexp traps like this:
// http://stackoverflow.com/a/1520853/971592
isWindowsMock.__mock.returnValue = true
expect(commandConvert('$test')).toBe(commandConvert('$test'))
expect(commandConvert('$test', env)).toBe(commandConvert('$test', env))
})

test(`converts embedded unix-style env variables usage for windows`, () => {
isWindowsMock.__mock.returnValue = true
expect(commandConvert('$test1/$test2/$test3')).toBe('%test1%/%test2%/%test3%')
expect(commandConvert('$test1/$test2/$test3', env)).toBe('%test1%/%test2%/%test3%')
})

// eslint-disable-next-line max-len
test(`leaves embedded variables unchanged when using correct operating system`, () => {
isWindowsMock.__mock.returnValue = false
expect(commandConvert('$test1/$test2/$test3')).toBe('$test1/$test2/$test3')
expect(commandConvert('$test1/$test2/$test3', env)).toBe('$test1/$test2/$test3')
})

test(`converts braced unix-style env variable usage for windows`, () => {
isWindowsMock.__mock.returnValue = true
// eslint-disable-next-line no-template-curly-in-string
expect(commandConvert('${test}')).toBe('%test%')
expect(commandConvert('${test}', env)).toBe('%test%')
})

test(`removes non-existent variables from the converted command`, () => {
isWindowsMock.__mock.returnValue = true
expect(commandConvert('$test1/$foo/$test2', env)).toBe('%test1%//%test2%')
})

test(`removes empty variables from the converted command`, () => {
isWindowsMock.__mock.returnValue = true
expect(commandConvert('$foo/$test/$empty_var', env)).toBe('/%test%/')
})

test(`normalizes command on windows`, () => {
isWindowsMock.__mock.returnValue = true
// index.js calls `commandConvert` with `normalize` param
// as `true` for command only
expect(commandConvert('./cmd.bat', true)).toBe('cmd.bat')
expect(commandConvert('./cmd.bat', env, true)).toBe('cmd.bat')
})
12 changes: 10 additions & 2 deletions src/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,24 @@ export default commandConvert
/**
* Converts an environment variable usage to be appropriate for the current OS
* @param {String} command Command to convert
* @param {Object} env Map of the current environment variable names and their values
* @param {boolean} normalize If the command should be normalized using `path`
* after converting
* @returns {String} Converted command
*/
function commandConvert(command, normalize = false) {
function commandConvert(command, env, normalize = false) {
if (!isWindows()) {
return command
}
const envUnixRegex = /\$(\w+)|\${(\w+)}/g // $my_var or ${my_var}
const convertedCmd = command.replace(envUnixRegex, '%$1$2%')
const convertedCmd = command.replace(envUnixRegex, (match, $1, $2) => {
const varName = $1 || $2
// In Windows, non-existent variables are not replaced by the shell,
// so for example "echo %FOO%" will literally print the string "%FOO%", as
// opposed to printing an empty string in UNIX. See kentcdodds/cross-env#145
// If the env variable isn't defined at runtime, just strip it from the command entirely
return env[varName] ? `%${varName}%` : ''
})
// Normalization is required for commands with relative paths
// For example, `./cmd.bat`. See kentcdodds/cross-env#127
// However, it should not be done for command arguments.
Expand Down
7 changes: 4 additions & 3 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,17 @@ const envSetterRegex = /(\w+)=('(.*)'|"(.*)"|(.*))/

function crossEnv(args, options = {}) {
const [envSetters, command, commandArgs] = parseCommand(args)
const env = getEnvVars(envSetters)
if (command) {
const proc = spawn(
// run `path.normalize` for command(on windows)
commandConvert(command, true),
commandConvert(command, env, true),
// by default normalize is `false`, so not run for cmd args
commandArgs.map(arg => commandConvert(arg)),
commandArgs.map(arg => commandConvert(arg, env)),
{
stdio: 'inherit',
shell: options.shell,
env: getEnvVars(envSetters),
env,
},
)
process.on('SIGTERM', () => proc.kill('SIGTERM'))
Expand Down