Use human-readable names in LSP and playground diagnostics - #26058
Conversation
ntBre
added
playground
|
| severity: MarkerSeverity.Error, | ||
| tags: | ||
| diagnostic.code === "F401" || diagnostic.code === "F841" | ||
| diagnostic.code === "F401" || |
There was a problem hiding this comment.
Codex said this:
The clean fix is to expose the diagnostic’s existing primary_tags() through ruff_wasm and map Unnecessary/Deprecated to Monaco tags.
which sounds reasonable, especially now that we're setting tags in more rules, but I figured it could also be left for a later PR (and maybe someone who knows more typescript).
There was a problem hiding this comment.
I agree, that makes the most sense. But this can also be done as a separate PR (codex is fairly capable at writing TypeScript)
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
Thanks
It would be good to at at least one server E2E test that verifies this new behavior.
I'm also not sure if this is already on your list, but we also need to change the hover request handler to recognize rule names and the codeAction handler to use human readable names also
| severity: MarkerSeverity.Error, | ||
| tags: | ||
| diagnostic.code === "F401" || diagnostic.code === "F841" | ||
| diagnostic.code === "F401" || |
There was a problem hiding this comment.
I agree, that makes the most sense. But this can also be done as a separate PR (codex is fairly capable at writing TypeScript)
|
Added a test! Those were not on my list, but I'll follow up on them and on the tags. Thanks! |
## Summary Follow up to #26058 (comment) to use `DiagnosticTag`s directly in the playground instead of hard-coding a couple of known `Unnecessary` cases. ## Test Plan A new ruff_wasm test and manual testing in the local playground: <img width="523" height="343" alt="Screenshot 2026-06-17 at 10 53 30" src="https://github.com/user-attachments/assets/64cf0fa0-1eb4-4451-baeb-8f3f1e7622d6" /> The strikethrough seems a bit aggressive to me for deprecated tags. Codex said we could either turn off `showDeprecated` in our Monaco settings or just customize the CSS. Alternatively, I guess we could filter out deprecated tags and only show the unnecessary ones, if we were going to disable `showDeprecated`. But I'm also fine with the default styling if others are.
…n code actions (#26114) ## Summary This PR addresses the hover and code action parts of #26058 (review), as well as adding hover support for `ruff:ignore`-style comments in general, which previously did not support hovering like `noqa` comments. I went back and forth on whether to split the two commits into two PRs, but they seemed interrelated enough to leave together. ## Test Plan New e2e tests and manual testing in VS Code: https://github.com/user-attachments/assets/d568527f-8e6c-482d-8417-13bd8c0d06b0 You'll probably notice a range issue like I did in this video. I fixed that in the third commit but just recorded a short follow up video instead of trying to redo the full video: <details><summary>Updated range</summary> <p> https://github.com/user-attachments/assets/dfe54181-88d8-42a7-842e-855356ceea6b </p> </details> --------- Co-authored-by: Micha Reiser <micha@reiser.io>
2 participants
Summary
This PR follows up on #25937 to display human-readable names in the LSP and playground too, when preview is enabled.
Test Plan
In VS Code with
preview = truein myruff.toml:with
preview = false:In the playground with preview enabled:
and with preview disabled: