-
Notifications
You must be signed in to change notification settings - Fork 752
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
Properly warn for E262 with non breaking whitespaces #1035
Properly warn for E262 with non breaking whitespaces #1035
Conversation
pycodestyle.py
Outdated
@@ -131,7 +132,8 @@ def lru_cache(maxsize=128): # noqa as it's a fake implementation. | |||
'and', 'in', 'is', 'or'] + | |||
FUNCTION_RETURN_ANNOTATION_OP + | |||
ASSIGNMENT_EXPRESSION_OP) | |||
WHITESPACE = frozenset(' \t') | |||
NON_BREAKING_WHITESPACE = ' ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the escape sequence instead otherwise this code is very difficult to read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 6975080
pycodestyle.py
Outdated
@@ -1,4 +1,5 @@ | |||
#!/usr/bin/env python | |||
# -*- coding: utf8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this shouldn't be needed now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted the commit as we need the encoding for examples in docstrings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the examples in the docstrings aren't readable, I'd prefer writing \xa0
there as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you rather have no docstring exemples with NBSP or some hard to read one ? Because E262: x = x + 1 # \xa0Increment x
does not trigger E262 (a real one will).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's other examples where \t
is used similarly, putting E262: x = x + 1 # \xa0Increment x
(literally) into the docstring I believe gets the point across and is readable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a self test that check that the example are working apparently:
____________________________________________________________________________________ PycodestyleTestCase.test_selftest ____________________________________________________________________________________
self = <testsuite.test_all.PycodestyleTestCase testMethod=test_selftest>
def test_selftest(self):
fail_s, done_s = selftest(self._style.options)
self.assertTrue(done_s, msg='tests not found')
> self.assertFalse(fail_s, msg='%s failure(s)' % fail_s)
E AssertionError: 1 is not false : 1 failure(s)
testsuite/test_all.py:33: AssertionError
------------------------------------------------------------------------------------------ Captured stdout call -------------------------------------------------------------------------------------------
pycodestyle.py: failed to find E262:
x = x + 1 # \xa0Increment x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you remove the r
on that docstring it should fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in fe23aa2
pycodestyle.py
Outdated
@@ -131,7 +132,8 @@ def lru_cache(maxsize=128): # noqa as it's a fake implementation. | |||
'and', 'in', 'is', 'or'] + | |||
FUNCTION_RETURN_ANNOTATION_OP + | |||
ASSIGNMENT_EXPRESSION_OP) | |||
WHITESPACE = frozenset(' \t') | |||
NON_BREAKING_WHITESPACE = '\xa0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this constant isn't used anywhere but below -- inline it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8db2cad
to
6939343
Compare
If you rebase @Pierre-Sassoulas your tests should pass now that we're not testing against 2.7 |
6939343
to
59854e1
Compare
This is rebased/cleaned up ! Thank you for the review, and thank you for dropping python 2.7, not having to deal with that encoding issue is a relief 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #1034