Skip to content
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

PR: Avoid comments to generate pep8 warnings #4651

Merged
merged 3 commits into from
Jun 30, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions spyder/widgets/sourcecode/codeeditor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2258,14 +2258,15 @@ def toggle_comment(self):

def comment(self):
"""Comment current line or selection."""
self.add_prefix(self.comment_string)
self.add_prefix(self.comment_string + " ")

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))
return self.comment_string + ' ' + '=' * (78 - len(self.comment_string))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that hardcoded 78?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 + ' '))

Copy link
Member

@goanpeca goanpeca Jun 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I mean why is 79 hardcoded!!, that is an adjustable setting

screen shot 2017-06-28 at 12 52 03

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, then we should depend on that config here?, I think the 79 is from the maximum line length in PEP8

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dalthviz please open an issue to keep track of this missing connection


def transform_to_uppercase(self):
"""Change to uppercase current line or selection."""
Expand Down