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
Open

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
850c73d
Update Settings.php
paladox Sep 9, 2016
43a45c6
Update Settings.php
paladox Sep 9, 2016
c6f9ef4
part 2
paladox Sep 9, 2016
27dbaec
Update
paladox Sep 10, 2016
68af75d
Update
paladox Sep 10, 2016
68b1de8
Update Manager.php
paladox Sep 10, 2016
a39753a
Update Manager.php
paladox Sep 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function run(Settings $settings = null)

$output->writeHeader($phpExecutable->getVersionId(), $settings->parallelJobs, $phpExecutable->getHhvmVersion());

$files = $this->getFilesFromPaths($settings->paths, $settings->extensions, $settings->excluded);
$files = $this->getFilesFromPaths($settings->paths, $settings->extensions, $settings->excluded, $settings->OnlyFilesChanged);

if (empty($files)) {
throw new Exception('No file found to check.');
Expand Down Expand Up @@ -144,10 +144,11 @@ protected function gitBlame(Result $result, Settings $settings)
* @param array $paths
* @param array $extensions
* @param array $excluded
* @param array $OnlyFilesChanged
* @return array
* @throws NotExistsPathException
*/
protected function getFilesFromPaths(array $paths, array $extensions, array $excluded = array())
protected function getFilesFromPaths(array $paths, array $extensions, array $excluded = array(), array $OnlyFilesChanged = array() )
{
$extensions = array_flip($extensions);
$files = array();
Expand All @@ -157,8 +158,12 @@ protected function getFilesFromPaths(array $paths, array $extensions, array $exc
$files[] = $path;
} elseif (is_dir($path)) {
$iterator = new \RecursiveDirectoryIterator($path, \FilesystemIterator::SKIP_DOTS);
if (!empty($excluded)) {
$iterator = new RecursiveDirectoryFilterIterator($iterator, $excluded);
if (!empty($excluded) && !empty($OnlyFilesChanged)) {
$iterator = new RecursiveDirectoryFilterIterator($iterator, $excluded, $OnlyFilesChanged);
} elseif (!empty($OnlyFilesChanged) ) {
$iterator = new RecursiveDirectoryFilterIterator($iterator, $excluded, $OnlyFilesChanged);
} elseif (!empty($excluded)) {
$iterator = new RecursiveDirectoryFilterIterator($iterator, $excluded, $OnlyFilesChanged);
Copy link
Contributor

Choose a reason for hiding this comment

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

What about if you get $onlyFilesChanged array then create iterator only with those files (and don't use files from $iterator)?

}
$iterator = new \RecursiveIteratorIterator(
$iterator,
Expand Down Expand Up @@ -191,15 +196,19 @@ class RecursiveDirectoryFilterIterator extends \RecursiveFilterIterator
/** @var array */
private $excluded = array();

/** @var array */
private $OnlyFilesChanged = array();
Copy link
Contributor

Choose a reason for hiding this comment

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

What about use other name for this class variable? What about something like $changedFiles?


/**
* @param \RecursiveDirectoryIterator $iterator
* @param array $excluded
*/
public function __construct(\RecursiveDirectoryIterator $iterator, array $excluded)
public function __construct(\RecursiveDirectoryIterator $iterator, array $excluded, array $OnlyFilesChanged)
{
parent::__construct($iterator);
$this->iterator = $iterator;
$this->excluded = array_map(array($this, 'getPathname'), $excluded);
$this->OnlyFilesChanged = array_map(array($this, 'getPathname'), $OnlyFilesChanged);
}

/**
Expand Down
18 changes: 18 additions & 0 deletions src/Settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ class Settings
*/
public $phpExecutable = 'php';

/**
* Git executable
* @var string
*/
public $OnlyFilesChanged = 'git';
Copy link
Contributor

Choose a reason for hiding this comment

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

This project use camel case. Please, can you change it to onlyFilesChanged?

And second think in this line - why the class variable have value git? I don't understand to a meaning.

Copy link
Author

Choose a reason for hiding this comment

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

Since I'm not sure what I put as the value, do I leave it blank? like ''


/**
* Check code inside PHP opening short tag <? or <?= in PHP 5.3
* @var bool
Expand Down Expand Up @@ -126,6 +132,14 @@ public static function parseArguments(array $arguments)
{
$arguments = new ArrayIterator(array_slice($arguments, 1));
$settings = new self;

$git = "git show HEAD \
--name-only \
--diff-filter=ACM \
-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.

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

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.


foreach ($arguments as $argument) {
if ($argument{0} !== '-') {
Expand All @@ -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

$settings->OnlyFilesChanged = exec($git);
break;

case '-e':
$settings->extensions = array_map('trim', explode(',', $arguments->getNext()));
break;
Expand Down