PR: Avoid comments to generate pep8 warnings #4651
Conversation
Comment function : adds a space after the comment string (# for Python) Uncomment function : removes the comment string and removes one space if there is one
|
3rd time is the charm ... hopefully |
ccordoba12
changed the title
|
@Ericvulpi, thanks a lot for your contribution. @dalthviz, please review this one. |
|
|
||
| def uncomment(self): | ||
| """Uncomment current line or selection.""" | ||
| self.remove_prefix(self.comment_string) | ||
| self.remove_prefix(" ") | ||
|
|
||
| def __blockcomment_bar(self): | ||
| return self.comment_string + '='*(79-len(self.comment_string)) |
There was a problem hiding this comment.
@Ericvulpi I think that we need to change the line above like you proposed adding a space (' ') for the block comments. It should be something like:
return self.comment_string + ' ' + '=' * (78 - len(self.comment_string))You can test this with Ctrl+4 (blockcomment) and Ctrl+5 (unblockcomment). The change should make a block comment that starts with # ===... instead of #===... (note the space between the # and the = symbols).
There was a problem hiding this comment.
It was right under my nose! but i dont use block comments so didn't notice it
Added the block comment modification as suggested by dalthviz
|
@dalthviz, do you approve this one? |
|
@ccordoba12 I think the changes are ok, however I don't know the reason for the tests to fail. |
|
@Ericvulpi, please rebase this one on top of our latest 3.x branch to fix the test errors. Then I'll merge :-) |
|
|
||
| def __blockcomment_bar(self): | ||
| return self.comment_string + '='*(79-len(self.comment_string)) | ||
| return self.comment_string + ' ' + '=' * (78 - len(self.comment_string)) |
There was a problem hiding this comment.
It is because of the space added between the comment string and the '='. Maybe this could be rewritten as:
return self.comment_string + ' ' + '=' * (79 - len(self.comment_string + ' '))There was a problem hiding this comment.
No I mean why is 79 hardcoded!!, that is an adjustable setting
There was a problem hiding this comment.
Oh, then we should depend on that config here?, I think the 79 is from the maximum line length in PEP8
There was a problem hiding this comment.
@dalthviz is right, 79 is the default column width for pep8. This width can be configured for pycodestyle, but currently is not connected to our config system.
So let's leave 79 for now.
There was a problem hiding this comment.
@dalthviz please open an issue to keep track of this missing connection
|
Thanks @Ericvulpi for your contribution, this is going in! |
|
@ccordoba12 This patch breaks backwards compatibility when uncommenting scripts with comments in code blocks. eg. Script with old comment: Now uncomment with latest 3.2: Imagine a whole block of those with different block depths.(Maybe I this is a bad practice but I don't think I'm alone in this) I can't even shift-tab and tab back to replace the 3 spaces with 4 (maybe another bug here) I think pep8 errors are small compared to code errors introduced from uncommenting. I would suggest holding off on this until we get comments like this: This might break backward compatibility but at least I think this is waiting until 4.0. |
|
@bcolsen, thanks for your feedback. I think you're right, we need to revert this change. @dalthviz, please create a PR to revert this. @Ericvulpi, sorry for this, we'll fix this for good in Spyder 4. |
Fixes #1785.
Comment function : adds a space after the comment string (# for Python)
Uncomment function : removes the comment string and removes one space
if there is one