Skip to content

Conversation

@amyreese
Copy link
Member

@amyreese amyreese commented Dec 10, 2025

  • Reorganize suppression documentation, document range suppressions
  • Note preview mode requirement

Issue #21874, #3711

@amyreese amyreese added documentation Improvements or additions to documentation suppression Related to supression of violations e.g. noqa labels Dec 10, 2025
@amyreese amyreese linked an issue Dec 10, 2025 that may be closed by this pull request
Copy link
Member

@MichaReiser MichaReiser left a 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
Comment on lines 404 to 405
- Codes to be suppressed must be separated by commas, with optional whitespace
before or after each code.
Copy link
Member

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point

@amyreese
Copy link
Member Author

Screenshot 2025-12-10 at 08-27-15 The Ruff Linter Ruff

Copy link
Contributor

@ntBre ntBre left a 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
Copy link
Contributor

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.

Copy link
Member Author

@amyreese amyreese Dec 10, 2025

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"

@MichaReiser
Copy link
Member

The three-layer deep repetition of Suppressions in the navigation isn't great but I'm not sure if I like omitting it from Inline, File, Range

@amyreese
Copy link
Member Author

The three-layer deep repetition of Suppressions in the navigation isn't great but I'm not sure if I like omitting it from Inline, File, Range

How about renaming the "Suppression Comments" section to "Comment Directives" instead?

@MichaReiser
Copy link
Member

How about renaming the "Suppression Comments" section to "Comment Directives" instead?

I think I'd lean towards just calling it Comments and then use file-level, line-level, block-level as the subheadings similar to pyright https://microsoft.github.io/pyright/#/comments

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)

@amyreese amyreese force-pushed the amy/document-suppressions branch from da34aaa to 36a1f17 Compare December 10, 2025 18:17
@amyreese
Copy link
Member Author

Updated headers and typos, and clarified ruff flag usage, moving the full commands into separate boxes.

@amyreese
Copy link
Member Author

Screenshot 2025-12-10 at 10-21-36 The Ruff Linter Ruff

@amyreese
Copy link
Member Author

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.

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?

@MichaReiser
Copy link
Member

MichaReiser commented Dec 11, 2025

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

@amyreese amyreese merged commit c055d66 into main Dec 11, 2025
35 checks passed
@amyreese amyreese deleted the amy/document-suppressions branch December 11, 2025 19:16
dcreager added a commit that referenced this pull request Dec 11, 2025
* 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)
  ...
dcreager added a commit that referenced this pull request Dec 11, 2025
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation suppression Related to supression of violations e.g. noqa

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document new range suppressions

4 participants