Skip to content

Fix spacing of general (non-specific) tokens + tests #1222

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Mar 28, 2018
Merged

Fix spacing of general (non-specific) tokens + tests #1222

merged 15 commits into from
Mar 28, 2018

Conversation

MikhailArkhipov
Copy link

Fixes #1096

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)

brettcannon
brettcannon suggested changes Mar 27, 2018
View reviewed changes
src/test/pythonFiles/formatting/fileToFormatOnEnter.py
@@ -6,4 +6,7 @@
x+1 #
@x
x.y
if x<=1:
Copy link
Member

Choose a reason for hiding this comment

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

This actually won't trigger the bug. It has to have a number on the left-hand side of the operator, e.g. if 1<=x:. Maybe worth keeping this one and just adding another test?

Copy link
Author

Choose a reason for hiding this comment

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

OK

Copy link
Author

Choose a reason for hiding this comment

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

Done

src/test/format/extension.onEnterFormat.test.ts Outdated

test('Formatting space after open brace', async () => {
const text = await formatAtPosition(11, 0);
assert.equal(text, 'while(1)', 'Method definition with arguments was not formatted');
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a method definition so I think the comment of what failed isn't accurate.

src/client/formatters/lineFormatter.ts Outdated
}

// In general, keep tokes separated
Copy link
Member

Choose a reason for hiding this comment

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

"tokens" and missing a period (couple of other comments are also missing periods).

MikhailArkhipov added 2 commits March 27, 2018 16:30
@brettcannon
Copy link
Member

@MikhailArkhipov did you mean to close this?

@codecov
Copy link

codecov bot commented Mar 28, 2018 •
edited
Loading

Codecov Report

Merging #1222 into master will increase coverage by 0.2%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #1222     +/-   ##
=========================================
+ Coverage   71.91%   72.11%   +0.2%     
=========================================
  Files         260      260             
  Lines       11933    11972     +39     
  Branches     2121     2134     +13     
=========================================
+ Hits         8582     8634     +52     
+ Misses       3221     3214      -7     
+ Partials      130      124      -6
Impacted Files Coverage Δ
src/client/formatters/lineFormatter.ts 99.2% <100%> (+0.36%) ⬆️
src/client/language/tokenizer.ts 96.73% <0%> (+0.4%) ⬆️
src/client/debugger/Main.ts 50.61% <0%> (+0.49%) ⬆️
...rc/client/debugger/PythonProcessCallbackHandler.ts 52.96% <0%> (+0.65%) ⬆️
src/client/debugger/PythonProcess.ts 48.75% <0%> (+3.33%) ⬆️

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 905841a...99e037c. Read the comment docs.

@brettcannon brettcannon merged commit c82e04e into microsoft:master Mar 28, 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.
Reviewers

@brettcannon brettcannon brettcannon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The if statement is automatically formatted incorrectly after carriage return
2 participants
@MikhailArkhipov @brettcannon