Added support for additional Script parameters to install and update commands #1734
Conversation
- Scripts and callbacks can be configured by adding commandline parameters: `composer install --script-param="name1:key1=value1"` `composer install --script-param="value2"` (key will be equal to value2) Multiple parameters can be added: `composer install --script-param="name1:key1=value1" --script-param="value2"` Events get an additional method: - `getScriptParams()`: returns an array of commandline parameters added with --script-param
- Scripts and callbacks can be configured by adding commandline parameters:
`composer install --script-param="name1:key1=value1"`
`composer install --script-param="value2"` (key will be equal to value2)
Multiple parameters can be added:
`composer install --script-param="name1:key1=value1" --script-param="value2"`
Scripts in composer.json can have named parameters:
"scripts": {
"post-install-cmd": [
"myscript {name1} {value2}"
]
}
Events get an additional method:
- `getScriptParams()`: returns an array of commandline parameters added with --script-param
|
@pborreli I appreciate the comment, but need a little help.. what can I do that fixes "CS"? |
|
@pborreli nevermind, I got it. |
|
sorry ! you have some empty lines or useless spaces, it was more a reminder to repo owner before merging :) thanks for the PR anyway :) |
There was a problem hiding this comment.
Capital letter Set and trailing dot.
|
Excellent help, thank you! Do you use a tool for that? I'm only now discovering http://cs.sensiolabs.org/ for example.. |
|
no I was trained by @stof as a codesniffer dog :) but cs.sensiolabs.org is a great tool, good find ! |
There was a problem hiding this comment.
@pborreli in my defense, there are lots of examples without leading capital and trailing dot. It seems inconsistent, some files have it some files don't
There was a problem hiding this comment.
omg don't think i can excuse you if you make typo in my nick
There was a problem hiding this comment.
What typo? grin
[omg, notice how I had to update twice to fix my mistake ..just call me F_cku_p500 ;) ]
|
@pborreli ( <-- the one and only) |
|
👍 |
|
I'm not strictly against this, but I want to say that scripts inherit the composer env vars. As such you can easily pass stuff to scripts like |
|
True; and it makes sense that it's always possible to write a wrapper for existing scripts/binaries that work with parameters.. But as this didn't break anything and ads some flexibility I thought I'd see it through completely and add a pull request. What happens next is up to the maintainer ;) |
|
This adds complexity in the command, and once you add a command using script params, they become required to avoid breaking the composer call (because running So for me, it is a -1. |
|
@stof I'm not sure I understand you correctly.. you can't add script-params in older composer versions because the InstallCommand would not allow it, and params are always optional to scrips because the {tags} are always replaced (with nothing if there was no param with the same name). |
|
As an example, it could become quite tedious to run: and having a wrapper script with the sole purpose of coverting those environment variables back to: In my case it would be: And your scripts configured as: Or maybe: All tags are always removed from the command so only if your own script requires the param would you be required to add the additional parameters to composer install (and then only if the script returns a non zero exit status). That being said, I have no examples of scripts we use with composer needing multiple parameters, so perhaps it's just an edge case without real world usefulness. |
|
:+1 for the idea |
|
I think we can safely put this one to rest :-) |
This pull request is an attempt at allowing scripts to have parameters defined at runtime.
An example would be to use >composer install --script-tag="env:--env=prod" on production.
Scripts would then be configured in composer.json with "myscript {env}".
PHP Callbacks would have access to the parameters through the Event object:
$event->getScriptParams();
There are some decisions that need review in my opinion:
-- Is it desirable to add the scriptParams to the Event constructor?
-- Would the InstallCommand and UpdateCommand need more explanation?
-- Is the help page correctly altered?
-- Are there tests needed? I could not find a good place to add tests for this...