SpaceConsistencyBear: Add blanklines_at_EOF option #2173

khanchi97 · 2017-12-16T12:02:03Z

Add allow_trailing_blanklines option for whether to allow
trailing blanklines at EOF.

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

I read the commit guidelines and I've followed
them.
I ran coala over my code locally. (All commits have to pass
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use cobot mark wip <URL> to get it out
of the review queue.

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

gitmate-bot · 2017-12-16T12:02:25Z

bears/general/SpaceConsistencyBear.py

+
+            if no_blanklines:
+                if enforce_newline_at_EOF:
+                    # Since every line contains at the end at least one \n, only


E501 line too long (80 > 79 characters)

Origin: PycodestyleBear (E501), Section: autopep8.

gitmate-bot · 2017-12-16T12:02:26Z

bears/general/SpaceConsistencyBear.py

+                        replacement += '\n'
+                        result_texts.append('No newline at EOF.')
+                        additional_info_texts.append(
+                            "A trailing newline character ('\\n') is missing from "


E501 line too long (83 > 79 characters)

Origin: PycodestyleBear (E501), Section: autopep8.

gitmate-bot · 2017-12-16T12:02:27Z

bears/general/SpaceConsistencyBear.py

+                        additional_info_texts.append(
+                            "A trailing newline character ('\\n') is missing from "
+                            'your file. '
+                            '<http://stackoverflow.com/a/5813359/3212182> gives '


E501 line too long (81 > 79 characters)

Origin: PycodestyleBear (E501), Section: autopep8.

gitmate-bot · 2017-12-16T12:02:28Z

tests/general/SpaceConsistencyBearTest.py

@@ -70,3 +70,21 @@ def test_enforce_newline_at_eof(self):
                               "    print('funny')\n",
                               "    print('the result is not funny...')"],
                              force_linebreaks=False)


The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp40ntr2v2/tests/general/SpaceConsistencyBearTest.py +++ b/tmp/tmp40ntr2v2/tests/general/SpaceConsistencyBearTest.py @@ -70,6 +70,7 @@ " print('funny')\n", " print('the result is not funny...')"], force_linebreaks=False) + def test_trailing_blanklines(self): self.section.append(Setting('use_spaces', 'true')) self.section.append(Setting('allow_trailing_whitespace', 'false'))

this is the end of a class right? thats why PEP8Bear wants an additional blank line

gitmate-bot · 2017-12-16T12:02:29Z

tests/general/SpaceConsistencyBearTest.py

+        self.section.append(Setting('allow_trailing_blanklines', 'False'))
+
+        self.check_invalidity(self.uut,
+                            ['def funny_code():\n',


The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp40ntr2v2/tests/general/SpaceConsistencyBearTest.py +++ b/tmp/tmp40ntr2v2/tests/general/SpaceConsistencyBearTest.py @@ -77,12 +77,12 @@ self.section.append(Setting('allow_trailing_blanklines', 'False')) self.check_invalidity(self.uut, - ['def funny_code():\n', - " print('Is it funny?')\n", - " print('Yeah, funny')\n", - ' ', - ''], - force_linebreaks=False) + ['def funny_code():\n', + " print('Is it funny?')\n", + " print('Yeah, funny')\n", + ' ', + ''], + force_linebreaks=False) self.check_validity(self.uut, ['def funny_code():\n', " print('Is it funny?')\n",

gitmate-bot · 2017-12-16T12:02:30Z

tests/general/SpaceConsistencyBearTest.py

@@ -70,3 +70,21 @@ def test_enforce_newline_at_eof(self):
                               "    print('funny')\n",
                               "    print('the result is not funny...')"],
                              force_linebreaks=False)
+    def test_trailing_blanklines(self):


E301 expected 1 blank line, found 0

Origin: PycodestyleBear (E301), Section: autopep8.

gitmate-bot · 2017-12-16T12:02:31Z

tests/general/SpaceConsistencyBearTest.py

+        self.section.append(Setting('allow_trailing_blanklines', 'False'))
+
+        self.check_invalidity(self.uut,
+                            ['def funny_code():\n',


E128 continuation line under-indented for visual indent

Origin: PycodestyleBear (E128), Section: autopep8.

gitmate-bot · 2017-12-16T12:02:31Z

bears/general/SpaceConsistencyBear.py

+
+            if no_blanklines:
+                if enforce_newline_at_EOF:
+                    # Since every line contains at the end at least one \n, only


Line is longer than allowed. (80 > 79)

Origin: LineLengthBear, Section: linelength.

gitmate-bot · 2017-12-16T12:02:32Z

bears/general/SpaceConsistencyBear.py

+                        replacement += '\n'
+                        result_texts.append('No newline at EOF.')
+                        additional_info_texts.append(
+                            "A trailing newline character ('\\n') is missing from "


Line is longer than allowed. (83 > 79)

Origin: LineLengthBear, Section: linelength.

gitmate-bot · 2017-12-16T13:52:44Z

tests/general/SpaceConsistencyBearTest.py


        self.check_validity(self.uut,
                            ['hello world  \n'],
                            force_linebreaks=False)
        self.check_validity(self.uut,
                            ['def somecode():\n',
                             "    print('funny')\n",
-                             "    print('funny end.')\n"],
+                             "    print('funny end.')\n",
+                             "  \n"


You do not use the preferred quotation marks.

Origin: QuotesBear, Section: python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpl6now4yj/tests/general/SpaceConsistencyBearTest.py +++ b/tmp/tmpl6now4yj/tests/general/SpaceConsistencyBearTest.py @@ -62,7 +62,7 @@ ['def somecode():\n', " print('funny')\n", " print('funny end.')\n", - " \n" + ' \n' "\n"], force_linebreaks=False) self.check_invalidity(self.uut,

gitmate-bot · 2017-12-16T13:52:45Z

tests/general/SpaceConsistencyBearTest.py

-                             "    print('funny end.')\n"],
+                             "    print('funny end.')\n",
+                             "  \n"
+                             "\n"],


You do not use the preferred quotation marks.

Origin: QuotesBear, Section: python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpl6now4yj/tests/general/SpaceConsistencyBearTest.py +++ b/tmp/tmpl6now4yj/tests/general/SpaceConsistencyBearTest.py @@ -63,7 +63,7 @@ " print('funny')\n", " print('funny end.')\n", " \n" - "\n"], + '\n'], force_linebreaks=False) self.check_invalidity(self.uut, [' no hello world'],

gitmate-bot · 2017-12-17T06:30:38Z

bears/general/SpaceConsistencyBear.py

+                                             reversed(file))
+                rev_enumerated_tuple = tuple(rev_enumerated_zip_obj)
+                line_nr_start = -1
+                blanklines_status = False


This file contains unused source code.

Origin: PyUnusedCodeBear, Section: flakes.

The issue can be fixed by applying the following patch:

--- a/tmp/tmplpvp2vcv/bears/general/SpaceConsistencyBear.py +++ b/tmp/tmplpvp2vcv/bears/general/SpaceConsistencyBear.py @@ -48,7 +48,6 @@ reversed(file)) rev_enumerated_tuple = tuple(rev_enumerated_zip_obj) line_nr_start = -1 - blanklines_status = False for line_number, line in rev_enumerated_tuple: replacement = line

gitmate-bot · 2017-12-17T06:30:39Z

bears/general/SpaceConsistencyBear.py

+                for line_number, line in rev_enumerated_tuple:
+                    replacement = line
+                    if replacement.strip() == '':
+                        blanklines_status = True


This file contains unused source code.

Origin: PyUnusedCodeBear, Section: flakes.

The issue can be fixed by applying the following patch:

--- a/tmp/tmplpvp2vcv/bears/general/SpaceConsistencyBear.py +++ b/tmp/tmplpvp2vcv/bears/general/SpaceConsistencyBear.py @@ -53,7 +53,6 @@ for line_number, line in rev_enumerated_tuple: replacement = line if replacement.strip() == '': - blanklines_status = True line_nr_start = line_number else: break

Makman2 · 2018-01-01T20:01:50Z

bears/general/SpaceConsistencyBear.py

@@ -38,48 +41,84 @@ def run(self,
        result_texts = []
        additional_info_texts = []

+        def get_blanklines_nr_start():
+


though not violating PEP8, I usually prefer no blanklines between a def and code, this kinda interrupts reading flow imo. Optional though if you see it differently ;)

Removed 👍

Makman2 · 2018-01-19T00:46:22Z

bears/general/SpaceConsistencyBear.py

+                additional_info_text = (
+                        'Your source code contains trailing blanklines at EOF.'
+                        'Those usually have no meaning. Please consider'
+                        'removing them.')


Too much text. Just Trailing blanklines found or similar. No intentions, no suggestions ;)

Then you can easily inline this value as well.

Ah nvm that's the additional-info-text

Makman2 · 2018-01-19T00:47:11Z

bears/general/SpaceConsistencyBear.py

+                    'Trailing blanklines found.',
+                    diffs={filename: diff},
+                    file=filename,
+                    additional_info=additional_info_text)


could you also fill the affected_code attribute? Not sure if we are that smart to extract the ranges from the diff^^

Makman2 · 2018-01-19T00:47:48Z

bears/general/SpaceConsistencyBear.py

+                additional_info_text = (
+                        'Your source code contains trailing blanklines at EOF.'
+                        'Those usually have no meaning. Please consider'
+                        'removing them.')


Indentation: 4 spaces relative to statement

Makman2 · 2018-01-19T01:04:02Z

bears/general/SpaceConsistencyBear.py

+
+        else:
+            blanklines_start_line = line_nr_start + 1 \
+                    if line_nr_start < len(file) else False


I have the feeling you are unnecessary trying to use special values (--> False).

You can easily check if you have to yield a result if line_nr_start < len(file).
The whole stuff can be refactored to roughly this:

if not allow_trailing_blanklines: line_nr_start = next((i for i, line in reversed(list( enumerate(file, start=1))) if line.strip() != ''), 0) # TODO: Maybe more performance: Don't increment from start=1, but start=0 and # the next-default value = -1, and increment in the end: next(..., -1) + 1 if line_nr_start < len(file): # Note: You have to provide line_nr_start + 1 to refer to the right lines. # line_nr_start is right now designed to give the starting line referring # to zero-based indices. yield Result(...) else: line_nr_start = len(file) for line_number, line in enumerate(file[:line_nr_start])

Note also how I've put the relevant analysis inside the if, to save performance in case someone wants not to take care of blanklines

yeah, I wanted to do the same.But I was finding it difficult to cover the case when file is empty.That's why I used that (--> False), but now it will work as we are using start=0 and next default value -1.

Makman2 · 2018-01-19T01:07:17Z

bears/general/SpaceConsistencyBear.py

@@ -38,7 +41,36 @@ def run(self,
        result_texts = []
        additional_info_texts = []

-        for line_number, line in enumerate(file, start=1):
+        line_nr_start = next((i for i, line in reversed(list(
+            enumerate(file, start=1))) if line.strip() != ''), 0)


Optional:
Is it maybe possible to wrap the statement somehow so it's more readable?

Something in the direction of

line_nr_start = next( (i for i, line in reversed(list(enumerate(file, start=1))) if line.strip() != ''), 0)

Makman2 · 2018-01-19T01:10:33Z

tests/general/SpaceConsistencyBearTest.py

+                            ['def funny_code():\n',
+                             "  print('Is it funny?')\n",
+                             "  print('Yeah, funny')\n"],
+                            force_linebreaks=False)


I want to see tests for

an empty file

a file with only a single blank line

a file with only multiple blank lines

a file with leading and trailing blank lines

Makman2 · 2018-01-19T01:10:49Z

tests/general/SpaceConsistencyBearTest.py

+                            ['def funny_code():\n',
+                             "  print('Is it funny?')\n",
+                             "  print('Yeah, funny')\n"],
+                            force_linebreaks=False)


And please use check_results to assert for the bear results 👍

Makman2 · 2018-01-19T01:11:31Z

unack 2e2eaac

Add allow_trailing_blanklines option for whether to allow trailing blanklines at EOF.And some blanklines are also removed from some files just because of `blanklines_at_EOF` option. Closes coala#1793

gitmate-bot · 2018-01-23T18:03:57Z

bears/general/SpaceConsistencyBear.py

+                    end_column=len(file[-1]),
+                    additional_info=additional_info_text)
+
+        else :


The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpyheznxr2/bears/general/SpaceConsistencyBear.py +++ b/tmp/tmpyheznxr2/bears/general/SpaceConsistencyBear.py @@ -64,7 +64,7 @@ end_column=len(file[-1]), additional_info=additional_info_text) - else : + else: line_nr_start = len(file) for line_number, line in enumerate(file[:line_nr_start],

gitmate-bot · 2018-01-23T18:03:58Z

bears/general/SpaceConsistencyBear.py

+                    end_column=len(file[-1]),
+                    additional_info=additional_info_text)
+
+        else :


E203 whitespace before ':'

Origin: PycodestyleBear (E203), Section: autopep8.

gitmate-bot · 2018-01-23T18:03:59Z

tests/general/SpaceConsistencyBearTest.py

+                                               column=1,
+                                               end_line=1,
+                                               end_column=3)],
+                           filename = 'xyz.py')


The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpyheznxr2/tests/general/SpaceConsistencyBearTest.py +++ b/tmp/tmpyheznxr2/tests/general/SpaceConsistencyBearTest.py @@ -94,7 +94,7 @@ column=1, end_line=1, end_column=3)], - filename = 'xyz.py') + filename='xyz.py') # self.check_results(self.uut, # [' \n', # ' \n',

gitmate-bot · 2018-01-23T18:04:00Z

tests/general/SpaceConsistencyBearTest.py

+                                               column=1,
+                                               end_line=1,
+                                               end_column=3)],
+                           filename = 'xyz.py')


E251 unexpected spaces around keyword / parameter equals

Origin: PycodestyleBear (E251), Section: autopep8.

gitmate-bot · 2018-01-23T18:04:01Z

tests/general/SpaceConsistencyBearTest.py

+                                               column=1,
+                                               end_line=1,
+                                               end_column=3)],
+                           filename = 'xyz.py')


E251 unexpected spaces around keyword / parameter equals

Origin: PycodestyleBear (E251), Section: autopep8.

Makman2 · 2018-01-28T00:28:11Z

please fix the gitmate issues ;)

jayvdb · 2018-04-21T23:07:39Z

Conflicts to be resolved.

khanchi97 · 2018-04-22T05:57:08Z

@Makman2 @jayvdb Can I ask for adding someone as collaborator to my PRs(which are almost completed) to do remaining work. ( I am busy in some other work :) )

jayvdb · 2018-04-22T06:21:02Z

no. They are easy to finish.
Best we can do is add reviewers, and give more precise guidance on what needs to be done.

sangamcse · 2018-04-22T18:58:27Z

bears/general/SpaceConsistencyBear.py

+                    end_column=len(file[-1]),
+                    additional_info=additional_info_text)
+
+        else :


Fix this whitespace issue @khanchi97. Run coala over your local repository before pushing it to remote 😉

sangamcse · 2018-04-22T19:02:24Z

bears/general/SpaceConsistencyBear.py

@@ -18,6 +18,7 @@ def run(self,
            file,
            use_spaces: bool,
            allow_trailing_whitespace: bool=False,
+            allow_trailing_blanklines: bool=False,


Alphabetical order 😉

sangamcse · 2018-04-22T19:56:53Z

bears/general/SpaceConsistencyBear.py

+                     if line.strip() != ''), -1) + 1
+            if line_nr_start < len(file):
+                additional_info_text = (
+                    'Your source code contains trailing blanklines at EOF.'


Add a space after EOF..

sangamcse · 2018-04-22T19:57:14Z

bears/general/SpaceConsistencyBear.py

+            if line_nr_start < len(file):
+                additional_info_text = (
+                    'Your source code contains trailing blanklines at EOF.'
+                    'Those usually have no meaning. Please consider'


sangamcse · 2018-04-22T20:02:27Z

tests/general/SpaceConsistencyBearTest.py

@@ -44,23 +46,28 @@ def test_data_sets_tabs(self):
        self.section.append(Setting('use_spaces', 'false'))
        self.section.append(Setting('allow_trailing_whitespace', 'true'))
        self.section.append(Setting('enforce_newline_at_EOF', 'false'))
+        self.section.append(Setting('allow_trailing_blanklines', 'false'))


This line is not required.

sangamcse · 2018-04-22T20:05:54Z

tests/general/SpaceConsistencyBearTest.py

+        self.section['allow_trailing_whitespace'] = 'false'
+        self.section['enforce_newline_at_EOF'] = 'true'
+        self.section['allow_trailing_blanklines'] = 'false'
+        self.check_results(self.uut,


What do you say about this?

def test_trailing_blanklines(self): self.maxDiff = None self.section.append(Setting('use_spaces', 'true')) self.section.append(Setting('allow_trailing_whitespace', 'false')) self.section.append(Setting('enforce_newline_at_EOF', 'true')) self.section.append(Setting('allow_trailing_blanklines', 'false')) self.check_validity(self.uut, ['sangam\n'], force_linebreaks=False) self.check_invalidity(self.uut, [' \n'], force_linebreaks=False)

gitmate-bot added process/pending review size/S area/genericbears difficulty/low labels Dec 16, 2017

gitmate-bot assigned khanchi97 Dec 16, 2017

gitmate-bot reviewed Dec 16, 2017

View reviewed changes

khanchi97 force-pushed the spaceConsistency-blanklines branch from 99aadeb to df062a5 Compare December 16, 2017 13:52

gitmate-bot reviewed Dec 16, 2017

View reviewed changes

khanchi97 force-pushed the spaceConsistency-blanklines branch 3 times, most recently from 6550c5f to e784493 Compare December 17, 2017 06:30

gitmate-bot reviewed Dec 17, 2017

View reviewed changes

khanchi97 force-pushed the spaceConsistency-blanklines branch 6 times, most recently from 934b7d6 to 015c86c Compare December 19, 2017 15:24

Makman2 reviewed Jan 1, 2018

View reviewed changes

khanchi97 force-pushed the spaceConsistency-blanklines branch from 015c86c to e1f1a4c Compare January 2, 2018 04:28

Makman2 reviewed Jan 19, 2018

View reviewed changes

Makman2 added process/wip and removed process/pending review labels Jan 19, 2018

SpaceConsistencyBear: Add blanklines_at_EOF option

3d50da1

Add allow_trailing_blanklines option for whether to allow trailing blanklines at EOF.And some blanklines are also removed from some files just because of `blanklines_at_EOF` option. Closes coala#1793

khanchi97 force-pushed the spaceConsistency-blanklines branch from 2e2eaac to 3d50da1 Compare January 23, 2018 18:03

gitmate-bot added process/pending review and removed process/wip labels Jan 23, 2018

gitmate-bot reviewed Jan 23, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Apr 21, 2018

jayvdb requested a review from sangamcse April 22, 2018 06:22

sangamcse reviewed Apr 22, 2018

View reviewed changes

sangamcse suggested changes Apr 22, 2018

View reviewed changes

khanchi97 closed this Mar 16, 2022

SpaceConsistencyBear: Add blanklines_at_EOF option #2173

SpaceConsistencyBear: Add blanklines_at_EOF option #2173

Conversation

khanchi97 commented Dec 16, 2017

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Makman2 commented Jan 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Makman2 commented Jan 28, 2018

jayvdb commented Apr 21, 2018

khanchi97 commented Apr 22, 2018

jayvdb commented Apr 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment