-
-
Notifications
You must be signed in to change notification settings - Fork 364
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
Jump to instance definition and explain typeclass evidence #4392
base: master
Are you sure you want to change the base?
Conversation
0ccc4d3
to
5568ca3
Compare
Just at a high level, am I right that if I have
and I call "go to implementation" on "foo" I will get the option to go to the |
Yes, that's exactly what will happen! |
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.
Definitely needs some tests, it's not at all clear to me how it's going to show up.
-> Position | ||
-> MaybeT m [(Location, Identifier)] | ||
gotoDefinition withHieDb getHieFile ideOpts imports srcSpans pos | ||
= lift $ locationsAtPoint withHieDb getHieFile ideOpts imports pos srcSpans | ||
|
||
-- | Locate the implementation definition of the name at a given position. | ||
-- Finds the implementation for a overloaded function. |
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 can keep saying "gotoImplementation" and so on, since this corresponds to that LSP method, but let's make this comment clear about what it actually does!
] | ||
prettyName (Right n, dets) | ||
-- We don't want to print evidence variables as they are generated. | ||
| any isEvidenceUse (identInfo dets) = pure $ maybe "" (printOutputable . renderEvidenceTree) (getEvidenceTree rf n) <> "\n" |
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.
the maybe ""
seems supicious. What case is that for?
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.
The getEvidenceTree
may return Nothing
for a variety of reasons. For example, when the name is unexpectedly not found in the reference map. I think in practice, it will always produce something.
We could log it if we failed to build the EvidenceTree for a given name.
@@ -307,6 +331,39 @@ atPoint IdeOptions{} (HAR _ hf _ _ (kind :: HieKind hietype)) (DKMap dm km) env | |||
UnhelpfulLoc {} | isInternalName name || isSystemName name -> Nothing | |||
_ -> Just $ "*Defined " <> printOutputable (pprNameDefnLoc name) <> "*" | |||
|
|||
-- We want to render the root constraint even if it is a let, | |||
-- but we don't want to render any subsequent lets |
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'm assuming this makes sense to someone who knows how evidence is represented in GHC...
Can we include a test to see whether this now gives us some kind of useful hover information on overloaded record dot usage? |
dfb7002
to
dacc0b7
Compare
dacc0b7
to
95a50f3
Compare
Adds the necessary instances for handling the request type `Method_TextDocumentImplementation`. Further, wire up the appropriate handlers for the "gotoImplementation" request.
95a50f3
to
0557bba
Compare
Rebased and slightly modified version of #1983
Still requires some tests and docs.