-
Notifications
You must be signed in to change notification settings - Fork 749
Add whitespace checks for match
and case
#990
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
Conversation
pycodestyle.py
Outdated
@register_check | ||
def missing_whitespace_after_match_case(logical_line): |
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 would prefer if the existing checks were extended (this will provide better compatibility and performance)
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.
That was my initial idea. However, at least AFAIK it isn't that simple. As example, the best place would probably have been the whitespace_around_keywords
check. The issue there is that it also checks for whitespace before the keyword and that match
and case
are only soft keywords.
It might make sense to rename the new check to something like whitespace_around_soft_keywords
. That would at least provide options to extend it in the future.
Lines 473 to 494 in 61138ca
@register_check | |
def whitespace_around_keywords(logical_line): | |
r"""Avoid extraneous whitespace around keywords. | |
Okay: True and False | |
E271: True and False | |
E272: True and False | |
E273: True and\tFalse | |
E274: True\tand False | |
""" | |
for match in KEYWORD_REGEX.finditer(logical_line): | |
before, after = match.groups() | |
if '\t' in before: | |
yield match.start(1), "E274 tab before keyword" | |
elif len(before) > 1: | |
yield match.start(1), "E272 multiple spaces before keyword" | |
if '\t' in after: | |
yield match.start(2), "E273 tab after keyword" | |
elif len(after) > 1: | |
yield match.start(2), "E271 multiple spaces after keyword" |
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 don't see why you can't add it to the bottom of the code you've linked? renaming or adding functions leads to a bunch of annoying issues created by operating system distributors that intentionally and erroneously violate dependency constraints so I'd really like to avoid adding / renaming functions when possible
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.
Ok, done.
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 caused #1024 -- I'm just going to revert this for now -- we can potentially add it back later |
It also fixes the regression PY-51141 caused by PyCQA/pycodestyle#990 that we integrated hastily for the initial pattern matching support. GitOrigin-RevId: 71742859eb8909214cb5f62247787ccf9a718684
It also fixes the regression PY-51141 caused by PyCQA/pycodestyle#990 that we integrated hastily for the initial pattern matching support. (cherry picked from commit 71742859eb8909214cb5f62247787ccf9a718684) IJ-CR-18999 GitOrigin-RevId: da795b01f9dc03bc36730b63612eb3540d45ad5e
E271 multiple spaces after keyword
andE275 missing whitespace after keyword
should also also be emitted formatch
andcase
. However, since they are only soft keywords, some additional logic is needed.If accepted and the next version is released, this check could be added to flake8 as well (flake8/setup.cfg).