[sigmavirus24]

Here we update the logic in the StyleGuide to allow users to specify both
--select and --ignore to combine the power of both flags. Take for
example:

$ pep8 --select=E1 --ignore=E112 testsuite/E11.py

The example shows that we are now allowed to filter out E112 while
selecting the other E1xx checks.

A (potentially unfortunate) consequence of this change is that if a user
specifies --select without --ignore and has codes ignored in their
config file, then we will merge that into the options and continue to
ignore those codes.

Closes #390

---

The tests are going to fail, but I wanted to throw this up for @glyph and others to test.

[sigmavirus24]

So right now this accommodates the use case in the commit message, select something broad but ignore the rest. It doesn't, however allow for the inverse, e.g.,

pep8 --select=E24 --ignore=E

I'm not sure we need to either. Semantically that's the same thing as

pep8 --select=E24

That said, it doesn't behave exactly the same way. Take for example

~/s/pep8 git:bug/390 ❯❯❯ python pep8.py --ignore=E --select=E1 testsuite/W19.py
~/s/pep8 git:bug/390 ❯❯❯ python pep8.py --select=E1 testsuite/W19.py
testsuite/W19.py:12:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:16:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:19:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:21:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:24:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:26:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:31:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:32:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:35:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:38:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:43:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:44:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:45:2: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:52:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:54:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:57:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:58:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:62:2: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:63:2: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:64:2: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:65:2: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:71:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:73:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:77:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:78:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:81:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:83:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:92:2: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:99:2: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:106:1: E101 indentation contains mixed spaces and tabs
testsuite/W19.py:125:1: E101 indentation contains mixed spaces and tabs

The question I have is, do we want to pre-process these options to check for problems like this and warn the user or just silently do "the right thing"? Technically we've been doing what someone thought was the right thing for a while now and it's actually been not very intuitive. I'd love to get some feedback on this before proceeding.

[doismellburning]

The question I have is, do we want to pre-process these options to check for problems like this and warn the user or just silently do "the right thing"?

The behaviour you described, where --ignore=E --select=E1 allows a file with E101 issues to pass, feels surprising to me.

I feel like "most specific wins" is the least astonishing behaviour here, so for a file with E101 issues (pseudocode):

• pep8 -> fail
• pep8 --ignore E -> pass
• pep8 --select E -> fail
• pep8 --ignore E --select E1 -> fail
• pep8 --select E --ignore E1 -> pass
• pep8 --select E --ignore E -> helpful error message

et cetera

Naturally I've not yet looked into the practicalities of tests / implementation here, but I'd be interested in your views please!

[sigmavirus24]

The behaviour you described, where --ignore=E --select=E1 allows a file with E101 issues to pass, feels surprising to me.

Right. I feel the same. To me, I'm thinking that we should say "well, the selected code(s) is(are) a subset of ignore, we should just use the generic --select logic and discard the ignore values". Make sense?

[doismellburning]

"well, the selected code(s) is(are) a subset of ignore, we should just use the generic --select logic and discard the ignore values"

I've played around with this in a few incarnations, and I'm not sure I follow what you mean, sorry.

What would you expect the following to do on a file with E101 and E102 errors:

$ pep8  --select E --ignore E1 --select E10 --ignore E101
$ pep8  --ignore E --select E1 --ignore E10 --select E101

[sigmavirus24]

So I wouldn't expect repeated usage of --select and --ignore to produce anything coherent. Let me break it down into 4 distinct examples though:

1. pep8 --select E --ignore E1 I expect pep8 to only report on lines with codes starting with E but not E1 (e.g., E2xx, E3xx, etc.)
2. pep8 --select E10 --ignore E101 If we had something like E102 I would expect that to be the only code shown, since E101 is the only E10 class code, I expect nothing to be printed. If this was something like pep8 --select E11 --ignore E111 I'd expect E112, E113, ..., E116 to all be shown if present.
3. pep8 --ignore E --select E1 I would expect this to work the same as pep8 --select E1 does now
4. pep8 --ignore E10 --select E101 I would expect this to work the same as pep8 --select E101 does nwo

[doismellburning]

Ah, I think I've realised my issue here - I'm interpreting select as analogous to "enable" or "allow" rather than "choose" or "pick" (i.e. assuming everything is enabled by default, using select without ignore would be a noop)

I should have realised from the behaviour you demonstrated in #433 (comment) - my bad!

I'm'a take some time to try fix my mental model ;) Thanks!

[sigmavirus24]

@doismellburning no worries! --select has weird mental models for different people. It's hard to explain it correctly all the time.

[IanLee1521]

@sigmavirus24 -- Other than the merge conflicts, do you believe this PR is mergeable?

[IanLee1521]

I suspect you might also have new / more insight now from your work with flake8

[sigmavirus24]

This could be merged. I'm less inclined to work on the rebase given that the functionality is included in Flake8 in a significantly better way that's far more powerful.

I can rebase this, if people really want it here. Previously Flake8 needed it here but 3.0 makes it unnecessary.

[IanLee1521]

I think it would be valuable for those that don't use pycodestyle by way of flake8. I'll take it myself to update the pull request and merge it.

[sigmavirus24]

I think it would be valuable for those that don't use pycodestyle by way of flake8. I'll take it myself to update the pull request and merge it.

Maybe but pycodestyle only had that bug reported by Flake8 users who were redirected here. Further, this is something of a breaking change that should have only been included in 2.0. So unless you're eager for a 3.0, I don't think this is a change you should deliver any time soon.