Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for parameter expansion #342

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hpohlmeyer
Copy link

Adds support for parameter expansions in strings. For example:

./node_modules/.bin/env-cmd -x echo "Hello ${WHO}!"

and

{
  "script": {
    "hello": "env-cmd -x echo \"Hello ${WHO}!\""
  }
}

Closes #341

return str.replace(/(?<!\\)\$[a-zA-Z0-9_]+/g, varName => {
const varValue = envs[varName.slice(1)]
return varValue === undefined ? varName : varValue
return str.replace(/(?<!\\)\$\{?([a-zA-Z0-9_]+)\}?/g, (_, varName: string) => {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation does not check, if the pattern is contained in a string. That would make the regex way more complex.

Currently something like env-cmd -x ${WHO} would also be expanded. I don’t think that is a big deal, but if so we could use something like ((?<="[^"]*)\${([a-zA-Z0-9_]+)}(?=[^"]*")|(?<!\\)\$([a-zA-Z0-9_]+)).

Comment on lines +92 to +93
process.kill(process.pid, signal)
return
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linting was failing for me on this one, because process.kill seems to return a boolean, but the function expects the return value to be void.


it('should replace environment variables in args', (): void => {
// eslint-disable-next-line no-template-curly-in-string
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to disable the rule for this line, otherwise I could not commit my test string.

@nkrul
Copy link

nkrul commented Feb 22, 2022

Think I submitted something similar in #253 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parameter Expansion support
2 participants