Skip to content

Bool expression comment placement#7269

Merged
MichaReiser merged 2 commits intomainfrom
bool-op-binary-like
Sep 12, 2023
Merged

Bool expression comment placement#7269
MichaReiser merged 2 commits intomainfrom
bool-op-binary-like

Conversation

@MichaReiser
Copy link
Copy Markdown
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
Copy Markdown
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
Comment thread crates/ruff_python_formatter/src/comments/placement.rs Outdated
Comment thread crates/ruff_python_formatter/src/expression/binary_like.rs
Comment thread crates/ruff_python_formatter/src/expression/binary_like.rs Outdated
Comment thread crates/ruff_python_formatter/src/expression/binary_like.rs Outdated
@konstin
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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

Development

Successfully merging this pull request may close these issues.

Formatter: Trailing end of line comments in binary like expressions Formatter: Allow comments between and/or and the right hand side

2 participants