-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add f-string formatting to the docs #15341
Conversation
512aa0a
to
7d2f6cc
Compare
The section does feel a bit out of place. That makes me wonder if we should rename the Black compatibility section to Style guide, rephrase the section to say that we follow Black's style guide with a few exceptions and that this section documents where we go beyond what black does? |
Interesting. This seems like a good idea. |
docs/formatter.md
Outdated
By default, Ruff formats f-string expressions. This is a [known deviation](formatter/black.md#f-strings) | ||
from Black, which does not format f-strings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: By default suggests that there's a way to disable the behavior.
By default, Ruff formats f-string expressions. This is a [known deviation](formatter/black.md#f-strings) | |
from Black, which does not format f-strings. | |
Unlike Black, Ruff formats f-string expressions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think also it might be worth making clear exactly what we mean by "f-string expressions". I.e., I believe black will change f'{foo()}'
to f"{foo()}"
-- it just won't format the interpolated expressions inside the braces, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct. Good point, I'll make the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, kind of, it's complicated. Black does change the quotes but only if there are no nested expressions with quotes.
docs/formatter.md
Outdated
Ruff employs several heuristics to determine how an f-string should be formatted, including the | ||
following considerations: | ||
|
||
* The quote style for both outer and nested f-strings | ||
* Whether to break the f-string into multiple lines | ||
* When to skip formatting the f-string altogether |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaning toward removing those bullets because they're each covered by a sub-heading (except how to skip f-string formatting)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, makes sense. I'll add the one for the last bullet point.
docs/formatter.md
Outdated
Starting with Python 3.12 ([PEP 701](https://peps.python.org/pep-0701/)), f-string expressions can | ||
span multiple lines. Ruff adopts a similar heuristic as [Prettier](https://prettier.io/docs/en/next/rationale.html#template-literals) | ||
for when to introduce a line break in an f-string expression: it introduces line breaks only | ||
if the original f-string expression already contains them. | ||
|
||
For example, the following code: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaning towards motivating this closer to what Prettier wrote
Template literals can contain interpolations. Deciding whether it's appropriate to insert a linebreak within an interpolation unfortunately depends on the semantic content of the template - for example, introducing a linebreak in the middle of a natural-language sentence is usually undesirable. Since Prettier doesn't have enough information to make this decision itself, it uses a heuristic similar to that used for objects: it will only split an interpolation expression across multiple lines if there was already a linebreak within that interpolation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've provided a link to the relevant section in Prettier which is why I thought not to add the same detail here and keep it short.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including a link to the source of our inspiration is great but I don't expect many users to click it and one important aspect of the style guide section is that we motivate our formatting. That's why I'd still prefer at least a few words why we trade consistency with flexibility.
Remaining bits after dinner
dbcccd6
to
92f8021
Compare
@MichaReiser I've made some changes to introduce a "Style Guide" section. I've removed some content from previous section because it doesn't really fit in the new section. Happy to hear any thoughts on it, I can refine the section a bit more and add some more content if it looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I really like where this is going. We might have to port some more deviations to the style guide section in the future but this is a great start.
One more nit and if we could motivate the line break behavior a bit more would be great.
docs/formatter.md
Outdated
Starting with Python 3.12 ([PEP 701](https://peps.python.org/pep-0701/)), f-string expressions can | ||
span multiple lines. Ruff adopts a similar heuristic as [Prettier](https://prettier.io/docs/en/next/rationale.html#template-literals) | ||
for when to introduce a line break in an f-string expression: it introduces line breaks only | ||
if the original f-string expression already contains them. | ||
|
||
For example, the following code: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including a link to the source of our inspiration is great but I don't expect many users to click it and one important aspect of the style guide section is that we motivate our formatting. That's why I'd still prefer at least a few words why we trade consistency with flexibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! A few grammatical nits
Summary
This PR adds a new f-string formatting section to the documentation.
It adds it to the https://docs.astral.sh/ruff/formatter page and provides a backlink in the deviation section as well.
There isn't a great place to add this kind of section in our documentation as ideally it would go under "Ruff code style" (similar to Black code style) section.
Question
I've found having the "Quotes" and "Line breaks" section in structuring the content but I'm open to removing them. Any opinions are welcome!
There can be more content to it but I'm not sure if we want to provide a lot of details. For example, we can expand on the "Quotes" section to enumerate all additional cases where we preserve the quote style.
Preview