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

Bool expression comment placement #7269

Merged
merged 2 commits into from
Sep 12, 2023
Merged

Bool expression comment placement #7269

merged 2 commits into from
Sep 12, 2023

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Sep 11, 2023

Summary

This PR fixes all comment placement issues for boolean expression (and, or) for the django project by

a) Rewriting the boolean expression formatting to use the BinaryLike formatting to get consistent formatting for all binary like expressions
b) Making a\n # comment\n and b a trailing comment of a the same as for binary expressions
c) Respecting whether a comment was inside or outside of the parentheses when formatting a parenthesized expression.

Closes #7004
Closes #6062

Test Plan

Added tests

this PR

project similarity index total files changed files
cpython 0.76084 1789 1632
django 0.99979 2760 45
transformers 0.99944 2587 413
twine 1.00000 33 0
typeshed 0.99978 3496 2173
warehouse 0.99834 648 20
zulip 0.99956 1437 23

main

project similarity index total files changed files
cpython 0.76083 1789 1632
django 0.99966 2760 58
transformers 0.99930 2587 447
twine 1.00000 33 0
typeshed 0.99978 3496 2173
warehouse 0.99825 648 22
zulip 0.99950 1437 27

@MichaReiser
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser added the formatter Related to the formatter label Sep 11, 2023
@MichaReiser MichaReiser requested a review from konstin September 11, 2023 13:37
@MichaReiser MichaReiser marked this pull request as ready for review September 11, 2023 13:38
@konstin
Copy link
Member

konstin commented Sep 11, 2023

nice work on the compatibility!

@MichaReiser MichaReiser enabled auto-merge (squash) September 12, 2023 06:25
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 12, 2023

CodSpeed Performance Report

Merging #7269 will degrade performances by 3.73%

Comparing bool-op-binary-like (6ffae39) with main (babf8d7)

Summary

❌ 5 (👁 5) regressions
✅ 20 untouched benchmarks

Benchmarks breakdown

Benchmark main bool-op-binary-like Change
👁 linter/all-rules[numpy/ctypeslib.py] 33.5 ms 34.5 ms -3.01%
👁 linter/all-rules[unicode/pypinyin.py] 15.2 ms 15.7 ms -2.89%
👁 linter/all-rules[pydantic/types.py] 70 ms 72.3 ms -3.14%
👁 linter/all-rules[large/dataset.py] 156.9 ms 162.9 ms -3.73%
👁 linter/all-rules[numpy/globals.py] 4 ms 4.1 ms -2.6%

@MichaReiser MichaReiser merged commit 1e6df19 into main Sep 12, 2023
@MichaReiser MichaReiser deleted the bool-op-binary-like branch September 12, 2023 06:39
let comments = f.context().comments().clone();
let expression_comments = comments.leading_dangling_trailing(expression);

// Format leading comments that are before the inner most `(` outside of the expression's parentheses.
Copy link
Member

Choose a reason for hiding this comment

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

thanks! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
2 participants