FixCommand - allow to pass multiple path argument #1903
Conversation
keradus
force-pushed
the
2_fix_multi
branch
4 times, most recently
from
8176fe1
to
d939f87
Compare
|
Travis is dead ? |
| @@ -401,52 +403,69 @@ private function resolveConfigPath() | |||
| return; | |||
| } | |||
|
|
|||
| if (null === $this->path) { | |||
| if (empty($this->path)) { | |||
There was a problem hiding this comment.
This is no longer possible I think?
0 length can only be with stdIn which is covered at line 400?
There was a problem hiding this comment.
$this->path could be an empty array when no path argument was passed via CLI
| $configDir = $this->cwd; | ||
| // path is directory | ||
| } elseif (1 < count($path)) { | ||
| throw new InvalidConfigurationException('For multiple paths config parameter is required.'); |
There was a problem hiding this comment.
Hi there, I cannot understand why this exception is thrown?
Basically when I try to run the fixer with multiple paths, it complains that I don't have a config. Why would I need a config in that case?
Thanks if you have any answer for that. I could work on a PR if that's a bug, but before I would like to understand.
There was a problem hiding this comment.
It was a design decision.
Basically, if you provide a path but don't provide a config file, it's localization is automatically discovered. if you provide multiple path, autodiscovering is extra tricky and could lead to discover multiple config files, while tool can only use one config per run.
There was a problem hiding this comment.
Thanks for the answer! That's not really in my interest ;) but at least I understand now.
To give some context I'm working on https://github.com/mnapoli/sniff What I want to do is basically the same thing as .php_cs except with a JSON file. My current approach is maybe not the best but at least I made it work, I run this:
php-cs-fixer path1 path2 … --config=null-config-file.php --rules=...
And here is the "null config file": https://github.com/mnapoli/sniff/blob/master/php-cs-null-config.php
There was a problem hiding this comment.
yep, just specify the config. i would even rename the file into standard .php_cs.dist
There was a problem hiding this comment.
Yes but projects using sniff should have a .sniff.json instead of .php_cs, which is why they don't have a config (but may have multiple paths).
That's why I wrote a "null config file" which returns an empty config: it's just so that I don't get the error For multiple paths config parameter is required.
Anyway I don't want to bore you with all that because that's my fault: I'm using php-cs-fixer in a weird way ;)
There was a problem hiding this comment.
I'm coming from #4024 (comment) and I didn't want to derail the other issue which is about something else.
you do not need to change this line, just point which config file to use via cli parameter
Apologies @keradus , I couldn't follow this one.
it's localization is automatically discovered
I also don't understand this part 😅 "localization" => text translation? 🤔 Nah, doesn't make sense.
So maybe this is about ambiguity of resolving possible multiple config files which isn't support, in case multiple files are provided to the fixer?
Because at least the --help output would signal to me multiple "paths" are supported:
Usage:
fix [options] [--] [<path>...]
I see that there is a dedicated --config and usually I see a fallback to .php_cs.dist/.php_cs. Apologies, I don't quite understand. And the reason I'm asking, I think it would a useful to simple provide multiple files on the command line to fix.
Thank you for your patience with me :)
There was a problem hiding this comment.
Was answered in #4024 (comment)
possible since creation of this github issue. just explicitly fingerpoint the config that should be used.
… and in fact like the message says, just use something like this; "ops":
find … | xargs -n … -P … ./php-cs-fixer.phar --config=…
Closes #1230
Replaces #1231
I still need to update readme file.