Bash style variable expension in env var supports defaults using variables

[wilmardo]

Closes: #2250

PR Type

• Feature Pull Request

Expand the interpolation so expands default defined variables. Based on the implementation in bash:

# export TEST=test
# echo ${NONE:-$TEST}
test
# echo ${NONE:-$NONE}

When the default variable is not found it returns an empty string.

I use this to postfix the platform names so no conflicts happen, in the CI it uses a predefined Gitlab variable, for local testing it takes the user running molecule.

platforms:
  - name: Docker-CE-${CI_COMMIT_REF_SLUG:-$VMWARE_USER}

Changes:

• Expanded the existing test so it tests both the curly and non curly braced syntax
• Added a testcase for the existing default string interpolation
• Implemented default variable interpolation and added a testcase

[wilmardo]

Same unrelated PR fail as in #2193

[ssbarnea]

I like the feature but the implicit expansion to empty string worries me. I know that bash has this behavior when not run in strict mode.

Still, I am not very keen on replicating bash bugs, especially as one like this would easily get ignored.

I am not +1/-1 because I want to see what other think about desired behavior when variable is not defined.

I personally prefer getting an error and asking user to provide a default if they want, like: $FOO would give error but if you put ${FOO:-} it will not be an error as you mention explicitly that undefined goes as empty string.

[wilmardo]

@ssbarnea Thanks for the feedback! I am open for changes and I am keen to what other have to say!

My reasoning is that this behavior lays in the extend of the syntax and is expected for the endusers (Bash is default non strict).

[wilmardo]

@decentral1se Any input on this PR?

[decentral1se]

LGTM.

[fabianvf]

Big +1 from me, I've definitely wanted this at least a couple times. My only comment is that it might make sense to make the interpolation recursive (so you could do ${UNDEFINED:-${STILL_UNDEFINED:-hello}}), but I've never really needed more than two levels anyway so it's probably more complex than it's worth.

[wilmardo]

@fabianvf Thanks for the opinion! I think the recursive option is a little overkill, at some point you should reconsider your approach ;)

Will fix the conflicts first thing tomorrow :)