Exclude pragma comments from measured line width#7008
Exclude pragma comments from measured line width#7008MichaReiser merged 7 commits intoastral-sh:mainfrom
Conversation
There was a problem hiding this comment.
Looking good. I've one small nit and would like to ask you to add a few pragma comment-specific tests that test the behavior outside of black's test suite (you may want to include some of your interesting NBSP cases too!)
This PR
| project | similarity index | total files | changed files |
|---|---|---|---|
| cpython | 0.76079 | 1789 | 1634 |
| django | 0.99948 | 2760 | 85 |
| transformers | 0.99925 | 2587 | 495 |
| twine | 0.99982 | 33 | 2 |
| typeshed | 0.99978 | 3496 | 2172 |
| warehouse | 0.99661 | 648 | 39 |
| zulip | 0.99942 | 1437 | 42 |
Base
| project | similarity index | total files | changed files |
|---|---|---|---|
| cpython | 0.76079 | 1789 | 1635 |
| django | 0.99948 | 2760 | 85 |
| transformers | 0.99925 | 2587 | 495 |
| twine | 0.99965 | 33 | 3 |
| typeshed | 0.99947 | 3496 | 2173 |
| warehouse | 0.99659 | 648 | 41 |
| zulip | 0.99938 | 1437 | 47 |
Funny how typeshed is close to a similarity index of 1 but every other file gets reformatted 🙈
|
NBSP is such a niche thing to focus on, so I'll move on soon, but while adding these fixtures I noticed the following isn't aligned between the formatter and # Input: \u{a0}\u{a0}type
i = "" # type: Add space before two leading NBSP
# Ruff: \u{a0}type
i = "" # type: Add space before two leading NBSP
# Black: \u{a0}\u{a0}type
i = "" # type: Add space before two leading NBSPShould be simple, so I'll submit it in a follow up PR and correct any snapshots. |
| i = "" #type: A space is added | ||
| i = "" # type: Add space before leading NBSP followed by a space | ||
| i = "" # type: Add space before leading NBSP | ||
| i = "" # type: Add space before two leading NBSP |
There was a problem hiding this comment.
Note I'll address this in a follow up PR. See #7008 (comment)
|
|
||
|
|
||
| # A noqa as `#\u{A0}\u{A0}noqa` becomes `# \u{A0}noqa` | ||
| i = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" # noqa |
There was a problem hiding this comment.
Same comment about aligning with black for NBSP edge cases
# Input: #\u{a0}\u{a0}noqa
i = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" # noqa
# Black: # \u{a0}noqa
i = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" # noqa
# Ruff: # noqa
i = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" # noqa
|
I'm going to mark as a draft and do another review of this during lunch today (~3 hours from now; would review my fixtures and the resolved merge conflict again). If you decide to merge anyway I'm cool with that too. |
CodSpeed Performance ReportMerging #7008 will degrade performances by 1.52%Comparing Summary
Benchmarks breakdown
|
cnpryer
changed the title
2 participants
Closes #6772
Summary
If the comment doesn't include a NBSP and starts withnoqa,type:,pyright:, orpylint:then we assign it0reserved width for its line suffix. Otherwise measure width.See #6772 (comment)
This PR aligns with
rufflinting behavior in that it will disregard the kind of white-space in order to identify pragmas to exclude from measured line width.Test Plan
Current fixtures and new trailing comments fixtures.