Edit report at https://pear.php.net/bugs/bug.php?id=19648&edit=1
ID: 19648
Updated by: [email protected]
Reported By: [email protected]
Summary: Ignore patterns no longer work as expected in 1.4
-Status: Feedback
+Status: Closed
Type: Bug
Package: PHP_CodeSniffer
Operating System: Linux
Package Version: 1.4.0
PHP Version: 5.3.0
Assigned To: squiz
Roadmap Versions:
New Comment:
-Status: Feedback
+Status: Closed
This change has now been released.
Previous Comments:
------------------------------------------------------------------------
[2012-10-11 22:32:34] squiz
No, the latest code is not yet available in PEAR. Only from the git
repo. If you dont
have git installed and can't run those commands I posted, email me
(gsherwood at
squiz dot net) and I'll package up a custom PEAR release for you.
And I guess we think alike, because the XML tag attribute is exactly how
I ended up
implementing the change. All ignore paths are absolute by default, but
you can set
them to relative if you want:
<exclude-pattern type="relative">^/tests/*</exclude-pattern>
<exclude-pattern type="relative">^/data/*</exclude-pattern>
And you can obviously mix absolute and relative paths together.
As for the memory change I implemented, it is only for the summary
report. The
biggest issue with memory over a large code base is storing the messages
themselves in memory. The files, for example, never remain in memory
unless you
are using a multi-file sniff (like the included sample one Generic
DuplicateClassNameSniff, or a custom one you've built). The summary
report doesn't
show the messages themselves, so it doesn't need to store the messages.
It now just
stores a counter for each file, reducing memory significantly.
So if you're using 4GB of memory, you've either got more errors than
I've ever seen
before (I can store about 30k in less than 200MB) or you are are using
multi-file
sniffs over a large code base, which I would recommend removing.
Hope that helps.
------------------------------------------------------------------------
[2012-10-11 20:21:31] droch_trulia
Is the latest branch available through pear?
Thanks for fixing that.
To give you more background info, the reason we specify individual paths
like that is
because I had to break up the phpcs ant task into 4 parts because phpcs
was eating
through more than 4GB of memory running over our whole repo (I have a
post
phpcs process that merges the 4 output files into one).
I noticed that one of the versions reduced the memory footprint as well,
so we may
not need to do it that way anymore (i've also since reduced the
ruleset).
As an alternative to your recent change to fix the issue we have, could
you have an
option that specifies whether the ignore pattern is checked on relative
or absolute
paths? That might be flexible to all scenarios.
Perhaps something like:
<exclude-pattern path="absolute">*/js/yui*.js</exclude-pattern>
and
<exclude-pattern path="relative">^yui*.js</exclude-pattern>
And then just default to the most common case.
------------------------------------------------------------------------
[2012-10-11 05:03:10] squiz
I've reverted PHPCS back to using absolute paths. If you can, please
grab the latest
code from the git repo and take a look:
git clone git://github.com/squizlabs/PHP_CodeSniffer.git
cd PHP_CodeSniffer
php scripts/phpcs --ignore=*/js/yu*.js
/path/to/build/project/include/js
Or revert back your my_std.xml file and use that standard instead.
------------------------------------------------------------------------
[2012-10-11 04:21:10] squiz
(Sorry for this long winded reply. This is also for my own notes if I
ever have to
come back here.)
When you run your command like this:
phpcs --standard=my_std.xml project/include/js
PHPCS is going to see the yui_history.js file without any proceeding
path details. So
your ignore regex is going to have to be ^yui*.js or *yui*.js or just
yui*.js, as you
discovered originally.
Is this how you always run PHPCS or is this just a one-off check of this
directory? I
am assuming this is just a command you've run and you find the ignore
pattern
otherwise works fine.
If so, I understand the issue you are having because your my_std.xml
file obviously
contains ignore patterns that work when you check your whole project but
they wont
necessarily work when manually checking sub-directories due to the
relative path
change. But in this particular case, it is only if you are directly
checking a directory
that has an ignore rule. For example, checking a 'js' directory when the
ignore rule is
'*/js/...' or checking a 'build' directory when you have an ignore rule
'*/build/...'.
In these case, it is best to include 2 ignore paths to cover all the
cases:
1. */js/yui*\.js for any yui files inside js dirs anywhere in your
project
2. ^(js/)?yui*\.js for when checking the include dir or the js dir
directly
You could merge these 2 ignore paths into ^((js|*/js)/)?yui[^/]+\.js
That covers most things, but still has a problem in that it will ignore
any
yui[something].js file if it exists directly under the directory you are
checking. It can't
enforce the fact that it has to be in a directory called js without
having access to the
full path. And that is where the real problem is here I think.
So what I'd like to do is change the ignore path stuff so that it is
back to how it used
to be. That means changing it to be based on the absolute path instead
of the
relative one. I'll then need to add an option for ruleset.xml files so
they can specify a
ignore rule as being relative.
------------------------------------------------------------------------
[2012-10-11 02:25:56] squiz
Yes, the relative checks *are* relative to the directory on the command
line. Where
you run PHPCS from is not used.
If you look at the pull request, you can see why the change was made and
the
specific case that required this change. I assure you, they are still
useful, but now
also much more portable for large projects.
Thanks a lot for getting back to me so quickly, and for providing the
directory
structure. I will take a look at what pattern is needed to exclude that
file and get
back to you.
Don't worry; this isn't something I'm trying to close off without
resolving. I'll figure
out what the patterns are doing in your code and fix any issues I find.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
http://pear.php.net/bugs/bug.php?id=19648
