-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Discrepancy between format
and check
for a long string literal
#9926
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
Comments
Actually, there is a case where a reformatting can be applied to fix the issue, but isn't. a = {
"key": "very very very very very very very very very very very very very long line "
}
a = {
"key": (
"very very very very very very very very very very very very very long line "
)
} Which puts it within 88 characters. |
Ah yeah, this is intentional and it's discussed a bit in the linter-formatter compatibility section of the docs. In short, the formatter treats Your second comment may be related to #8438 which were some changes we considered making to expression formatting but decided to revisit holistically before making further changes -- though @MichaReiser would be able to say for sure whether he'd expect that reformatting to happen today in latest Ruff (or not). |
a = {
"key": (
"very very very very very very very very very very very very very long line "
)
} Ruff doesn't parenthesize long dict values. Black considers parenthesizing long dictionary values, but this is a preview style. Parenthesizing values in expression positions sets a new precedence in Black's style guide and is something that should be considered carefully (see my feedback on Black's preview style). Parenthesizing values that can't be split further is an option. It raises the question if the parentheses should also be added if the string, when parenthesized, doesn't fit: a = {
"key": (
"very very very very very very very very very very very very very long long long long line "
)
} IMO, I would always parenthesize it because it can reduce the amount that the string exceeds the line length limit (not guaranteed). However, this "optimisation" is currently implemented for assignments where the string only gets parenthesized if it makes it fit on the line. Unfortunately, this style has a larger impact on runtime performance because, in the worst case, the formatter needs to consider 3 layouts for every string:
|
If there is a long string key and a long string value in a dict literal, {
"blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah": "blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah"
} In fact, if there is a newline after the ":" in a dict literal, ruff format will remove that newline even if that makes the line exceed the max line length. I consider both of those bugs in The python formatter built into PyCharm does the correct thing and inserts a newline after the ":" if necessary to keep the line under the max line length. {
"blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah":
"blah blah blah blah blah blah blah blah blah blah blah blah blah blah blah"
} |
I'm running into the much more simple case of: - line = f"{arg_match} {line[len(arg_match):]}"
+ line = f"{arg_match} {line[len(arg_match) :]}" Where the diff is formatter the first, and check the second... https://results.pre-commit.ci/run/github/68465360/1708361725.A80kgi75QACdOne_exTHUA @MichaReiser is this a valid bug? |
@gaborbernat yes, this is a bug (with the lint rule) and tracked in #10041 (comment) |
@gaborbernat I'm trying to reproduce your diff, but I'm struggling to understand it. What I understand is that.
That the fix's formatting doesn't precisely matches the formatter's isn't intended but accepted. We have plans to format all fixes automatically, but we aren't today. That's why we recommend running We don't intend to make the |
@henryiii told me the opposite, that the formatter should run first and then the checker because the checker is guaranteed to keep the formatters style...
I don't want to manually define a lot of stylistic checks one by one. Do you have a group alias for these? So I can add to the |
Where did I say that? That sounds reversed from what I was told by @charliermarsh and I think I always put it, the formatter is guaranteed to not introduce new failing checks, but the checker doesn't format its output. https://github.com/pypa/build/blob/9ceb49db39d81d5e7efc50a371e4612323fbdb80/.pre-commit-config.yaml#L35-L37 for example, follows that. As does https://learn.scientific-python.org/development/guides/style/#format (with "ruff would go here"). |
Perhaps I've read the opposite but is what I remembered 😅 |
Unfortunately not yet but we plan to change this with #1774 |
Anything we can do to help there? |
1 similar comment
Anything we can do to help there? |
Not yet, because we haven't started yet. But we plan to pick this up soon (I think @AlexWaygood would be the perfect person to do it) and your feedback on the categorization would be extremely valuable. |
Ruff version: 0.2.1
Consider the following code:
So
ruff format
doesn't find anything to fix, butruff check
still complains about the formatting. Is it intentional? I feel like these two commands should use the same rules when deciding what is a formatting error. But I understand the current approach too (if it was planned) - it's hard to automatically fix that long line; one can split it in two, but the position of splitting can't be chosen without understanding what the literal contains.The text was updated successfully, but these errors were encountered: