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

SpaceConsistencyBear: Add leading_blankline option #2222

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

khanchi97
Copy link
Member

Add allow_trailing_blanklines option for whether to allow
leading blanklines in starting of a file.

Closes #2207

line_nr_end = False
if not allow_leading_blanklines:
enumerated_zip_obj = zip(range(1, len(file) + 1),
file)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp6ju9xmm6/bears/general/SpaceConsistencyBear.py
+++ b/tmp/tmp6ju9xmm6/bears/general/SpaceConsistencyBear.py
@@ -45,7 +45,7 @@
             line_nr_end = False
             if not allow_leading_blanklines:
                 enumerated_zip_obj = zip(range(1, len(file) + 1),
-                                             file)
+                                         file)
                 enumerated_tuple = tuple(enumerated_zip_obj)
 
                 for line_number, line in enumerated_tuple:

result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file[start_line_of_file - 1:], start = start_line_of_file):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp6ju9xmm6/bears/general/SpaceConsistencyBear.py
+++ b/tmp/tmp6ju9xmm6/bears/general/SpaceConsistencyBear.py
@@ -84,7 +84,7 @@
                 result_texts = []
                 additional_info_texts = []
 
-        for line_number, line in enumerate(file[start_line_of_file - 1:], start = start_line_of_file):
+        for line_number, line in enumerate(file[start_line_of_file - 1:], start=start_line_of_file):
             replacement = line
 
             if enforce_newline_at_EOF:

line_nr_end = False
if not allow_leading_blanklines:
enumerated_zip_obj = zip(range(1, len(file) + 1),
file)
Copy link
Collaborator

Choose a reason for hiding this comment

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

E127 continuation line over-indented for visual indent

Origin: PycodestyleBear (E127), Section: autopep8.

result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file[start_line_of_file - 1:], start = start_line_of_file):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E251 unexpected spaces around keyword / parameter equals

Origin: PycodestyleBear (E251), Section: autopep8.

result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file[start_line_of_file - 1:], start = start_line_of_file):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E501 line too long (102 > 79 characters)

Origin: PycodestyleBear (E501), Section: autopep8.

result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file[start_line_of_file - 1:], start = start_line_of_file):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E251 unexpected spaces around keyword / parameter equals

Origin: PycodestyleBear (E251), Section: autopep8.

result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file[start_line_of_file - 1:], start = start_line_of_file):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line is longer than allowed. (102 > 79)

Origin: LineLengthBear, Section: linelength.

@khanchi97 khanchi97 force-pushed the spaceConsistency-2207 branch 2 times, most recently from 35c71c6 to 540ef02 Compare January 8, 2018 14:50
Add allow_trailing_blanklines option for whether to allow
leading blanklines in starting of a file.

Closes coala#2207
@khanchi97 khanchi97 force-pushed the spaceConsistency-2207 branch from 540ef02 to d5865a5 Compare January 9, 2018 08:25
Copy link
Member

@shreyateeza shreyateeza left a comment

Choose a reason for hiding this comment

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

This should be merged @jayvdb

@@ -1,4 +1,3 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change? I think you sud file a new issue for this or just include this in a different commit

@@ -1,4 +1,3 @@

from bears.c_languages.ClangBear import ClangBear
Copy link
Contributor

Choose a reason for hiding this comment

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

This as well ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

@newbazz as I told you before :) this patch was shown in CI just because of feature that I have added ;)
you can see I deleted leading blanklines from these files.

Copy link

@thealphadollar thealphadollar left a comment

Choose a reason for hiding this comment

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

LGTM

@jayvdb jayvdb requested a review from ankitxjoshi April 9, 2018 13:43
Copy link
Contributor

@ankitxjoshi ankitxjoshi left a comment

Choose a reason for hiding this comment

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

The enhancement works as required. Just some changes needed. Perform a rebase before pushing.

@@ -29,6 +30,9 @@ def run(self,
True if spaces are to be used instead of tabs.
:param allow_trailing_whitespace:
Whether to allow trailing whitespace or not.
:param allow_leading_blanklines:
Whether to allow leading blanklines
Copy link
Contributor

Choose a reason for hiding this comment

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

blank lines not blanklines

@@ -29,6 +30,9 @@ def run(self,
True if spaces are to be used instead of tabs.
:param allow_trailing_whitespace:
Whether to allow trailing whitespace or not.
:param allow_leading_blanklines:
Whether to allow leading blanklines
at start of file or not.
Copy link
Contributor

Choose a reason for hiding this comment

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

at the start not at start

@@ -38,7 +42,50 @@ def run(self,
result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file, start=1):
def get_blanklines_nr_end():
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we come up with a better name for the function? This name is quite misleading.

enumerated_tuple = tuple(enumerated_zip_obj)

for line_number, line in enumerated_tuple:
replacement = line
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need for a temporary variable. Python uses Call by Object Reference and not Pass by reference.

start_line_of_file = blanklines_nr_end + 1
result_texts.append('Leading blanklines.')
additional_info_texts.append(
'Your source code contains leading blanklines.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Again blank lines

result_texts.append('Leading blanklines.')
additional_info_texts.append(
'Your source code contains leading blanklines.'
'Those usually have no meaning. Please consider'
Copy link
Contributor

Choose a reason for hiding this comment

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

You missed a space after Please consider

@@ -38,7 +42,50 @@ def run(self,
result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file, start=1):
def get_blanklines_nr_end():
line_nr_end = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Again a better name for the variable.

Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

See review by @MacBox7

@@ -18,6 +18,7 @@ def run(self,
file,
use_spaces: bool,
allow_trailing_whitespace: bool=False,
allow_leading_blanklines: bool=False,
Copy link
Member

Choose a reason for hiding this comment

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

add spaces around = ; new style requirement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

7 participants