Skip to content
This repository has been archived by the owner on Jul 1, 2023. It is now read-only.

Support only checking the files you changed with the -h option #76

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

paladox
Copy link

@paladox paladox commented Sep 9, 2016

Fixes #75

@paladox
Copy link
Author

paladox commented Sep 9, 2016

@grogy hi, could I have help please? I doint know what to do here?

@@ -139,7 +139,7 @@ public static function parseArguments(array $arguments)
-m \
--first-parent \
--format=format: \
-- "${PATH_ARGS[@]}" | egrep -v '^$' || :";
-- "'*' . array('.php', '.phtml', '.php3', '.php4', '.php5')" | egrep -v '^$' || :";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do this line? It do concat string with array. I mean that a result of this expression have non-sence.

@grogy
Copy link
Contributor

grogy commented Sep 10, 2016

@paladox hello, I added comments to your commits. It is better now? Or what do you need know? :-)

@paladox
Copy link
Author

paladox commented Sep 10, 2016

I'm not sure what I'm doing, just guessing.

@@ -150,6 +164,10 @@ public static function parseArguments(array $arguments)
$settings->excluded[] = $arguments->getNext();
break;

case '-h':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will not work, because -h is used for help

@grogy
Copy link
Contributor

grogy commented Sep 10, 2016

@paladox you are in good way.. 👍 I like you that you tried send PR. Together we will finish it. :-)

@paladox
Copy link
Author

paladox commented Sep 10, 2016

Ok thank. I'm not sure what the next part is now, how do I get head to be checked first and then the files checked after?

and thanks for helping me

@grogy
Copy link
Contributor

grogy commented Sep 12, 2016

@paladox in my opinion is necessary changed (only in your use case) $iterator - you need iterate only in changed files. Do you understand me?

-m \
--first-parent \
--format=format: \
-- "'*' . array('.php', '.phtml', '.php3', '.php4', '.php5')" | egrep -v '^$' || :";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

egrep is deprecated use grep -E

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on second thought, you should probably refactor this to avoid use shell pipe formatting. what is you trying to filter out? can you give example? perhaps fixing --format or other arguments helps

otherwise filter it in php side. more readable

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP Parallel Lint is supported on Windows too and this does not work on this OS. So I think, this must be reimplemented in PHP.

@VasekPurchart
Copy link
Contributor

I don't think this is something PHPLint should be trying to solve, there too many moving underlying parts which would make this feature complicated and unreliable.

Also from the usecase point of view, I don't think this is something a separate tool should be trying to implement. Usually there are more different tools - coding standards, static analysis, ... and most of them can be "optimized" by checking only the changed files (at least all the tools where you need to work with single files only). So it makes sense to build the list only once and then give it to the separate tools. In this way all the tools will be analyzing exactly the same input, not relying on the implementation of the detection of each one of these. This can be achieved by reading the files from params, read them from input (which is supported), or if this is not enough maybe implement reading the file list from a file.

@glensc
Copy link
Contributor

glensc commented Oct 10, 2016

i think the tool should accept filenames to process from commandline, then you are free to customize as you like. git diff --name-only --diff-filter=..., for example:

PHP-CS-Fixer/README.rst#using-php-cs-fixer-on-ci

@JakubOnderka
Copy link
Owner

@paladox Sorry, but I don't think this will be ever merged. Because now it does not support Windows, it is Git only and the same behavior can be easily replaced with this command:

git show HEAD --name-only --no-commit-id --pretty="" | ./parallel-lint --stdin

Or I am wrong?

jrfnl added a commit to jrfnl/PHP-Parallel-Lint that referenced this pull request Dec 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants