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

Ignore option doesn't work. #66

Closed
Xety opened this issue Oct 6, 2015 · 14 comments
Closed

Ignore option doesn't work. #66

Xety opened this issue Oct 6, 2015 · 14 comments
Labels

Comments

@Xety
Copy link

Xety commented Oct 6, 2015

Hello,

I have the following settings :
wpbhk50

But it doesn't work :
screenshot_20

@steelbrain
Copy link
Contributor

Can we get a non-trimmed version of the first picture please?

@Xety
Copy link
Author

Xety commented Oct 6, 2015

Yes of course :
67tdtoh

@Arcanemagus
Copy link
Member

I'm actually not sure why that option is even in there. The documentation basically says it is meant for when you are running phpcs on an entire directory, but we are no matter what running on a single file.

I think what is going on here is that it sees the ignore option... but then ignores it since we explicitly told it to lint that file. In this case I would either put the code described in the link above at the top of the file, or change the type of the file to not be PHP (but then you lose syntax highlighting, so that is most definitely not ideal).

I'm going to close this issue and submit a PR soon to remove that option as I have no idea how it would ever be relevant in this usage.

@Xety
Copy link
Author

Xety commented Oct 6, 2015

This option worked fine 1/2 weeks ago. I remember it was me who added this option in a936699. Something has broke it.

@Arcanemagus
Copy link
Member

@Xety possibly broken on phpcs's side then? Nothing should have changed with the running of it here.

(I still don't see how that setting is relevant for a single file...)

@Arcanemagus Arcanemagus reopened this Oct 6, 2015
@Arcanemagus
Copy link
Member

Okay, did some testing here, it works on the command line with v2.3.4, but not within Atom under linter-phpcs. I'll see if I can figure it out.

@Xety
Copy link
Author

Xety commented Oct 6, 2015

I just tested previous version and
1.0.7 => work
1.1.0 => broke
1.1.x => broke

@Arcanemagus
Copy link
Member

It's a bug in phpcs, we moved to feeding it the file content via stdin to enable linting on the fly. The issue is that, even though we modify the text to tell it the file name, it still considers the file name to be "STDIN" for the purposes of the ignore test.

So the options are either:

  • Disable this option
  • Re-implement it locally to stop execution if a matching pattern is found
  • Disable linting on the fly

As I personally use lintOnFly I'm not going to disable that, which leaves us the first two options. Does anyone want to submit a PR for the second?

@skyrpex
Copy link
Contributor

skyrpex commented Oct 6, 2015

I could implement option two using isaacs/minimatch, for example. Should I?

@Arcanemagus
Copy link
Member

Go for it 😄, as long as it works properly I'd be happy to merge it.

@tchaffee
Copy link

tchaffee commented Oct 8, 2015

I had to turn off linter-phpcs because of this bug. Eagerly awaiting a fix.

@Arcanemagus
Copy link
Member

Fixed version published in v1.2.1, enjoy and please report any further issues you find!

@Xety
Copy link
Author

Xety commented Oct 13, 2015

It work well now, thanks !

@skyrpex
Copy link
Contributor

skyrpex commented Oct 13, 2015

👍

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

No branches or pull requests

5 participants