search

phpro / grumphp

A PHP code-quality tool
MIT License
4.13k stars 430 forks source link

Add possibility to append files to shell task #906

Closed yguedidi closed 3 years ago

yguedidi commented 3 years ago
Q A
Branch master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Documented? yes
Fixed tickets

Some commands may take the list of changed files as arguments.

My case is that I have several PHP scripts that are checking different kind of things, and they all take the list of changed files as arguments. I plan to define a custom task per PHP script/command thanks to the metadata.task parameter

veewee commented 3 years ago

Can you give me some example use-cases of those bash scripts?

I really want to avoid adding all files to pretty much every task inside grumphp, because there are limitations on CLI input length. For linux, those are pretty high - but still limited.

Only the pre-commit hook should be allowed to pass files to CLI commands, since the list of changed files (normally) is quite small. For the regular run command, this can become quite a list, resulting in task failure.

It somehow feels like a bad command if it can't work without providing files (or other config).

yguedidi commented 3 years ago

They work without providing files, but they also support a list of files to be passed, because they are used as pre-commit too. They are script like "check this thing must be followed by one of this things" or "forbid this string everywhere".

yguedidi commented 3 years ago

So if I understood correctly, you may accept this PR if the addition of files is wrapped by a check on the precommit context? :)

veewee commented 3 years ago

Hello @yguedidi,

Thanks for your work on this. I don't think adding files to a shell script is a good idea, since every shell script is different. One could read from a file, or stdin or cli arguments. Since shell is way to flexible, I'm gonna close this issue.

yguedidi commented 3 years ago

Hello @veewee ! thanks for answering!

since every shell script is different.

isn't it because every shell script is different that GrumPHP should be able to support all of them? those who read from a file, those who read from stdin, and those who read from CLI arguments.

don't you think that home made scripts for precommit checks is a use case?

without this possibility, at least in the context of precommit, how could I make GrumPHP use my scripts in precommit context? without running on all project files, only the changed ones. should I rewrite them in PHP? or prepend them all by a home made detection of changed files?

I accept your decision, I just need a solution to my need :sweat_smile: