Skip to content

Conversation

Adist319
Copy link
Contributor

Addresses #890. Implements hover functionality that shows suppressed errors when hovering over # pyrefly: ignore or # type: ignore comments.

@meta-cla meta-cla bot added the cla signed label Oct 14, 2025
@kinto0 kinto0 self-requested a review October 16, 2025 16:14
Copy link

meta-codesync bot commented Oct 16, 2025

@kinto0 has imported this pull request. If you are a Meta employee, you can view this in D84829334.

Copy link
Contributor

@kinto0 kinto0 left a comment

Choose a reason for hiding this comment

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

looks great! thank you for the contribution. and sorry it took us a while to get to this.

just a minor suggestion to keep the APIs more clear: we should add the comment range to the Suppression struct so we parse once instead of parsing on the fly.

@Adist319
Copy link
Contributor Author

@kinto0 Thanks for the feedback! Refactored to use the comment range suppression API

  • Moved comment parsing logic into parse_ignores() so ranges are computed once during initialization
  • Added TextRange field to Suppression struct to store pre-computed comment positions
  • Simplified hover.rs by using the new get_comment_range() API
  • Fixed pending suppression handling to properly clear on empty lines
  • I added a unit/smoke test for above-line ignores in ignore.rs. I was having issues adding integration tests
    for above-line ignores in the hover test file due to the cursor marker (#^) interfering with the suppression
    logic. The marker line sits between the comment and code, breaking the above-line detection. Let me know if this is fine.

Copy link
Contributor

@kinto0 kinto0 left a comment

Choose a reason for hiding this comment

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

thanks for making the changes so quickly! looks like you improved string parsing a lot, too

Copy link
Contributor

@yangdanny97 yangdanny97 left a comment

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@meta-codesync meta-codesync bot closed this in 3e8787b Oct 20, 2025
Copy link

meta-codesync bot commented Oct 20, 2025

@kinto0 merged this pull request in 3e8787b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants