feat: add default value handling and PWD resolution

[yinzara]

What:

Adds support for specifying default values for environment variables using the standard POSIX/bash syntax ${ENV_VAR_NAME:-default value}. Also support recursive resolution through defaults (i.e. ${ENV_VAR_NAME:-${OTHER_ENV_VAR:-default if both not set}})

Also adds support for resolution of ${PWD}/$PWD into the current working directory.

Why:

Currently there is no way with the library to specify a default value for an environment variable when not present. There is a reference in the readme to another library that seems to be a fork of this library with the added feature however I can't find the source for it and the documentation for the library incorrectly references this library. Additionally how they suggest you specify a default value (i.e. ${ENV_VAR_NAME:default value} ) doesn't follow standard POSIX/bash so there is no portability of this syntax beyond cross-env.

In most linux/unix shells the PWD environment variable points to the current working directory but in Windows no such environment variable exists. This PR also adds resolution in windows using the NodeJS process.cwd() function.

How:

This required a rewrite of the primary regular expression based implementation of value replacement to a combination iterative/recursive algorithm.

To implement the recursive replacement in the same fashion as Bash does (so that if the value was evaluated in Bash before it wouldn't cause functional changes), I had to use a brace matching iterative state machine over the characters of the string that recursed on the default values calculation.

This allowed me to replace the double implementation (in command.js and variable.js) of the string replacement algorithm. Previously there were two nearly identical implementations that only varied in Windows and for the command.js only.

I've created a new "env-replace.js" that exports a function that achieves this for both. It's documented in the file. All existing test cases pass with no changes and a new test case has been added for an extremely complex case: start-${PWD}-${EMPTY_VAR:-${NO_VAR:-$VAR1}}-${NO_VAR:-$VAR2}-${NO_VAR:-${EMPTY_VAR:-value3}}-${EMPTY_VAR:-$PWD}-end which now passes. I will provide screen shots of this evaluating in both PowerShell (through a npm script) and Mac OS Bash (both in the console in via an npm script).

Checklist:

• [X ] Documentation
• [X ] Tests
• [X ] Ready to be merged

[codecov[bot]]

Codecov Report

Merging #219 into master will not change coverage. The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #219   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      5    +1     
  Lines          78    132   +54     
  Branches       18     35   +17     
=====================================
+ Hits           78    132   +54

Impacted Files	Coverage Δ
src/env-replace.js	100% <100%> (ø)
src/command.js	100% <100%> (ø)	:arrow_up:
src/variable.js	100% <100%> (ø)	:arrow_up:

---

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 67f21c3...318f2f7. Read the comment docs.

[kentcdodds]

Thanks for this!

I'm concerned that the shell may eagerly replace these values before they get to cross-env (so people will have to use cross-env-shell instead). Could you test this out locally to make sure it works as expected? You should be able to simply run: node ./src/bin/cross-env.js to get cross-env running locally, then try out a few combinations and provide a screenshot or something.

[franklin-ross]

@kentcdodds, does that practically matter? If the shell converts them, all good, if you're not on a shell that understands that syntax, cross-env will convert them. Everyone's a winner! :-D

[kentcdodds]

I had that thought as well. But I just want to be sure.

[yinzara]

Yeah I agree. I'll make an example in both Windows PowerShell and Bash in Linux and we'll see if it comes out with the same results but I'm pretty sure it will. However, as stated, since I'm just duplicating the functionality of what Bash already supports, I can't see how it wouldn't function identically.

[franklin-ross]

I guess the only issue would be if the timing matters for some reason. Like if some insane person does BOB=5 cross-env BOB=${BOB:10}, then I think bash would replace it early, meaning BOB would be 10, whereas cross-env would see BOB as already set, and then it would be 5. I'm no bash wizard, but that seems like what would happen to me. It's a pretty unusual case though.

[yinzara]

Good news and bad news.

Good news. CMD does not expand the variables. If you run cross-env directly from PowerShell, it DOES eagerly expands them but it does this already (i.e. cross-env already doesn't work correctly with PowerShell directly) because variables in PowerShell start with a $. However, if you use cross-env from within a package.json "scripts" script from within PowerShell it does not eagerly expand the variables. Since this is the primary use of cross-env I'm not concerned.

However, there is bad news. When I started to use more complex examples of replacements it turns out the regex style approach I was using has examples that break. I will add test cases to cover this and see if I can rework the approach.

[yinzara]

Not bad. I need to add some test cases to keep us at 100% (I won't say this is good until then).

[yinzara]

CMD.exe

[yinzara]

Windows PowerShell

[yinzara]

Mac OS X Bash

[yinzara]

Any chance I could get some love on this? I really want to use this with my team but keeping the custom version is a maintenance nightmare.

[derekgreer]

Any updates on this?

[franklin-ross]

Hey, my apologies. I got a bit lost in my new workplace. I'm going to jump back on this and create a PR.

[franklin-ross]

I have to admit that I initially balked when I realised exactly how messy this all is. I believe the library I've written is quite true to bash, but the nature of cross-env running within an arbitrary shell which may or may not parse some of the input before we get to it makes it quite hard to decide on any kind of concrete acceptance criteria.

I need to get over that and accept that cross-env is practically very useful despite that messiness, and that this will make it more useful.

I'm afraid that this will increase the (already significant) number of issues raised to the tune of "this doesn't work for this corner case". I think it's still worth it, but I want to point that out.

[derekgreer]

Would be great if we could get this feature merged.

[kentcdodds]

https://github.com/kentcdodds/cross-env/issues/257

[yinzara]

Thanks @kentcdodds for all your work. Totally understand about this.