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

F-String formatting in assignment positions #13813

Closed
Tracked by #13371
MichaReiser opened this issue Oct 18, 2024 · 4 comments · Fixed by #14454
Closed
Tracked by #13371

F-String formatting in assignment positions #13813

MichaReiser opened this issue Oct 18, 2024 · 4 comments · Fixed by #14454
Assignees
Labels
formatter Related to the formatter preview Related to preview mode features

Comments

@MichaReiser
Copy link
Member

The new f-string formatting in assignment-value positions seems inconsistent to me:

aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{
    expression}moreeeeeeeeeeeeeeeee"

Gets formatted to:

aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{
    expression
}moreeeeeeeeeeeeeeeee"

Which avoids parentheses but it doesn't feel like the ideal formatting and it doesn't match the formatting if the expression starts out "flat"

aaaaaaaaaaaaaaaaaa = (
    f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee"
)
@MichaReiser MichaReiser added formatter Related to the formatter preview Related to preview mode features labels Oct 18, 2024
@MichaReiser MichaReiser mentioned this issue Oct 18, 2024
29 tasks
@MichaReiser
Copy link
Member Author

Hmm, I might have to fix this for the implicit concatenated string formatting because it handles this correctly and this now results in instable formatting

@MichaReiser
Copy link
Member Author

One challenge is that we don't want to inline the comments if the expression breaks:

a = f"test{
    expression
}flat" f"can be {
    joined
} togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" # inline

Should be formatted to

a = (
    f"test{expression}flatcan be {
        joined
    } togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
)   # inline

and not

```python
a = (
    f"test{expression}flatcan be {
        joined
    } togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"  # inline
)

@dhruvmanila
Copy link
Member

Hmm, I might have to fix this for the implicit concatenated string formatting because it handles this correctly and this now results in instable formatting

@MichaReiser Just want to check-in whether you ended up fixing this with implicit concatenated string formatting?

@MichaReiser
Copy link
Member Author

I fixed the instability but I didn't fix the formatting. I suggest waiting with this task until the implicit concatenated string formatting PR is merged or that you work on top of it. We'll otherwise end up with horrible merge conflicts

@dhruvmanila dhruvmanila self-assigned this Nov 15, 2024
dhruvmanila added a commit that referenced this issue Nov 26, 2024
## Summary

fixes: #13813

This PR fixes a bug in the formatting assignment statement when the
value is an f-string.

This is resolved by using custom best fit layouts if the f-string is (a)
not already a flat f-string (thus, cannot be multiline) and (b) is not a
multiline string (thus, cannot be flattened). So, it is used in cases
like the following:
```py
aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{
    expression}moreeeeeeeeeeeeeeeee"
```
Which is (a) `FStringLayout::Multiline` and (b) not a multiline.

There are various other examples in the PR diff along with additional
explanation and context as code comments.

## Test Plan

Add multiple test cases for various scenarios.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter preview Related to preview mode features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants