Running a --diff from standard input looks at files anyway

This is probably actually a feature, but the help doesn't mention this, so I consider it a bug.

I run the PEP-8 checker in the context of a git hook, since I don't want to commit any PEP-8 mistakes. Here's how it works:

#!/bin/bash
git diff --cached | pep8 --diff

I only want to have the version of the file checked that is in the diff. And according to the help string, this is exactly what I need, because

--diff               report only lines changed according to the unified diff
                     received on STDIN

This is important, because I stage different things to git than reside of the filesystem.

For example, suppose I have this file I'm adding:

def f(x):
    return x + 1


def g(x):
    return x - 1

It looks good, so I stage it. But then, I take a side-trip to test something else, which requires a new function:

def f(x):
    return x + 1

def test(x): return "stuff"

def g(x):
    return x - 1

That contains a PEP-8 error -- a PEP-8 error I have no intention of committing.

I forget to clean up this change before I commit, but that's okay. PEP-8 will only look at the diff, right? And the diff looks like this:

@@ -0,0 +1,5 @@
+def f(x):
+       return x + 1
+
+
+def g(x):
+       return x - 1

So I should be good to go, right?

Wrong:

test.py:4:1: E302 expected 2 blank lines, found 1

If I rename test.py to something else, then I get the behavior I expect: only the lines in the diff are checked. But apparently, PEP-8 silently detects if the file exists first, and does the entire thing anyway. (Or maybe tries to apply the patch to it? Can't tell.)

Ideally, I'd like a flag or config option to control this behavior, which I can then turn off to fix my problem. But at the very least, please rework the help string to indicate this silently happens.

[sigmavirus24]

So looking at the docs, I see this:

  --diff               report only lines changed according to the unified diff
                       received on STDIN

Note that it says "only lines changed according to the unified diff". I haven't looked at the implementation, but I'm fairly certain that this means it only reports errors on lines in the ranges of the unified diff. (Diffs include line number information.) In other words, pep8 analyzes the whole file (because that's how it works) and only prints errors for line numbers included in the diff. So by putting your test function definition smack in the middle, pep8 is doing exactly what it's advertising. There's no good way for pep8 to only analyze stuff from a diff. It must analyze the file and only print errors for lines that are in the diff and the easiest way to do that is by checking line numbers.

I'm pretty sure this is not a bug. You may consider it a feature request, but I would consider it an unreasonable one unless you're willing to (in a backwards compatible way) make this work as your mental model dictates.

So it checks line numbers from to the diff. Makes sense. Don't I feel smart. I'll close this.

Though I would still suggest tweaking the docstring to put the word "numbers" in there, unless you think I'm unique in my literal-mindedness.

[sigmavirus24]

No I don't. You also don't have to close this. I didn't even check the implementation, but that's my guess for how it's implemented. Would you mind sending a PR to update the help string? I think it would be accepted if it were around (and that is the way the --diff flag works).

[sigmavirus24]

To satisfy my curiousity, I checked and that is how it's implemented. (Reference: https://github.com/jcrocholl/pep8/blob/435d1cbf995a659a82d1d4b42d25e3459556ef21/pep8.py#L1770)