phpro / grumphp

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

[919] Add logic to scan the current directory and add any files that … #920

Closed jklmnop closed 2 years ago

jklmnop commented 2 years ago
Q A
Branch master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Documented? yes
Fixed tickets #919

Unfortunately, this will not work until friendsoftwig/twigcs changes how they exclude files, but there is a PR for it.

see: https://github.com/friendsoftwig/twigcs/issues/183 see: https://github.com/friendsoftwig/twigcs/pull/184

This bit of code will compare the files that have changed with the files in the project and add non-matching file paths to the exclude config.

New Task Checklist:

veewee commented 2 years ago

Since twigcs does not support this yet, wouldn't it make sense to work with an include list instead of an exclude? That way we could just pass the list of all changed twig files instead of adding some dark magic exclude logic :)

jklmnop commented 2 years ago

@veewee Absolutely, but TwigCS does not support StdIn or an "include" list at this moment, so I'm just trying to work with what they do have. 🤦‍♀️

I do not think this is the best solution for the problem, but as of now, I think it's the ONLY solution with the options they provide (and my patch to their project). I don't think it should even make it into master.

At least there is a commit someone can reference with the dependency in composer.json in the event they need this problem solved too. Failing commits for unchanged files is a huge bummer and would mean we couldn't reliably use the tool without a bunch of unnecessary noise and false positive failures and git commit -n.

I'm hoping they'll at least change the strategy for exclusion, and better yet, maybe someone much more knowledgable than me could implement a StdIn/include strategy a la PHPCS. Best I could do right now is change which method they're calling to scan for excludes in TwigCS coupled with the "dark magic 🧙‍♂️" in this commit for GrumPHP.

veewee commented 2 years ago

Hello @jklmnop

Thanks for investigating this. I am closing this PR for now. Feel free to provide a new PR if TwigCs decides to support this and you got a working PR.

jklmnop commented 2 years ago

Thank you so much, @veewee! Eventually, I realized that a patch might be a better option since I was losing out on any updates to master without maintaining this PR. (Here's that patch for anyone who finds themselves here)

I might take a crack at making TwigCs accept a piped list of files a la PHP_CodeSniffer and other projects. Hoping if they do eventually support it, there won't be a need to update GrumPHP!