Skip to content

Don't show hovers for typeclass dictionaries (or other generated names) #3280

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

Closed
drone29a opened this issue Oct 10, 2022 · 6 comments
Closed
Labels
type: enhancement New feature or request

Comments

@drone29a
Copy link
Contributor

Your environment

Which OS do you use?
macOS

Which version of GHC do you use and how did you install it?
9.2.2 via Stack project

How is your project built (alternative: link to the project)?
Stack.
The project is available here: https://github.com/kudu-dynamics/blaze

Which LSP client (editor/plugin) do you use?
emacs+lsp-mode

Which version of HLS do you use and how did you install it?
1.8.0.0 from ghcup

Have you configured HLS in any way (especially: a hie.yaml file)?
Yes, see the project link above for details.

What's wrong?

In certain Haskell modules, the hovertip may end up with thousands of lines of entries which reference Core variables (e.g., $dEq). I've created a patch that updates the AtPoint module in HLS to filter out variables by name when they have prefixes that look like Core variables (e.g., "$d" and "$c").

The issue seems to only happens in modules with both typeclass and type definitions. In such a module, when the cursor is on a comment above a type definition which includes a deriving clause, HLS ends up thinking there are thousands of lines to include in the hovertip. The rendering of such a large hovertip buffer can cause emacs to freeze for an extended period of time. In practice, I've noticed ~30 seconds on my several years old MBP.

Should HLS be presenting Core variables to the user through hovertips? Would a patch that removes Core variables be considered for fixing this issue? I ask because I have written such a patch in order to prevent HLS from freezing up emacs. However, I believe there may be a deeper issue: why is an HieAST for a comment associated with thousands of Core variables?

Debug information

A smaller reproducible case is available here: https://github.com/drone-rites/debug-hls

Move the cursor on a comment before one of the data definitions in the Main module and note that there are 126 lines of hover output produced with references to non-unique names of Core variables used in the implementation of type classes (e.g., $dEq_1234 becomes $dEq).

In the blaze-analysis project referenced earlier, the number of lines of hovertip documentation is in the thousands. To reproduce, place the cursor over a comment above any of the type definitions in the Blaze.Types.Pil.Checker module.

@drone29a drone29a added status: needs triage type: support User support tickets, questions, help with setup etc. labels Oct 10, 2022
@michaelpj
Copy link
Collaborator

So at the moment we show hovers for all the variables at a location, which includes typeclass dictionaries. We probably don't want to do that (nobody wants to see the type signatures of the dictionaries!). I think it would be reasonable to filter out everything generated if we can figure out how to identify them.

@michaelpj michaelpj added type: enhancement New feature or request and removed type: support User support tickets, questions, help with setup etc. status: needs triage labels Nov 2, 2022
@michaelpj michaelpj changed the title Excessively large hovertip text for comments above type definitions Don't show hovers for typeclass dictionaries (or other generated names) Nov 2, 2022
@drone29a
Copy link
Contributor Author

drone29a commented Nov 2, 2022

Thanks for clarifying the cause of the issue. I'm happy to write a patch for this. Just to be sure, your recommendation is to filter out typeclass dictionaries for all locations—not just comment locations—is that right? If so, I believe I have a patch that's ready for review.

@michaelpj
Copy link
Collaborator

I can't think of a situation where you'd want to see them, tbh. It's sort of cool that we have the information, but I don't think this is the way to display it.

@drone29a
Copy link
Contributor Author

drone29a commented Nov 2, 2022

Submitted a PR (#3316). The fix is relatively simple, but please let me know if you spot a way to make the name matching more efficient.

@santiweight
Copy link
Collaborator

@japui-coder Did you make a mistake? You've linked to the same issue you're in :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants
@drone29a @michaelpj @santiweight and others