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

[formatter] target-python option #8051

Closed
smackesey opened this issue Oct 18, 2023 · 1 comment · Fixed by #8055
Closed

[formatter] target-python option #8051

smackesey opened this issue Oct 18, 2023 · 1 comment · Fixed by #8055
Assignees

Comments

@smackesey
Copy link

smackesey commented Oct 18, 2023

Working on migrating our formatter setup to ruff. I noticed this does not work:

ruff format --target-python=py38 .

Does the target-python setting not affect the formatter? Black has this setting.

Related, ruff format --line-length=100 . does work, but line-length isn't listed under ruff format --help.

@charliermarsh
Copy link
Member

Good question... It doesn't affect the formatter right now, because we never reformat in a way that introduces incompatible syntax. For example, we don't add parentheses around context managers (though we do preserve them if they're already present). It may be necessary when we start to support preview style, since Black does add parentheses around context managers for supported versions when --preview is enabled.

Like in the linter, we don't error if you set --target-python=py38 and already use some syntax that isn't Python 3.8-compatible, because our parser just supports the latest Python syntax. (This may change in the future.) This is different than in Black, where (IIUC) they use the target version to modify the set of supported features in the parser.

(I did some research on this in #7234, if interested.)

All that being said... I think we should add this flag to the formatter, even if it doesn't modulate behavior right now. It's surprising that it doesn't exist, and it will need to exist in the future.

@charliermarsh charliermarsh self-assigned this Oct 18, 2023
charliermarsh added a commit that referenced this issue Oct 19, 2023
## Summary

This doesn't affect behavior _yet_ (see:
#7234), but it will be needed in
the future, and it's surprising to users that it doesn't exist.

Closes #8051.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants