-
Notifications
You must be signed in to change notification settings - Fork 61
Support only checking the files you changed with the -h option #76
base: master
Are you sure you want to change the base?
Conversation
@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 '^$' || :"; |
There was a problem hiding this comment.
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.
@paladox hello, I added comments to your commits. It is better now? Or what do you need know? :-) |
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': |
There was a problem hiding this comment.
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
@paladox you are in good way.. 👍 I like you that you tried send PR. Together we will finish it. :-) |
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 |
@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 '^$' || :"; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
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. |
i think the tool should accept filenames to process from commandline, then you are free to customize as you like. |
@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:
Or I am wrong? |
Fixes #75