peterjc / flake8-black

flake8 plugin to run black for checking Python coding style
MIT License
165 stars 10 forks source link

Fix typo: min -> len #2

Closed sapphire-janrain closed 5 years ago

sapphire-janrain commented 5 years ago

Was having this error occasionally while running flake8 over my code:

Traceback (most recent call last):
  File "/usr/lib/python3.6/multiprocessing/pool.py", line 119, in worker
    result = (True, func(*args, **kwds))
  File "~/.local/lib/python3.6/site-packages/flake8/checker.py", line 669, in _run_checks
    return checker.run_checks()
  File "~/.local/lib/python3.6/site-packages/flake8/checker.py", line 608, in run_checks
    self.run_ast_checks()
  File "~/.local/lib/python3.6/site-packages/flake8/checker.py", line 504, in run_ast_checks
    for (line_number, offset, text, check) in runner:
  File "~/.local/lib/python3.6/site-packages/flake8_black.py", line 108, in run
    line, col = find_diff_start(source, new_code)
  File "~/.local/lib/python3.6/site-packages/flake8_black.py", line 33, in find_diff_start
    return min(len(old_lines), min(new_lines)), 0
TypeError: '<' not supported between instances of 'str' and 'int'

Just a quick fix for it.

peterjc commented 5 years ago

Excellent - You’ve found what looks like the exception I had last month, but regrettably didn’t keep the trace back or test case for:

https://github.com/peterjc/flake8-black/issues/1#issuecomment-454738949

Do you still have an example triggering this error for turning into a minimal test case?

Update: Fixed typo; as noted below I could reproduce this error

peterjc commented 5 years ago

Sorry, reviewing this on a phone - the fix looks right and I think trailing blank lines at the end of a file should trigger it. I’ll try making a small test case and merge your fix when so have time in front of a computer, hopefully this afternoon. Thank you!

Update: Missing final new line character triggers this, plus flake8 W292 no newline at end of file as well.

peterjc commented 5 years ago

Also, since I've not written down any policy on contributing yet, would you be happy to be thanked by name and/or GitHub username in the release notes (currently in the README file) and/or a new contributor listing?

peterjc commented 5 years ago

I managed to look at this before lunch - how's this update to the pull request which adds a test case, bumps the version ready to release this afternoon, and thanks you in the release history?

sapphire-janrain commented 5 years ago

Oh, thank you!

It may have been blank lines at the end of the file in my case, as I did have one at some point, but I'm not too sure. It would work when I did the default line length but fail when I changed the max line length to 100. Since this fixed it and it seemed simple enough I didn't care too much to investigate why that mattered, sorry.

Edit: I already cleaned up the original code after getting the report to work, so I'm not sure I could reproduce it, either.

peterjc commented 5 years ago

Merged, v0.0.3 should be out shortly - thank you!