Set default path mode to intersection #371
Conversation
Landerstraeten
commented
Jun 16, 2017
•
Landerstraeten
commented
| Q | A |
|---|---|
| Branch | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | yes |
| Deprecations? | no |
| Documented? | yes |
| Fixed tickets | / |
|
Hi @Landerstraeten, Thanks for your PR! I've tested the PR and found out that during I think it's best to fix this issue by removing the ability to configure the path modus and hardcode it:
In the last case, it's also an option to implement this issue: @keradus : Can you confirm that running intersection path mode with empty path arguments is causing an empty set to validate? |
|
I can confirm. intersection is basically operation of sets. when one take common part of anything and empty set, it's always empty set |
|
@veewee , hardcoded path mode would actually follow reasoning why we introduced intersection. |
disagree, public argument was removed |
|
It is indeed a BC break, I've changed the description and the milestone. |
| @@ -91,6 +86,8 @@ public function run(ContextInterface $context) | |||
| return $this->runOnAllFiles($context, $arguments); | |||
| } | |||
|
|
|||
| $arguments->add('--path-mode=intersection'); | |||
There was a problem hiding this comment.
it will crash with Cannot create intersection with not-fully defined Finder in configuration file. exception if there is no finder configured in .php_cs(.dist)? file.
There was a problem hiding this comment.
How would you suggest to solve it?
There was a problem hiding this comment.
you cannot know it without executing config file.
There was a problem hiding this comment.
We could execute the config file, but this won't be able if php-cs-fixer is ran in PHAR modus. In that case, the PhpCsFixer\Config won't be available.
I do understand why it crashes, but isn't it more logical if it runs in intersect modus without a finder, it runs on the specified files only? However, this would require a change in php-cs-fixer2.
There was a problem hiding this comment.
sorry, not sure what are modus
We could execute the config file, but this won't be able if php-cs-fixer is ran in PHAR modus.
not true. this is not related do you execute code under phar or not.
isn't it more logical if it runs in intersect modus without a finder
no, it intersect you need to have at least two collections. here, it's cli args + finder from config.
There was a problem hiding this comment.
As you mentioned, currently the best option is to handle errors with intersection and an empty finder.
and that means you need to load config from file and change it, it finder is not fully configured.
but that, again, would require grumphp to understand the code it's executing (know the classes etc), which, as other solutions I suggested, you refused.
you want to be aware of configuration but refuse to know it classes. not much you can do with that restriction (except run cli up to 2 times approach)
There was a problem hiding this comment.
Hi @keradus ,
It's not that I refuse to know it classes, it's about some configurations using the globally installed php-cs-fixer. For locally installed fixers it wouldn't be a problem.
Instead of running the cli tool twice, it is also possible to make the path mode configurable again, but this time with a boolean that looks like intersect_on_commit with a default value of true. That would probably also fix this problem.
There was a problem hiding this comment.
you could know the classes and still execute phar from cli.
that new mode is a hacky way of doing this which abuse package that is external for you, putting special mode only for one user is no-go.
and that option is not intersect_on_commit, it's more like intersect_wich_finder_from_config_but_ignore_if_there_is_no_finder_in_config... hacky
There was a problem hiding this comment.
The only way we could know these classes is by adding php-cs-fixer to the dependencies, even for projects who don't want to use that tool. So this is something I would like to avoid. Or are you talking about some other way?
To be sure we are talking about the same thing: I was talking about a configuration inside grumphp named intersect_on_commit which will use the intersect path mode during a commit inside GrumPHP. There is no need for any changes in php-cs-fixer for that.
There was a problem hiding this comment.
I was talking about a configuration inside grumphp named intersect_on_commit which will use the intersect path mode during a commit inside GrumPHP. There is no need for any changes in php-cs-fixer for that.
Cool it's gonna be internal setting.
yet, when intersect_on_commit would be set up, then you will end up with current issue as well, key is configured, intersection is tried to be used, yet no local config finder is provided.
Or are you talking about some other way?
if you don't want to have that as dependency and treat it as a blackbox, then:
poor, quick and dirty solution would be to execute command with intersection mode, if it crashes on not fully defined finder, run it with override mode...
|
Fixed in #395 |
