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: single_line_format_skip_with_multiple_comments preview style #8892

Closed
Tracked by #8678
MichaReiser opened this issue Nov 29, 2023 · 7 comments · Fixed by #9395
Closed
Tracked by #8678

Formatter: single_line_format_skip_with_multiple_comments preview style #8892

MichaReiser opened this issue Nov 29, 2023 · 7 comments · Fixed by #9395
Assignees
Labels
formatter Related to the formatter preview Related to preview mode features

Comments

@MichaReiser
Copy link
Member

MichaReiser commented Nov 29, 2023

Implement Black's single_line_format_skip_with_multiple_comments style in Ruff.

I don't think it's necessary to gate this change as a preview style. We should also allow the same for fmt:off and fmt:on.

# fmt:skip                           <-- single comment
# noqa:XXX # fmt:skip # a nice line  <-- multiple comments (Preview)
# pylint:XXX; fmt:skip               <-- list of comments (; separated, Preview)
@MichaReiser MichaReiser added formatter Related to the formatter preview Related to preview mode features labels Nov 29, 2023
@MichaReiser MichaReiser added this to the Formatter: Stable milestone Nov 29, 2023
@MichaReiser MichaReiser changed the title single_line_format_skip_with_multiple_comments Support using noqa: E501 and fmt:skip on the same statement. Formatter: single_line_format_skip_with_multiple_comments preview style Nov 29, 2023
@charliermarsh
Copy link
Member

I don't mind requiring a # to delimit each pragma -- we already have this requirement in the linter IIRC.

@zanieb
Copy link
Member

zanieb commented Nov 29, 2023

I like the ; delimiter for pragmas, I think # is pretty awkward.

@charliermarsh
Copy link
Member

I find # more intuitive.

@zanieb
Copy link
Member

zanieb commented Nov 29, 2023

:D Are you suggesting that we should not support using ;?

@charliermarsh
Copy link
Member

I don't feel strongly about supporting it but I probably wouldn't recommend it. We have to use something here that all tools adhere to, and # is likely the most universal and portable.

@georgettica
Copy link

georgettica commented Dec 13, 2023

for me, the big addition is I can explain in the same comment I am skipping the formatting what led me to this

also, I am against the ; as its a new delimiter, and # is that we have and use now

@charliermarsh charliermarsh self-assigned this Jan 5, 2024
charliermarsh added a commit that referenced this issue Jan 5, 2024
## Summary

This is similar to #8876, but more
limited in scope:

1. It only applies to `# fmt: skip` (like Black). Like `# isort: on`, `#
fmt: on` needs to be on its own line (still).
2. It only delimits on `#`, so you can do `# fmt: skip # noqa`, but not
`# fmt: skip - some other content` or `# fmt: skip; noqa`.

If we want to support the `;`-delimited version, we should revisit
later, since we don't support that in the linter (so `# fmt: skip; noqa`
wouldn't register a `noqa`).

Closes #8892.
@MichaReiser
Copy link
Member Author

Thanks @charliermarsh for implementing this.

Allowing # is a good first step. Supporting ; or : is something that we may want to support in the future. For example, Biome has a standardized format for specifying the reason using biome-ignore rule: <reason>. Standardizing the format has the advantage that downstream tools (e.g., a dashboard) can use the explanation.

I recommend deferring this until we look into suppression comments holistically.

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.

4 participants