PyLint Comments Work With Other Linter Comments #2537
Conversation
This is the solution for [pylint-dev#2297](pylint-dev#2297), [pylint-dev#2470](pylint-dev#2470), and [pylint-dev#2485](pylint-dev#2485).
This is the solution for [pylint-dev#2297](pylint-dev#2297), [pylint-dev#2470](pylint-dev#2470), and [pylint-dev#2485](pylint-dev#2485).
This is the solution for [pylint-dev#2297](pylint-dev#2297), [pylint-dev#2470](pylint-dev#2470), and [pylint-dev#2485](pylint-dev#2485).
Added Devyn Collier Johnson
Added entry for the solution for [pylint-dev#2297](pylint-dev#2297), [pylint-dev#2470](pylint-dev#2470), and [pylint-dev#2485](pylint-dev#2485).
There was a problem hiding this comment.
@DevynCJohnson Thanks for the effort. I don't think though that this PR actually solves anything other than a line-too-long potential issue. I'm not sure how are you reproducing the bugs, as I cannot reproduce them locally using the latest. I think that for now we should remove the separate changes and bring in just the change in format.py using the re.sub approach instead of breaking every comment.
| @@ -1260,8 +1260,13 @@ def check_line(line, i): | |||
| _msg_id.strip() for _msg_id in back_of_equal.split(",") | |||
| }: | |||
| return None | |||
| line = line.rsplit("#", 1)[0].rstrip() | |||
|
|
|||
| # Find the Pylint comment among all other special comments on the line | |||
There was a problem hiding this comment.
I think a better solution would be to use re.sub:
line = OPTION_RGX.sub("", line)
| @@ -814,7 +814,13 @@ def process_tokens(self, tokens): | |||
| match = utils.OPTION_RGX.search(content) | |||
| if match is None: | |||
| continue | |||
|
|
|||
| # Find the Pylint comment among all other special comments on the line | |||
There was a problem hiding this comment.
This doesn't make sense, OPTION_RGX should be enough to find the corresponding pragma control. It also assumes that we only have disable, which is not the case.
| @@ -81,7 +81,7 @@ | |||
| # Allow stopping after the first semicolon/hash encountered, | |||
| # so that an option can be continued with the reasons | |||
| # why it is active or disabled. | |||
| OPTION_RGX = re.compile(r"\s*#.*\bpylint:\s*([^;#]+)[;#]{0,1}") | |||
| OPTION_RGX = re.compile(r'.*#\s*pylint:\s*([^;#]+)[;#]?.*') | |||
There was a problem hiding this comment.
I think replacing .* with \s* from after the hash makes sense, but not the rest of the changes.
| @@ -7,6 +7,16 @@ What's New in Pylint 2.2? | |||
|
|
|||
| Release date: TBA | |||
|
|
|||
| * PyLint comments now officially work with other linter comments | |||
|
|
|||
There was a problem hiding this comment.
Are you sure your PR actually fixes all these bugs? From what I'm noticing:
- Regression on
# pylint: disablecomment #2297 is already fixed in master - PyLint Ignore comments do not work if Flake8 Comment is Present #2470 is also fixed, and I cannot reproduce with your given example
- as well as # pylint: … line comments do not play well with other code analyzers (they have to appear at the end) #2485, which again I cannot reproduce using your example from # pylint: … line comments do not play well with other code analyzers (they have to appear at the end) #2485 (comment).
Unless you are testing with a completely different code than the one posted in the aforementioned comment, I don't think your PR "fixes" these issues, as they are already fixed. What instead is fixing is that it correctly supports long lines and additional disables, but that's a separate bug.
|
Closing as this is working as intended right now. |
Steps
doc/whatsnew/<current release.rst>.Description
This is the solution for #2297, #2470, and #2485. The code previously would split the the comments into a list and grab the first item without checking that it is grabbing the PyLint comment.
Type of Changes
Related Issue
# pylint: disablecomment #2297