Skip to content

Add blank lines to separate blocks of indented code #1515

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

Merged
merged 13 commits into from
Apr 30, 2018

Conversation

DonJayamanne
Copy link

Fixes #259

This pull request:

  • Has a title summarizes what is changing
  • Includes a news entry file (remember to thank yourself!)
  • Has unit tests & code coverage is not adversely affected (within reason)
  • Works on all actively maintained versions of Python (e.g. Python 2.7 & the latest Python 3 release)
  • Works on Windows 10, macOS, and Linux (e.g. considered file system case-sensitivity)

Copy link
Member

@brettcannon brettcannon left a comment

Choose a reason for hiding this comment

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

My changes are all rather minor, so I'm pre-approving this.

ast.NodeVisitor.generic_visit(self, node)


def _tokenize(source):
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment mentioning why you're using an undocumented API.

self.line_numbers_with_statements = []

def generic_visit(self, node):
node_type = type(node).__name__
Copy link
Member

Choose a reason for hiding this comment

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

Dead line.



def _get_global_statement_blocks(source, lines):
"""Gets a list of all global statement blocks.
Copy link
Member

Choose a reason for hiding this comment

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

"""Return a list of all global statement blocks.

    The list comprises of 3-item tuples that contain the starting line number, ending line number,
    and whether the statement is a single line.

"""

previous_statement = statement_ranges[-1]
previous_statement_is_oneline = previous_statement[2]
if previous_statement_is_oneline and current_statement_is_oneline:
statement_ranges[-1] = (previous_statement[0], end_line_number, True)
Copy link
Member

Choose a reason for hiding this comment

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

Parentheses are unnecessary.


statement_ranges = []
for index, line_number in enumerate(visitor.line_numbers_with_statements):
remaining_line_numbers = visitor.line_numbers_with_statements[index + 1:]
Copy link
Member

Choose a reason for hiding this comment

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

You can drop the spaces in index + 1.

if len(line.strip()) == 0 and token.tok_name[toknum] == 'NL' and spos[0] == epos[0])

for line_number in reversed(list(newlines_indexes_to_remove)):
del lines[line_number - 1]
Copy link
Member

Choose a reason for hiding this comment

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

You can drop the spaces in line_number - 1.


# Step 2: Add blank lines between each global statement block.
# A consequtive single lines blocks of code will be treated as a single statement,
# just to ensure we do not unnecessarily add too many blank lines.
Copy link
Member

Choose a reason for hiding this comment

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

Extra leading spaces in the comment.


global_statement_ranges = _get_global_statement_blocks(source, lines)

for line_number in (start_line for start_line, _, _ in reversed(global_statement_ranges) if start_line > 1):
Copy link
Member

Choose a reason for hiding this comment

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

filter(lambda x: x > 1, map(operator.itemgetter(0), reversed(global_statement_ranges)) is another option. 😁

@codecov
Copy link

codecov bot commented Apr 30, 2018

Codecov Report

Merging #1515 into master will decrease coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1515      +/-   ##
==========================================
- Coverage   71.43%   71.33%   -0.11%     
==========================================
  Files         273      273              
  Lines       12692    12700       +8     
  Branches     2282     2282              
==========================================
- Hits         9066     9059       -7     
- Misses       3492     3500       +8     
- Partials      134      141       +7
Impacted Files Coverage Δ
src/client/terminals/codeExecution/helper.ts 92.85% <100%> (+1.36%) ⬆️
src/client/debugger/PythonProcess.ts 45.83% <0%> (-5.42%) ⬇️
src/client/debugger/Main.ts 51.85% <0%> (-0.5%) ⬇️
...rc/client/debugger/configProviders/baseProvider.ts 92.95% <0%> (-0.29%) ⬇️
.../client/debugger/configProviders/pythonProvider.ts 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f1d10a...375df2a. Read the comment docs.

@DonJayamanne DonJayamanne added this to the _ milestone Apr 30, 2018
@DonJayamanne DonJayamanne merged commit da91e3f into microsoft:master Apr 30, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants