Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix usage of parameter --standard #16

Closed
wants to merge 1 commit into from

Conversation

das-peter
Copy link

This is a follow up of this pull request: pear#3 (comment)

Point Nr. 1 from the previous pull request was already fixed in latest dev - thanks :)

Point Nr. 2 the usage of the parameter --standard still needs some work. I've added a description of the issue and the way I decided to solve it to the commit itself: das-peter@ad0926b#commitcomment-953994

Thanks in advance & cheers
Peter

@gsherwood
Copy link
Member

Thanks a lot. I understand the scenario now and will look at applying your patch to correct this situation.

@gsherwood
Copy link
Member

There are 2 issues here that you are fixing, both caused by a standard designed to be installed in PHPCS itself being used outside PHPCS. This is obviously something that I want to support, so thanks again for taking the time to investigate and provide a patch.

The first is in _expandRulesetReference, where a sniff code cannot be converted into a file path because PHPCS assumes all sniff codes are for sniffs installed inside PHPCS itself. The change you provided works well. It wont help standards reference other standards, but that's a bigger issue. It's not needed here, but I just wanted to be clear for anyone reading this.

The second is the autoloader. Standards that are installed inside PHPCS can extend other sniffs installed inside PHPCS without having to include anything. If a standard is using sniffs outside PHPCS, it would obviously need to include the class files itself. The Drupal CS doesn't do this because it is designed exclusively to run inside PHPCS. It could include the class files using relative paths to allow it to run both inside and out, or the autoloader can be changed to accomodate this. I didn't really want to change the autoloader and I didn't want to change it to have to get the CLI command values. In the end, I decide to change the autoloader so that existing standards dont need to be changed, but I made $standardDir static (there will only ever be one phpcs anyway) so that the check is simpler and faster.

I've pushed these changes now and have tested with the latest drupalcs download. Everything seems to work fine for me, but please have a look if it works for you.

Commit info is here: 9ce8010

@das-peter
Copy link
Author

Awesome! Just tested it and it works like a charm.
Thank you very much.

@das-peter das-peter closed this Feb 24, 2012
@gsherwood
Copy link
Member

Good news. Thanks a lot for testing it and getting back to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants