-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Document range suppressions, reorganize suppression docs #21884
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
Conversation
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.
This is great.
It's a bit unfortunate that we can't land this before you added the new rule. We could consider omitting the mentions or sections mentioning the rule for now and add that later if you want to land this sooner.
Can you add a screenshot to how the new documentation looks in the browser? I'm mainly interested in seeing the navigation bar
docs/linter.md
Outdated
| - Codes to be suppressed must be separated by commas, with optional whitespace | ||
| before or after each 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 think we allow trailing commas
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.
Good point
ntBre
left a comment
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!
docs/linter.md
Outdated
| unused suppressions, run: `ruff check /path/to/file.py --extend-select RUF100 --fix`. | ||
|
|
||
| ### Inserting necessary suppression comments | ||
| ### Inserting necessary suppressions |
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 I very slightly preferred calling them suppression comments, but I'm happy either way.
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 mostly dropped the "comments" part so that the header wouldn't wrap in the sidebar, but I can put it back 😅
Edit: oh, and it also had better consistency with the title above it, "Detecting unused suppressions"
|
The three-layer deep repetition of Suppressions in the navigation isn't great but I'm not sure if I like omitting it from |
How about renaming the "Suppression Comments" section to "Comment Directives" instead? |
I think I'd lean towards just calling it The reason why I wouldn't use Directives is because it introduces an entirely new term that I don't think we use anywhere else (we sometimes use the term pragma comments but not sure if only on isuses or also in the documentation) |
da34aaa to
36a1f17
Compare
|
Updated headers and typos, and clarified ruff flag usage, moving the full commands into separate boxes. |
Would it be enough to pull out and land this commit from #21908 to define the RUF103 and RUF104 rules, even if they were never triggered, or just defer this part of the docs until that other PR is landed? |
It's a bit misleading to have rules that simply do nothing. It's also fine to wait for the rule PR to merge. It's very unlikely that you get merge conflicts on this branch |
* origin/main: (29 commits) Document range suppressions, reorganize suppression docs (#21884) Ignore ruff:isort like ruff:noqa in new suppressions (#21922) [ty] Handle `Definition`s in `SemanticModel::scope` (#21919) [ty] Attach salsa db when running ide tests for easier debugging (#21917) [ty] Don't show hover for expressions with no inferred type (#21924) [ty] avoid unions of generic aliases of the same class in fixpoint (#21909) [ty] Squash false positive logs for failing to find `builtins` as a real module [ty] Uniformly use "not supported" in diagnostics (#21916) [ty] Reduce size of ty-ide snapshots (#21915) [ty] Adjust scope completions to use all reachable symbols [ty] Rename `all_members_of_scope` to `all_end_of_scope_members` [ty] Remove `all_` prefix from some routines on UseDefMap Enable `--document-private-items` for `ruff_python_formatter` (#21903) Remove `BackwardsTokenizer` based `parenthesized_range` references in `ruff_linter` (#21836) [ty] Revert "Do not infer types for invalid binary expressions in annotations" (#21914) Skip over trivia tokens after re-lexing (#21895) [ty] Avoid inferring types for invalid binary expressions in string annotations (#21911) [ty] Improve overload call resolution tracing (#21913) [ty] fix missing heap_size on Salsa query (#21912) [ty] Support implicit type of `cls` in signatures (#21771) ...
* origin/main: (36 commits) [ty] Defer all parameter and return type annotations (#21906) [ty] Fix workspace symbols to return members too (#21926) Document range suppressions, reorganize suppression docs (#21884) Ignore ruff:isort like ruff:noqa in new suppressions (#21922) [ty] Handle `Definition`s in `SemanticModel::scope` (#21919) [ty] Attach salsa db when running ide tests for easier debugging (#21917) [ty] Don't show hover for expressions with no inferred type (#21924) [ty] avoid unions of generic aliases of the same class in fixpoint (#21909) [ty] Squash false positive logs for failing to find `builtins` as a real module [ty] Uniformly use "not supported" in diagnostics (#21916) [ty] Reduce size of ty-ide snapshots (#21915) [ty] Adjust scope completions to use all reachable symbols [ty] Rename `all_members_of_scope` to `all_end_of_scope_members` [ty] Remove `all_` prefix from some routines on UseDefMap Enable `--document-private-items` for `ruff_python_formatter` (#21903) Remove `BackwardsTokenizer` based `parenthesized_range` references in `ruff_linter` (#21836) [ty] Revert "Do not infer types for invalid binary expressions in annotations" (#21914) Skip over trivia tokens after re-lexing (#21895) [ty] Avoid inferring types for invalid binary expressions in string annotations (#21911) [ty] Improve overload call resolution tracing (#21913) ...


Issue #21874, #3711