-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Rework the documentation to incorporate the Ruff formatter #7732
Conversation
e81c509
to
c64f374
Compare
docs/formatter.md
Outdated
- [`bad-quotes-inline-string`](rules/bad-quotes-inline-string.md) | ||
- [`bad-quotes-multiline-string`](rules/bad-quotes-multiline-string.md) | ||
- [`bad-quotes-docstring`](rules/bad-quotes-docstring.md) | ||
- [`avoidable-escaped-quote`](rules/avoidable-escaped-quote.md) |
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.
These aren't necessarily conflicting with the formatter. The quote rules are probably consistent with it, assuming you have the same quote settings. And the trailing comma rules won't conflict with the formatter, they'll just end up changing the resulting formatting.
docs/formatter.md
Outdated
|
||
This will likely be incorporated into Black's preview style ([#3823](https://github.com/psf/black/pull/3823)). | ||
|
||
<h4><code>global</code> and <code>nonlocal</code> names are broken across multiple lines by continuations</h4> |
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.
Using <h4>
here ensures that this long list of intentional deviations is omitted from the table of contents on the left side of the page, which IMO is good. However, it also means they can't be permalinked, which IMO is bad...
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.
Can you manually give them id
? It's still not really accessible to users but at least it's possible if you know.
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 decided to move them to their own page.
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
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.
📜 📜 📜
docs/formatter.md
Outdated
|
||
This will likely be incorporated into Black's preview style ([#3823](https://github.com/psf/black/pull/3823)). | ||
|
||
<h4><code>global</code> and <code>nonlocal</code> names are broken across multiple lines by continuations</h4> |
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.
Can you manually give them id
? It's still not really accessible to users but at least it's possible if you know.
c64f374
to
38289bc
Compare
CodSpeed Performance ReportMerging #7732 will not alter performanceComparing Summary
|
72a64c6
to
72a1f4d
Compare
9fd6737
to
0b67439
Compare
0b67439
to
a086cde
Compare
We should add an example for how to exclude certain files from linting / formatting only #8000 |
@@ -208,26 +217,44 @@ exclude = [ | |||
# Same as Black. | |||
line-length = 88 |
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.
line-length = 88 | |
line-length = 88 | |
indent-width = 4 |
@@ -45,22 +37,45 @@ exclude = [ | |||
"node_modules", | |||
"venv", | |||
] | |||
per-file-ignores = {} | |||
|
|||
# Same as Black. | |||
line-length = 88 |
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.
line-length = 88 | |
indent-width = 4 |
@@ -45,22 +37,45 @@ exclude = [ | |||
"node_modules", | |||
"venv", | |||
] | |||
per-file-ignores = {} | |||
|
|||
# Same as Black. | |||
line-length = 88 |
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.
line-length = 88 | |
line-width = 88 |
# ...but use a different line length. | ||
line-length = 100 |
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.
# ...but use a different line length. | |
line-length = 100 | |
# ...but use a different line width. | |
line-width = 100 |
docs/tutorial.md
Outdated
# Decrease the maximum line length to 79 characters. | ||
line-length = 79 |
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.
# Decrease the maximum line length to 79 characters. | |
line-length = 79 | |
# Decrease the maximum line width to 79. | |
line-width = 79 |
docs/formatter.md
Outdated
formatter's treatment of import statements when set to non-default values: | ||
|
||
- [`lines-after-imports`](https://docs.astral.sh/ruff/settings/#isort-lines-after-imports) | ||
- [`lines-between-types`](https://docs.astral.sh/ruff/settings/#isort-lines-between-types) |
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.
Our internal configuration document mentions more conflicting rules and isort options. Are there reasons why you excluded some of the rules and options from here?
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'll add them, but it's the same confusion as in your PR that enables the warnings -- what constitutes a "conflict"?
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 also added ISC001
and Q003
, were those intentionally omitted?
7acc677
to
c365de2
Compare
Made all changes except I haven't done the |
2adc81f
to
cf8ba7a
Compare
527b370
to
27a56f6
Compare
## Summary As a follow-up of #7732, use `tool.ruff.lint` in more places in documentations, tests and internal usages.
Summary
This PR updates our documentation for the upcoming formatter release.
Broadly, the documentation is now structured as follows:
ruff check
ruff format
pre-commit
The major changes include:
[tool.ruff.lint]
and[tool.ruff.format]
.My suggestion is to pull and build the docs locally, and review by reading them in the browser rather than trying to parse all the code changes.
Closes #7235.
Closes #7647.