-
-
Notifications
You must be signed in to change notification settings - Fork 239
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
Support passthrough of additional arguments to commands via placeholders #307
Conversation
Hi, @gustavohenke! This PR is now rebased on the latest changes. After thinking about it, I've decided to introduce a new option to toggle this feature. The option is disabled by default which means this PR is not a breaking change. Hopefully, this makes it easier for you to consider this PR "acceptable". Thanks! |
Thanks @gustavohenke for your review! The PR has been updated accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome job! Only a couple more suggestions and it should be good to go.
Co-authored-by: Gustavo Henke <guhenke@gmail.com>
Co-authored-by: Gustavo Henke <guhenke@gmail.com>
Co-authored-by: Gustavo Henke <guhenke@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome job, and welcome aboard! Feel free to merge when ready 😃
Thank you very much! 😃 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Alright this is now out in |
This comment was marked as duplicate.
This comment was marked as duplicate.
Maybe it's a problem with use with ~/Projects/test-conc > npx concurrently -P "echo {1}" -- foo emiller at C02Z21Z9LVDL
[0] foo
[0] echo foo exited with code 0
~/Projects/test-conc > yarn concurrently -P "echo {1}" -- foo emiller at C02Z21Z9LVDL
yarn run v1.22.5
warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.
$ /Users/emiller/Projects/test-conc/node_modules/.bin/concurrently foo
[0] /bin/sh: foo: command not found
[0] foo exited with code 127
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
|
Yes, this is specific to ❯ yarn concurrently -P "echo {1}" -- foo
yarn run v1.22.18
warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.
$ /[...]/node_modules/.bin/concurrently foo On the command line you can work around this by adding an additional "--" in front of the actual arguments: yarn concurrently -- -P "echo {1}" -- foo When using "scripts": {
"example": "concurrently -P 'echo {1}' -- foo"
}, ❯ yarn example
yarn run v1.22.18
$ concurrently -P 'echo {1}' -- foo
[0] foo
[0] echo foo exited with code 0 |
Args we pass end up with unexpected quotation marks around them, and slashes. We're trying to pass args to Webpack like this: "build-dev": "concurrently --passthrough-arguments \"npm run build-preload -- {@}\" \"npm run build-main-dev -- {@}\" \"npm run build-renderer-dev -- {@}\" -- ",
"package-demo": "npm run build-dev -- --env IS_DEMO_MODE=true", When we run {
WEBPACK_BUNDLE: true,
WEBPACK_BUILD: true,
'IS_DEMO_MODE\\\\': 'true'
}
{
WEBPACK_BUNDLE: true,
WEBPACK_BUILD: true,
'IS_DEMO_MODE\\': 'true'
} We also tried using Big thanks to @paescuj for working on this! Edit: If we use |
Hey @slapbox, thanks for your feedback! |
Hey @paescuj. Indeed it is Windows, and a bit strange because we're using zsh through cygwin. The script |
Hey @slapbox, just wanna let you know that this issues is still on my todo list... I have some ideas in my head to get rid of it, but I need some time for testing & implementation. |
@paescuj thanks for the follow-up! I was able to get it working in our case by using "build-dev": "concurrently --passthrough-arguments \"npm run build-preload -- {@}\" \"npm run build-main -- {@}\" \"npm run build-renderer -- {@}\" -- ",
"package-demo": "npm run build-dev -- --env IS_DEMO_MODE", |
This pull request adds support to passthrough additional / positional arguments (passed to
concurrently
via command line after--
) to individual commands via the following placeholders:{<number>}
(for example{1}
) to use a single argument at a specific position{@}
to use all arguments{*}
to use all arguments combined into one argumentThis feature is disabled by default (so no breaking change) and can be enabled via the
--passthrough-arguments
(alias-P
) option.For example:
This is especially useful if you want to run essentially the same scripts while one of it having additional arguments (
build
vs.dev
):I decided to use placeholders because I think this gives us the most flexible solution. For example, this allows us to pass the arguments to only some of the commands instead of all commands:
Inspired by npm-run-all and #282 (Thanks!).
Tests are included in this pull request. Additionally, I've tested some scenarios manually (like the one above with
package.json
scripts).Closes #33
Closes #282 (as replacement)
In addition this pull requests also contains a couple of small enhancements, some of them in separate commits: