Skip to content

Allow lines parameter to fix whitespace between statements. #864

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 5 commits into from
Sep 20, 2020

Conversation

msuozzo
Copy link
Member

@msuozzo msuozzo commented Sep 18, 2020

The previous logic largely ignored lines values that were not part of
statements. This means that yapf required a statement to be formatted to
make whitespace-only changes.

This logic tweak allows lines to be taken into account even when the
adjacent statements are disabled (i.e. not in lines).

The previous logic considered all components of the generator in a
single-argument call such as 'f(a for a in x)' to be strongly connected
like a normal single-argument call (e.g. 'f(a_var)'). This caused
formatting irregularities in these cases.

This change retains normal split penalties in the single-argument
generator case.
The previous logic largely ignored `lines` values that were not part of
statements. This means that yapf required a statement to be formatted to
make whitespace-only changes.

This logic tweak allows `lines` to be taken into account even when the
adjacent statements are disabled (i.e. not in `lines`).
@google-cla google-cla bot added the cla: yes label Sep 18, 2020
@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 96.324% when pulling 032a57d on msuozzo:master into a3dacba on google:master.

@coveralls
Copy link

coveralls commented Sep 18, 2020

Coverage Status

Coverage decreased (-0.002%) to 96.321% when pulling 032a57d on msuozzo:master into a3dacba on google:master.

@msuozzo
Copy link
Member Author

msuozzo commented Sep 18, 2020

Coverage doesn't really look like it decreased. Could have just been the result of removing some code.

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

Successfully merging this pull request may close these issues.

None yet

3 participants