-
Notifications
You must be signed in to change notification settings - Fork 273
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
Hover info for local bindings #4969
base: trunk
Are you sure you want to change the base?
Conversation
Hmm I feel like I'm doing something wrong here, but when I try this out I get a totally different type than in your screenshot. Also I just get "No information found" for local variables in the more involved definition that I tried. Any idea why our results might differ? I'm running a local build of 00d32cc . |
@aryairani P.s. I don't think this is ready for review from anyone yet btw; I still have to fix let-recs, figure out what's up with Cody's issue, and fix some annotations 😄 |
@ceedubs Hrmm, I get your result if I remove the |
Oh I didn't realize that I failed to copy verbatim. Something something jpegs are lossy when copy/pasting code. But I guess that it was good because I might have caught an issue :) |
No matter how much I work on something I can always trust @ceedubs to find something I missed, which I actually really appreciated 😄 ; |
@ChrisPenner I don't have a sense of how much work it is to get this to a state in which you think that it is ready for review. It's certainly not a high priority, but I just wanted to let you know that this would be a very welcome feature if you think that it's not much work to push it through :) |
d521bbc
to
50382b8
Compare
Overview
Implementation notes
Interesting/controversial decisions
Test coverage
Loose ends
TODO:
Abs
nodes in the resulting tree, and then are moved around all over the place in the tree as part of normalization, This makes it tricky to determine which variable we're referencing when hovering over the var at the binding site; ironically we can find the type of a var, just not at it's definition point 😆 . I can probably fix this either by keeping more info in the tree, or adding some special cases to how the LSP crawls the ABT, but it'll take a little more time to sort out.Unlocks new features: